From 322e691a3fcc745bd01b0f93a88bc60e29b09ee3 Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 26 Sep 2026 02:14:46 +0200 Subject: [PATCH 1/2] test: expect the text attributes and URL parameters of a request to be text --- tests/test_nosql_injection.py | 146 +++++++++++++++++++++++++++++++--- 1 file changed, 136 insertions(+), 10 deletions(-) diff --git a/tests/test_nosql_injection.py b/tests/test_nosql_injection.py index dd29831..cbfef12 100644 --- a/tests/test_nosql_injection.py +++ b/tests/test_nosql_injection.py @@ -11,9 +11,13 @@ The engine ships no NOSQL sink; these tests declare one, ``docstore.find(filter)``. -Limits: a Django or aiohttp request object is tainted as a whole, so ``request.GET`` -carries ``NOSQL`` like ``request.body``; the keys of a dict literal are not followed, so -a string concatenated into ``$where`` is not reported. +Django, Django REST framework, aiohttp and tornado hand views a request object, tainted as +a whole: its text attributes (``GET``, ``headers``, ``query``, ``arguments``) are text, its +body and files are not, and the URL parameters a view receives after it are text. + +Limits: a helper function the request is passed to reads it as a whole, since function +summaries do not know where their parameters come from; the keys of a dict literal are +not followed, so a string concatenated into ``$where`` is not reported. """ from __future__ import annotations @@ -176,9 +180,12 @@ def test_standard_input_is_a_payload_and_the_command_line_text(plugins: list[Pat DJANGO = ( - "import json\n\nimport docstore\nfrom django.http import HttpRequest, JsonResponse\n\n" - "def users(request: HttpRequest):\n" - " return JsonResponse(docstore.find({query}))\n" + "import json\n\nimport docstore\nfrom django.http import HttpRequest, JsonResponse\nfrom django.urls import path\n\n" + "def search(request, slug):\n" + " return JsonResponse(docstore.find({query}))\n\n" + "def typed(request: HttpRequest):\n" + " return JsonResponse(docstore.find({query}))\n\n" + "urlpatterns = [path('search//', search)]\n" ) @@ -186,12 +193,131 @@ def test_standard_input_is_a_payload_and_the_command_line_text(plugins: list[Pat "query", [ "json.loads(request.body)", - # Limit: the request object is tainted as a whole, its text fields too. - '{"name": request.GET["name"]}', + "json.loads(request.read())", + "json.load(request.FILES['filter'])", + ], +) +def test_the_body_and_files_of_a_django_request_are_payloads(plugins: list[Path], query: str) -> None: + assert injected(plugins, DJANGO.format(query=query)) == [8, 11] + + +@pytest.mark.parametrize( + "query", + [ + "{'name': request.GET['name']}", + "{'name': request.POST.get('name')}", + "{'agent': request.headers['User-Agent'], 'meta': request.META['REMOTE_ADDR']}", + "{'token': request.COOKIES['token'], 'path': request.get_full_path()}", + ], +) +def test_the_text_attributes_of_a_django_request_are_text(plugins: list[Path], query: str) -> None: + assert injected(plugins, DJANGO.format(query=query)) == [] + + +def test_the_url_parameters_after_the_request_are_text(plugins: list[Path]) -> None: + text = DJANGO.format(query="{'tag': slug}").replace("def typed(request: HttpRequest)", "def typed(request, slug)") + + assert injected(plugins, text) == [] + + +@pytest.mark.parametrize( + "query, reported", + [ + ("json.loads(request.body)", True), + ("json.loads(self.request.body)", True), + ("{'name': request.POST['name']}", False), + ("{'name': self.request.GET['name']}", False), + ("{'tag': slug}", False), ], ) -def test_a_django_request_carries_nosql_as_a_whole(plugins: list[Path], query: str) -> None: - assert injected(plugins, DJANGO.format(query=query)) == [7] +def test_a_class_based_view_receives_the_same_request(plugins: list[Path], query: str, reported: bool) -> None: + text = ( + "import json\n\nimport docstore\nfrom django.views import View\n\n" + "class Search(View):\n" + " def post(self, request, slug):\n" + f" return docstore.find({query})\n" + ) + + assert injected(plugins, text) == ([8] if reported else []) + + +@pytest.mark.parametrize("query, reported", [("request.data", True), ("{'q': request.query_params['q']}", False)]) +def test_a_rest_framework_request_parses_the_body_into_data(plugins: list[Path], query: str, reported: bool) -> None: + text = ( + "import docstore\nfrom rest_framework.decorators import api_view\n\n" + "@api_view(['POST'])\n" + "def create(request):\n" + f" return docstore.find({query})\n" + ) + + assert injected(plugins, text) == ([6] if reported else []) + + +@pytest.mark.parametrize( + "query, reported", + [ + ("await request.json()", True), + ("{'name': request.match_info['name']}", False), + ("{'q': request.query['q'], 'agent': request.headers['User-Agent']}", False), + ("{'name': (await request.post())['name']}", False), + ], +) +def test_an_aiohttp_request_is_text_but_for_its_body(plugins: list[Path], query: str, reported: bool) -> None: + text = ( + "import docstore\nfrom aiohttp import web\n\n" + "routes = web.RouteTableDef()\n\n" + "@routes.post('/users/{{name}}')\n" + "async def users(request):\n" + f" return docstore.find({query})\n\n" + "async def typed(request: web.Request):\n" + f" return docstore.find({query})\n" + ) + + assert injected(plugins, text) == ([8, 11] if reported else []) + + +@pytest.mark.parametrize( + "query, reported", + [ + ("json.loads(self.request.body)", True), + ("{'args': self.request.arguments, 'uri': self.request.uri}", False), + ("{'name': name}", False), + ], +) +def test_a_tornado_request_is_text_but_for_its_body(plugins: list[Path], query: str, reported: bool) -> None: + text = ( + "import json\n\nimport docstore\nimport tornado.web\n\n" + "class Users(tornado.web.RequestHandler):\n" + " def post(self, name):\n" + f" docstore.find({query})\n" + ) + + assert injected(plugins, text) == ([8] if reported else []) + + +def test_text_attributes_still_carry_the_other_kinds(plugins: list[Path]) -> None: + text = ( + "from django.http import HttpResponse\nfrom django.urls import path\n\n" + "def hello(request, slug):\n" + " return HttpResponse(request.GET['name'] + slug)\n\n" + "urlpatterns = [path('hello//', hello)]\n" + ) + + assert [f.span.start_line for f in check(plugins, text) if f.rule_id == "xss"] == [5] + + +def test_a_helper_the_request_is_passed_to_reads_it_as_a_whole(plugins: list[Path]) -> None: + text = ( + "import docstore\nfrom rest_framework.decorators import api_view\n\n" + "def lookup(request):\n" + " return docstore.find({'name': request.GET['name']})\n\n" + "@api_view(['GET'])\n" + "def view(request):\n" + " return lookup(request)\n" + ) + + # Limit: the summary of ``lookup`` does not know its parameter is a request. + assert injected(plugins, text) == [9] def test_parameters_of_one_entry_point_remain_one_origin(plugins: list[Path]) -> None: From 4a188505c3e62f31db37914a23718c2a43176576 Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 26 Sep 2026 02:14:46 +0200 Subject: [PATCH 2/2] feat: let the text attributes and URL parameters of a request object carry no NOSQL A RequestObject model names the inputs that are request objects: the URL parameters a view receives after the request are text, and so is an attribute of the request its model lists, read directly or below a request source such as self.request. --- CHANGELOG.md | 10 ++++ docs/plugins.md | 1 + docs/usage.md | 2 +- .../bundled/models/aiohttp/aiohttp_models.py | 32 ++++++++---- .../bundled/models/django/django_models.py | 19 +++++++ .../bundled/models/tornado/tornado_models.py | 17 ++++++- src/coretrace_python/taint/__init__.py | 2 + src/coretrace_python/taint/engine.py | 51 ++++++++++++++----- src/coretrace_python/taint/models.py | 25 +++++++++ 9 files changed, 135 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 875c9e6..2cc3747 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## Unreleased + +### Precision + +- The text attributes and URL parameters of a request object carry no `NOSQL`. Django, REST framework, aiohttp and tornado views read a request tainted as a whole, so `request.GET["name"]` and a URL parameter were `nosql-injection` like the body. Now `GET`, `POST`, `COOKIES`, `META`, `headers`, `query_params`, aiohttp's `query` and `match_info`, tornado's `arguments` and the other text attributes are text, and so are the URL parameters a view receives after the request. The body, the files, REST framework's `data` and aiohttp's `json()` keep the kinds of the request. The other kinds are unchanged, and so is the regression corpus. A helper function the request is passed to still reads it as a whole (#162). + +### Plugins + +- A `RequestObject(symbol, text=())` model says that the inputs from an entry point, a route registrar, a typed parameter or a source are request objects, and which of their attributes are text (#162). + ## 0.12.0 (2026-09-26) ### Detection diff --git a/docs/plugins.md b/docs/plugins.md index 30379c3..b5038c8 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -151,6 +151,7 @@ class BottleModels(ModelPlugin): | `SafeArgument(symbol, argument, values, position=None, kinds=ALL)` | A call to the sink `symbol` whose `argument` (by keyword, or at `position`) denotes one of `values` is not a sink for `kinds`, such as `yaml.load` with `Loader=yaml.SafeLoader`. Values are written as the analyzer records arguments: a symbol by its canonical name, a constant as Python writes it (`True`). Only a value given explicitly makes the call safe. | | `TemplateRender(symbol, position=0, keyword="template_name")` | A call rendering, with autoescaping, the template named by its argument at `position` or `keyword`, such as `render_to_string`. What it returns carries no `HTML` when the name is written in the call and the project's template escapes everything it prints (see the usage guide). | | `Validator(symbol, kinds=ALL, argument=0)` | A callable whose truth proves its argument safe for `kinds`, such as `re.fullmatch`. A flow guarded by it is refuted when it carries no other kind into the sink, and is a hotspot otherwise: a redirect validator does not make a shell command safe. `symbol` may be a function of the project, by its project symbol (`python.hc.accounts.views._allow_redirect`), which then counts wherever it is called, in its own module too. | +| `RequestObject(symbol, text=())` | The inputs from `symbol`, an entry point, a route registrar, a typed parameter or a source, are request objects. A view receives the request first (after `self`), then URL parameters, which are text. Reading one of the request's `text` attributes gives text, as Django's `GET` or aiohttp's `query`; its body and files keep the request's kinds. An attribute read counts only when every source of the value is such a request. | | `AuthorizationGuard(symbol, label)` | A decorator or a condition restricting who reaches the code, such as `login_required`; a flow behind it is a hotspot. | | `Members(symbol, dynamic=None, defined=(), typed=())` | What the attributes and items of an instance of the class `symbol` give, for objects that give others by name. `typed` maps an attribute to the class of what it gives, called or not, since a call denotes its callee: pymongo's `get_database` gives a `Database`, as Flask-PyMongo's `db` property does. `dynamic`, when set, is the class of every item and of every attribute outside `defined` and `typed`: a pymongo client gives a `Database` by name (`client.shop`, `client["shop"]`), a database a `Collection`. Every path to a collection then denotes the collection class, so a sink on `Collection.find` covers them all. Without `dynamic`, an attribute keeps its path and an item its container's symbol, as for any class. | diff --git a/docs/usage.md b/docs/usage.md index 1cdc1f3..f47201b 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -112,7 +112,7 @@ variable is not treated that way yet. | `command-injection` | Shell or process execution (`os.system`, `subprocess` with a string, …). | | `code-injection` | The code `eval` or `exec` runs; the globals and locals passed with it are data. `dangerous-eval` reports every such call, this rule the ones attacker input reaches. | | `sql-injection` | A database statement (`cursor.execute`, SQLAlchemy `text`, Django `raw`, …). Parameters of a parameterised query are not statements. | -| `nosql-injection` | A NoSQL query, such as a MongoDB filter, given attacker input that may hold a structure: a JSON body giving `{"$ne": null}` matches any value. The input that may hold a structure is JSON, the raw bodies a `json.loads` decodes, files, WebSocket messages and standard input. Text cannot hold an operator: form fields, query parameters, headers, cookies, URL parameters, the command line, the environment, and an entry-point parameter annotated with scalars (`str`, `int`, `Optional[str]`, `Annotated[str, Query()]`, `list[str]`, …), which FastAPI validates. Limits: a Django or aiohttp request object is tainted as a whole, so `request.GET` counts like `request.body`; the keys of a dict are not followed, so a string concatenated into `$where` is not reported. The analyzer ships no NoSQL sink: model plugins declare them. | +| `nosql-injection` | A NoSQL query, such as a MongoDB filter, given attacker input that may hold a structure: a JSON body giving `{"$ne": null}` matches any value. The input that may hold a structure is JSON, the raw bodies a `json.loads` decodes, files, WebSocket messages and standard input. Text cannot hold an operator: form fields, query parameters, headers, cookies, URL parameters, the command line, the environment, and an entry-point parameter annotated with scalars (`str`, `int`, `Optional[str]`, `Annotated[str, Query()]`, `list[str]`, …), which FastAPI validates. A Django, REST framework, aiohttp or tornado view reads a request object: its text attributes (`GET`, `POST`, `headers`, `query_params`, `query`, `match_info`, `arguments`, …) and the URL parameters after it are text, its body, files and `data` are not. Limits: a helper function the request is passed to reads it as a whole; the keys of a dict are not followed, so a string concatenated into `$where` is not reported. The analyzer ships no NoSQL sink: model plugins declare them. | | `path-traversal` | A file system path (`open`, `send_file`, `os.remove`, …). | | `ssrf` | The URL of an HTTP client request (Requests, httpx, `urllib`), by position or as `url=`; the body, headers and query parameters are not the destination. | | `xss` | An HTTP response body built without escaping. A Django template rendered by `render_to_string` escapes, where the analyzer can establish it (below). | diff --git a/src/coretrace_python/bundled/models/aiohttp/aiohttp_models.py b/src/coretrace_python/bundled/models/aiohttp/aiohttp_models.py index 466acc7..dc0d272 100644 --- a/src/coretrace_python/bundled/models/aiohttp/aiohttp_models.py +++ b/src/coretrace_python/bundled/models/aiohttp/aiohttp_models.py @@ -12,6 +12,7 @@ from coretrace_python.taint import ( EntryPoint, Model, + RequestObject, RouteRegistrar, Sink, Source, @@ -22,6 +23,23 @@ _METHODS = ("get", "post", "put", "patch", "delete", "head", "options") _REQUEST_CLASSES = ("aiohttp.web.Request", "aiohttp.web_request.Request") _ROUTERS = ("aiohttp.web.Application.router", "aiohttp.web.UrlDispatcher") +# ``@routes.get('/')`` on a ``RouteTableDef``. +_ROUTE_TABLES = tuple(f"aiohttp.web.RouteTableDef.{method}" for method in (*_METHODS, "route", "view")) +# ``app.router.add_route('GET', '/', handler)``, ``app.router.add_get('/', handler)`` and +# ``app.add_routes([web.get('/', handler)])``, each with the argument naming the handler. +_ROUTE_CALLS = ( + *((f"{router}.add_route", 2) for router in _ROUTERS), + *((f"{router}.add_{method}", 1) for router in _ROUTERS for method in (*_METHODS, "view")), + ("aiohttp.web.route", 2), + *((f"aiohttp.web.{method}", 1) for method in (*_METHODS, "view")), +) +# What a request gives as text; its body (``json()``, ``text()``, ``read()``) may hold a +# structure. +_REQUEST_TEXT = ( + "query", "rel_url", "url", "path", "raw_path", "path_qs", "query_string", "match_info", "headers", + "raw_headers", "cookies", "method", "host", "scheme", "remote", "forwarded", "content_type", "charset", + "if_modified_since", "post", +) _REDIRECTS = ("HTTPFound", "HTTPMovedPermanently", "HTTPSeeOther", "HTTPTemporaryRedirect", "HTTPPermanentRedirect") _CLIENT_FUNCTIONS = ( *(f"aiohttp.ClientSession.{method}" for method in (*_METHODS, "request")), @@ -40,18 +58,12 @@ class AiohttpModels(ModelPlugin): # The whole request is attacker-controlled: its query, match info, headers, # cookies and body all come from the client. *(TypedParameter(_sym(cls), "http") for cls in _REQUEST_CLASSES), - # ``@routes.get('/')`` on a ``RouteTableDef``. - *(EntryPoint(_sym(f"aiohttp.web.RouteTableDef.{method}"), "http") for method in (*_METHODS, "route", "view")), - # ``app.router.add_route('GET', '/', handler)`` and ``app.router.add_get('/', handler)``. - *(RouteRegistrar(_sym(f"{router}.add_route"), 2, "http", keyword="handler") for router in _ROUTERS), + *(EntryPoint(_sym(table), "http") for table in _ROUTE_TABLES), + *(RouteRegistrar(_sym(call), argument, "http", keyword="handler") for call, argument in _ROUTE_CALLS), *( - RouteRegistrar(_sym(f"{router}.add_{method}"), 1, "http", keyword="handler") - for router in _ROUTERS - for method in (*_METHODS, "view") + RequestObject(_sym(symbol), _REQUEST_TEXT) + for symbol in (*_REQUEST_CLASSES, *_ROUTE_TABLES, *(call for call, _ in _ROUTE_CALLS)) ), - # ``app.add_routes([web.get('/', handler)])``: the route definitions name the handler. - RouteRegistrar(_sym("aiohttp.web.route"), 2, "http", keyword="handler"), - *(RouteRegistrar(_sym(f"aiohttp.web.{method}"), 1, "http", keyword="handler") for method in (*_METHODS, "view")), Sink(_sym("aiohttp.web.Response"), TaintKind.HTML), Sink(_sym("aiohttp.web.FileResponse"), TaintKind.PATH), *(Sink(_sym(f"aiohttp.web.{exception}"), TaintKind.REDIRECT) for exception in _REDIRECTS), diff --git a/src/coretrace_python/bundled/models/django/django_models.py b/src/coretrace_python/bundled/models/django/django_models.py index 47b3806..49ccdf4 100644 --- a/src/coretrace_python/bundled/models/django/django_models.py +++ b/src/coretrace_python/bundled/models/django/django_models.py @@ -17,6 +17,7 @@ AuthorizationGuard, EntryPoint, Model, + RequestObject, RouteRegistrar, Sanitizer, Sink, @@ -88,6 +89,23 @@ ) +# What a request object gives as text: the query string, form fields, headers, cookies +# and path. Its body, its files and REST framework's parsed ``data`` may hold a structure. +_REQUEST_TEXT = ( + "GET", "POST", "COOKIES", "META", "headers", "path", "path_info", "method", "scheme", "encoding", + "content_type", "content_params", "resolver_match", "get_full_path", "get_full_path_info", + "build_absolute_uri", "get_host", "get_port", "get_signed_cookie", "query_params", +) +# Every input a request object comes from: an annotated parameter, a class-based view's +# method or ``self.request``, a decorated view, a view registered in a URL configuration. +_REQUEST_INPUTS = ( + *_REQUEST_CLASSES, + *_VIEW_BASES, + *(f"{base}.request" for base in _VIEW_BASES), + *_VIEW_DECORATORS, + *_ROUTE_REGISTRARS, +) + _TARGET_ONLY = ((TaintKind.REDIRECT, (0,)),) @@ -103,6 +121,7 @@ class DjangoModels(ModelPlugin): # ``self.request`` in a class-based view: the attribute is inherited from the base. *(Source(_sym(f"{base}.request"), "http") for base in _VIEW_BASES), *(EntryPoint(_sym(decorator), "http") for decorator in _VIEW_DECORATORS), + *(RequestObject(_sym(symbol), _REQUEST_TEXT) for symbol in _REQUEST_INPUTS), Sink(_sym("django.db.connection.cursor.execute"), TaintKind.SQL | TaintKind.CREDENTIAL, ((TaintKind.SQL, (0,)),)), Sink(_sym("django.db.connection.cursor.executemany"), TaintKind.SQL | TaintKind.CREDENTIAL, ((TaintKind.SQL, (0,)),)), Sink(_sym("django.db.models.expressions.RawSQL"), TaintKind.SQL | TaintKind.CREDENTIAL), diff --git a/src/coretrace_python/bundled/models/tornado/tornado_models.py b/src/coretrace_python/bundled/models/tornado/tornado_models.py index 918e216..3e883c0 100644 --- a/src/coretrace_python/bundled/models/tornado/tornado_models.py +++ b/src/coretrace_python/bundled/models/tornado/tornado_models.py @@ -8,7 +8,16 @@ from coretrace_python.plugins import ModelPlugin from coretrace_python.semantic.symbols import SymbolId -from coretrace_python.taint import TEXT_KINDS, EntryPoint, Model, Sanitizer, Sink, Source, TaintKind +from coretrace_python.taint import ( + TEXT_KINDS, + EntryPoint, + Model, + RequestObject, + Sanitizer, + Sink, + Source, + TaintKind, +) _HANDLERS = ("tornado.web.RequestHandler", "tornado.websocket.WebSocketHandler") # Text, which cannot hold a query operator; the request object holds the raw body too. @@ -16,6 +25,11 @@ "get_argument", "get_arguments", "get_query_argument", "get_query_arguments", "get_body_argument", "get_body_arguments", "get_cookie", "path_args", "path_kwargs", ) +# What ``self.request`` gives as text; its body and files may hold a structure. +_REQUEST_TEXT = ( + "arguments", "query_arguments", "body_arguments", "query", "uri", "path", "headers", "cookies", + "method", "host", "host_name", "remote_ip", "protocol", "version", "full_url", +) _CLIENTS = ("tornado.httpclient.AsyncHTTPClient.fetch", "tornado.httpclient.HTTPClient.fetch") _TARGET_ONLY = ((TaintKind.REDIRECT, (0,)),) @@ -33,6 +47,7 @@ class TornadoModels(ModelPlugin): EntryPoint(_sym("tornado.websocket.WebSocketHandler"), "http"), *(Source(_sym(f"{handler}.{method}"), "http", TEXT_KINDS) for handler in _HANDLERS for method in _TEXT_METHODS), *(Source(_sym(f"{handler}.request"), "http") for handler in _HANDLERS), + *(RequestObject(_sym(f"{handler}.request"), _REQUEST_TEXT) for handler in _HANDLERS), *(Sink(_sym(f"{handler}.write"), TaintKind.HTML) for handler in _HANDLERS), *(Sink(_sym(f"{handler}.finish"), TaintKind.HTML) for handler in _HANDLERS), *(Sink(_sym(f"{handler}.redirect"), TaintKind.REDIRECT, _TARGET_ONLY) for handler in _HANDLERS), diff --git a/src/coretrace_python/taint/__init__.py b/src/coretrace_python/taint/__init__.py index 860dbbc..9219967 100644 --- a/src/coretrace_python/taint/__init__.py +++ b/src/coretrace_python/taint/__init__.py @@ -17,6 +17,7 @@ ModelError, ModelTable, NamedParameter, + RequestObject, RouteRegistrar, SafeArgument, Sanitizer, @@ -45,6 +46,7 @@ "ModelTable", "NamedParameter", "RegisteredRoutes", + "RequestObject", "RouteRegistrar", "Routes", "SafeArgument", diff --git a/src/coretrace_python/taint/engine.py b/src/coretrace_python/taint/engine.py index 22bedcf..6cf25b3 100644 --- a/src/coretrace_python/taint/engine.py +++ b/src/coretrace_python/taint/engine.py @@ -74,6 +74,7 @@ from coretrace_python.semantic.symbols import SymbolAnalysis, SymbolId, SymbolTable from coretrace_python.source import SourceSpan from coretrace_python.taint.models import ( + TEXT_KINDS, EntryPoint, ModelTable, SecurityModelAnalysis, @@ -84,6 +85,9 @@ from coretrace_python.taint.routes import RegisteredRoutes, Routes from coretrace_python.taint.templates import EscapedTemplates +# What only a structure carries, and text does not. +_STRUCTURED = TaintKind.ALL & ~TEXT_KINDS + @dataclass(frozen=True) class Taint: @@ -274,19 +278,42 @@ def instruction(self, instruction: Instruction, state: Mapping[Key, Taint]) -> T taint = Taint.none() symbol = self.symbols.get(instruction.result) if instruction.result else None if symbol is not None: - source = self.models.source_covering(symbol) - if source is not None: - taint = Taint(source.kinds, frozenset({source})) + taint = self.covered(symbol) if isinstance(instruction, Symbol | Compare): return taint for operand in instruction.operands(): - taint = taint.join(state.get(operand, Taint.none())) + operand_taint = state.get(operand, Taint.none()) + if isinstance(instruction, GetAttr): + operand_taint = self.attribute(operand_taint, instruction.attribute) + taint = taint.join(operand_taint) if isinstance(instruction, BuildList | BuildTuple | BuildDict | BuildSet): # ``[*xs]`` and ``{**d}`` copy the contents of what they unpack. for unpacked in instruction.unpacked: taint = taint.join(self.deep(unpacked, state)) return taint + def covered(self, symbol: SymbolId) -> Taint: + """The taint of the source covering ``symbol``, if any. Below a request source, as + ``self.request.GET`` is, it is the taint of the request attribute read.""" + + source = self.models.source_covering(symbol) + if source is None: + return Taint.none() + taint = Taint(source.kinds, frozenset({source})) + if symbol == source.symbol: + return taint + read = symbol.canonical_name.removeprefix(f"{source.symbol.canonical_name}.").split(".")[0] + return self.attribute(taint, read) + + def attribute(self, taint: Taint, name: str) -> Taint: + """What attribute ``name`` of a value carrying ``taint`` carries: text when every + source of the value is a request object listing ``name`` among its text attributes.""" + + requests = [self.models.request_object(source.symbol) for source in taint.sources] + if taint and all(request is not None and name in request.text for request in requests): + return taint.without(_STRUCTURED) + return taint + def call(self, call: Call, state: dict[Key, Taint], flows: list[TaintFlow]) -> Taint: arguments = tuple(self.deep(a, state) for a in call.arguments) keywords = Taint.none() @@ -420,11 +447,7 @@ def external( if cleared: return everything.without(TaintKind(cleared)) # A method on a tainted object returns tainted data (``request.args.get``). - everything = everything.join(state.get(call.callee, Taint.none())) - source = self.models.source_covering(symbol) - if source is not None: - return everything.join(Taint(source.kinds, frozenset({source}))) - return everything + return everything.join(state.get(call.callee, Taint.none())).join(self.covered(symbol)) def guarded(self, sink: Sink | None, arguments: Arguments) -> Sink | None: """The sink as this call makes it: an argument declared safe (``yaml.load`` with @@ -777,7 +800,8 @@ def parameter_sources( entry point (``self`` excepted for a method), including one registered elsewhere (``routes``), and every parameter annotated with a typed-parameter symbol. An entry point's parameter annotated with scalars is text: FastAPI validates it before the - handler runs, and a scalar cannot hold a query operator.""" + handler runs, and a scalar cannot hold a query operator. So are the URL parameters + a view receives after a request object.""" owner = _owner(module, function) sources: dict[int, Source] = {} @@ -786,10 +810,13 @@ def parameter_sources( entry = function_entry_point(function, module, models, scopes, symbols, instances, routes) if entry is not None: first = 1 if owner is not None else 0 + request = models.request_object(entry.symbol) for index in range(first, len(function.parameters)): annotation = function.parameters[index].annotation - scalar = annotation is not None and _scalar(annotation, symbols, enclosing) - kinds = entry.kinds & ~TaintKind.NOSQL if scalar else entry.kinds + text = (request is not None and index > first) or ( + annotation is not None and _scalar(annotation, symbols, enclosing) + ) + kinds = entry.kinds & ~_STRUCTURED if text else entry.kinds sources[index] = Source(entry.symbol, entry.label, kinds) for index, parameter in enumerate(function.parameters): if parameter.annotation is None: diff --git a/src/coretrace_python/taint/models.py b/src/coretrace_python/taint/models.py index afa2670..75c8035 100644 --- a/src/coretrace_python/taint/models.py +++ b/src/coretrace_python/taint/models.py @@ -205,6 +205,22 @@ class TemplateRender: keyword: str = "template_name" +@dataclass(frozen=True) +class RequestObject: + """The inputs from ``symbol`` are request objects; ``symbol`` names an entry point, a + route registrar, a typed parameter or a source. A view receives the request first + (after ``self``), then URL parameters, which are text. Reading one of the request's + ``text`` attributes (``GET``, ``headers``) gives text, while its body and files keep the + kinds of the request: text carries ``TEXT_KINDS``.""" + + symbol: SymbolId + text: tuple[str, ...] = () + + def __post_init__(self) -> None: + # Sorted: the models' repr is part of the cache key, the same in every process. + object.__setattr__(self, "text", tuple(sorted(set(self.text)))) + + @dataclass(frozen=True) class AuthorizationGuard: """A decorator, or a condition, that restricts who reaches the code behind it; a @@ -228,6 +244,7 @@ class AuthorizationGuard: | SafeArgument | TemplateRender | Members + | RequestObject ) @@ -246,6 +263,7 @@ class ModelTable: safe_arguments: tuple[SafeArgument, ...] = () template_renders: tuple[TemplateRender, ...] = () members: tuple[Members, ...] = () + request_objects: tuple[RequestObject, ...] = () _by_symbol: dict[type[Model], dict[SymbolId, Model]] = field( init=False, repr=False, compare=False ) @@ -262,6 +280,7 @@ def __post_init__(self) -> None: RouteRegistrar: {m.symbol: m for m in self.route_registrars}, SafeArgument: {m.symbol: m for m in self.safe_arguments}, TemplateRender: {m.symbol: m for m in self.template_renders}, + RequestObject: {m.symbol: m for m in self.request_objects}, } object.__setattr__(self, "_by_symbol", MappingProxyType(index)) @@ -330,6 +349,7 @@ def extended(self, *sinks: Sink) -> ModelTable: self.safe_arguments, self.template_renders, self.members, + self.request_objects, ) def sanitizer(self, symbol: SymbolId) -> Sanitizer | None: @@ -344,6 +364,10 @@ def template_render(self, symbol: SymbolId) -> TemplateRender | None: found = self._by_symbol[TemplateRender].get(symbol) return found if isinstance(found, TemplateRender) else None + def request_object(self, symbol: SymbolId) -> RequestObject | None: + found = self._by_symbol[RequestObject].get(symbol) + return found if isinstance(found, RequestObject) else None + def members_by_class(self) -> Mapping[SymbolId, Members]: """The ``Members`` models by class, as symbol resolution reads them.""" @@ -408,6 +432,7 @@ def freeze(self) -> ModelTable: safe_arguments=tuple(m for m in models if isinstance(m, SafeArgument)), template_renders=tuple(m for m in models if isinstance(m, TemplateRender)), members=tuple(m for m in models if isinstance(m, Members)), + request_objects=tuple(m for m in models if isinstance(m, RequestObject)), )