From 79dfbf7b2168497c5c7627413439709eb00a30e2 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 18 Dec 2024 15:28:29 +0100 Subject: [PATCH 1/4] Python: Add FastAPI request test Co-authored-by: Joe Farebrother --- .../frameworks/fastapi/taint_test.py | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/python/ql/test/library-tests/frameworks/fastapi/taint_test.py b/python/ql/test/library-tests/frameworks/fastapi/taint_test.py index 8b91dbb142c3..3f5fbb7be8c6 100644 --- a/python/ql/test/library-tests/frameworks/fastapi/taint_test.py +++ b/python/ql/test/library-tests/frameworks/fastapi/taint_test.py @@ -187,3 +187,38 @@ async def websocket_test(websocket: WebSocket): # $ requestHandler routedParamet async for data in websocket.iter_json(): ensure_tainted(data) # $ tainted + + +# --- Request --- + +import starlette.requests +from fastapi import Request + + +assert Request == starlette.requests.Request + +@app.websocket("/req") # $ routeSetup="/req" +async def request_test(request: Request): # $ requestHandler routedParameter=request + ensure_tainted( + request, # $ tainted + + await request.body(), # $ MISSING: tainted + + await request.json(), # $ MISSING: tainted + await request.json()["key"], # $ MISSING: tainted + + # form() returns a FormDat (which is a starlette ImmutableMultiDict) + await request.form(), # $ MISSING: tainted + await request.form()["key"], # $ MISSING: tainted + await request.form().getlist("key"), # $ MISSING: tainted + await request.form().getlist("key")[0], # $ MISSING: tainted + # data in the form could be an starlette.datastructures.UploadFile + await request.form()["file"].filename, # $ MISSING: tainted + await request.form().getlist("file")[0].filename, # $ MISSING: tainted + + request.cookies, # $ MISSING: tainted + request.cookies["key"], # $ MISSING: tainted + ) + + async for chunk in request.stream(): + ensure_tainted(chunk) # $ MISSING: tainted From 34631a8784652d7b680f4cdbf61621066877a829 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 18 Dec 2024 15:28:29 +0100 Subject: [PATCH 2/4] Python: Model FastAPI requests Co-authored-by: Joe Farebrother --- .../semmle/python/frameworks/Starlette.qll | 56 +++++++++++++++++++ .../frameworks/fastapi/taint_test.py | 18 +++--- 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/Starlette.qll b/python/ql/lib/semmle/python/frameworks/Starlette.qll index 7ced137fcfca..0bbd83b1584a 100644 --- a/python/ql/lib/semmle/python/frameworks/Starlette.qll +++ b/python/ql/lib/semmle/python/frameworks/Starlette.qll @@ -245,4 +245,60 @@ module Starlette { override DataFlow::Node getAPathArgument() { result = this.getParameter(0, "path").asSink() } } + + /** + * Provides models for the `starlette.requests.Request` class + * + * See https://www.starlette.io/requests/. + */ + module Request { + /** Gets a reference to the `starlette.requests.Request` class. */ + API::Node classRef() { + result = API::moduleImport("starlette").getMember("requests").getMember("Request") + or + result = API::moduleImport("fastapi").getMember("Request") + } + + /** + * A source of instances of `starlette.requests.Request`, extend this class to model new instances. + * + * This can include instantiations of the class, return values from function + * calls, or a special parameter that will be set when functions are called by an external + * library. + * + * Use the predicate `Request::instance()` to get references to instances of `starlette.requests.Request`. + */ + abstract class InstanceSource extends DataFlow::LocalSourceNode { } + + /** A direct instantiation of `starlette.requests.Request`. */ + private class ClassInstantiation extends InstanceSource { + ClassInstantiation() { this = classRef().getAnInstance().asSource() } + } + + /** Gets a reference to an instance of `starlette.requests.Request`. */ + private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) { + t.start() and + result instanceof InstanceSource + or + exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t)) + } + + /** Gets a reference to an instance of `starlette.requests.Request`. */ + DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) } + + /** + * Taint propagation for `starlette.requests.Request`. + */ + private class InstanceTaintSteps extends InstanceTaintStepsHelper { + InstanceTaintSteps() { this = "starlette.requests.Request" } + + override DataFlow::Node getInstance() { result = instance() } + + override string getAttributeName() { result in ["cookies"] } + + override string getMethodName() { none() } + + override string getAsyncMethodName() { result in ["body", "json", "form", "stream"] } + } + } } diff --git a/python/ql/test/library-tests/frameworks/fastapi/taint_test.py b/python/ql/test/library-tests/frameworks/fastapi/taint_test.py index 3f5fbb7be8c6..2ceca3b9dd0a 100644 --- a/python/ql/test/library-tests/frameworks/fastapi/taint_test.py +++ b/python/ql/test/library-tests/frameworks/fastapi/taint_test.py @@ -202,23 +202,23 @@ async def request_test(request: Request): # $ requestHandler routedParameter=req ensure_tainted( request, # $ tainted - await request.body(), # $ MISSING: tainted + await request.body(), # $ tainted - await request.json(), # $ MISSING: tainted - await request.json()["key"], # $ MISSING: tainted + await request.json(), # $ tainted + await request.json()["key"], # $ tainted - # form() returns a FormDat (which is a starlette ImmutableMultiDict) - await request.form(), # $ MISSING: tainted - await request.form()["key"], # $ MISSING: tainted + # form() returns a FormData (which is a starlette ImmutableMultiDict) + await request.form(), # $ tainted + await request.form()["key"], # $ tainted await request.form().getlist("key"), # $ MISSING: tainted await request.form().getlist("key")[0], # $ MISSING: tainted # data in the form could be an starlette.datastructures.UploadFile await request.form()["file"].filename, # $ MISSING: tainted await request.form().getlist("file")[0].filename, # $ MISSING: tainted - request.cookies, # $ MISSING: tainted - request.cookies["key"], # $ MISSING: tainted + request.cookies, # $ tainted + request.cookies["key"], # $ tainted ) async for chunk in request.stream(): - ensure_tainted(chunk) # $ MISSING: tainted + ensure_tainted(chunk) # $ tainted From 2b3fc9b36c5539fe1ab0d6fca95654ce935a5103 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 18 Dec 2024 15:28:29 +0100 Subject: [PATCH 3/4] Python: Add change-note --- .../lib/change-notes/2024-12-18-fastapi-request-modeling.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 python/ql/lib/change-notes/2024-12-18-fastapi-request-modeling.md diff --git a/python/ql/lib/change-notes/2024-12-18-fastapi-request-modeling.md b/python/ql/lib/change-notes/2024-12-18-fastapi-request-modeling.md new file mode 100644 index 000000000000..9cb58098c4a7 --- /dev/null +++ b/python/ql/lib/change-notes/2024-12-18-fastapi-request-modeling.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added modeling of `fastapi.Request`, which will improve taint-flow for users of FastAPI. From a9704d8de0c3d45959031139a04d75cb5213f3b3 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Thu, 19 Dec 2024 14:08:23 +0100 Subject: [PATCH 4/4] Update change-note wording Co-authored-by: Aditya Sharad <6874315+adityasharad@users.noreply.github.com> --- .../ql/lib/change-notes/2024-12-18-fastapi-request-modeling.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/ql/lib/change-notes/2024-12-18-fastapi-request-modeling.md b/python/ql/lib/change-notes/2024-12-18-fastapi-request-modeling.md index 9cb58098c4a7..adc7d39653f7 100644 --- a/python/ql/lib/change-notes/2024-12-18-fastapi-request-modeling.md +++ b/python/ql/lib/change-notes/2024-12-18-fastapi-request-modeling.md @@ -1,4 +1,5 @@ --- category: minorAnalysis --- -* Added modeling of `fastapi.Request`, which will improve taint-flow for users of FastAPI. +* Added modeling of `fastapi.Request` and `starlette.requests.Request` as sources of untrusted input, + and modeling of tainted data flow out of these request objects.