From 60e381c621a5a4155c2692e063313d4dfce15836 Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 26 Sep 2026 00:06:04 +0200 Subject: [PATCH 1/6] test: expect module-level names bound through an instance to resolve --- tests/test_frameworks.py | 49 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/tests/test_frameworks.py b/tests/test_frameworks.py index a1afdac..019fa4d 100644 --- a/tests/test_frameworks.py +++ b/tests/test_frameworks.py @@ -3,8 +3,9 @@ Framework plugins enrich the security model; detectors stay generic. Three engine mechanisms make that possible: -- a module-level ``name = Symbol(...)`` binds ``name`` to that symbol, so ``app.route`` - resolves to ``python.flask.Flask.route`` when ``app = Flask(__name__)``; +- a module-level name denotes what its value denotes in a function body, so ``app.route`` + resolves to ``python.flask.Flask.route`` when ``app = Flask(__name__)``, and + ``cursor = conn.cursor()`` goes through the ``conn`` bound before it; - inside functions, call results and ``with`` contexts carry their callee's symbol, so ``conn.cursor().execute`` resolves to ``python.sqlite3.connect.cursor.execute``; - an ``EntryPoint`` model taints every parameter of a function decorated by its symbol, @@ -89,6 +90,50 @@ def test_module_level_instances_resolve_to_their_class_symbol() -> None: assert symbols.resolve(f.id, "limit") is None +def test_module_level_names_bound_through_an_instance_denote_what_a_function_derives() -> None: + manager = manager_for( + "import os\nimport sqlite3\nfrom django.db import connections\n\n" + "early = conn.cursor()\n" + "conn = sqlite3.connect('app.db')\n" + "cursor = conn.cursor()\n" + "same = conn\n" + "run = os.system\n" + "home = os.environ['HOME']\n" + "raw = connections['default'].cursor()\n" + "limit = 3\n\n" + "def f():\n return cursor\n" + ) + symbols = manager.get(SymbolAnalysis) + scopes = manager.get(ScopeAnalysis) + f = next(s for s in scopes.children(scopes.module_scope.id) if s.name == "f") + + assert {name: symbols.resolve(f.id, name) for name in ("cursor", "same", "run", "home", "raw")} == { + "cursor": SymbolId("python.sqlite3.connect.cursor"), + "same": SymbolId("python.sqlite3.connect"), + "run": SymbolId("python.os.system"), + # An item carries its container's symbol, as in a function body. + "home": SymbolId("python.os.environ"), + "raw": SymbolId("python.django.db.connections.cursor"), + } + # Names resolve in statement order: ``conn`` is not bound yet when ``early`` is. + assert symbols.resolve(f.id, "early") is None + assert symbols.resolve(f.id, "limit") is None + + +def test_a_query_through_a_module_level_cursor_is_an_injection() -> None: + findings = check( + "import sqlite3\nfrom flask import Flask, request\n\n" + "app = Flask(__name__)\n" + "conn = sqlite3.connect('app.db')\n" + "cursor = conn.cursor()\n\n" + "@app.route('/users')\n" + "def users():\n" + " return str(cursor.execute(\"SELECT * FROM users WHERE name = '\" + request.args['q'] + \"'\").fetchall())\n" + ) + + assert [(f.rule_id, f.span.start_line) for f in findings if f.rule_id == "sql-injection"] == [("sql-injection", 10)] + + def test_attributes_of_module_level_instances_lower_to_symbols() -> None: module = module_for("from flask import Flask\napp = Flask(__name__)\n\ndef f():\n return app.route\n") output = format_module(lower_module(module)) From 66802d1ffc0653a43598a2dac3f52761cf5aeace Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 26 Sep 2026 00:06:04 +0200 Subject: [PATCH 2/6] fix: resolve module-level names bound through another instance A module-level name denotes what its value denotes in a function body: an alias, an attribute, an item or a call of an import or of an earlier module-level name, resolved in statement order. --- CHANGELOG.md | 6 +++++ src/coretrace_python/semantic/symbols.py | 30 +++++++++++++++++------- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bccc191..722ab03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Detection + +- A module-level name bound to an attribute, an item or a call of another module-level name resolves in functions, as the same value does in a function body. `cursor = conn.cursor()` after `conn = sqlite3.connect(...)`, `db = client.shop` and `run = os.system` used to leave the calls made through them unknown, so a query built from request data and executed through a module-level cursor was not reported. Names resolve in statement order. The regression corpus is unchanged (#155). + ## 0.11.0 (2026-09-25) ### Detection diff --git a/src/coretrace_python/semantic/symbols.py b/src/coretrace_python/semantic/symbols.py index 20d3de4..ed065bf 100644 --- a/src/coretrace_python/semantic/symbols.py +++ b/src/coretrace_python/semantic/symbols.py @@ -18,7 +18,8 @@ class SymbolTable: """Resolve names to canonical symbols through scopes, imports, builtins and the - module-level instances created by calling a symbol (``app = Flask(__name__)``).""" + module-level names bound to a value denoting a symbol (``app = Flask(__name__)``, + ``cursor = conn.cursor()``).""" def __init__( self, @@ -64,15 +65,28 @@ def analyze_symbols( instances: dict[str, SymbolId] = {} module_scope = scopes.module_scope.id for statement in module.body: - if ( - isinstance(statement, nodes.Assign) - and isinstance(statement.target, nodes.Name) - and isinstance(statement.value, nodes.Call) - ): - symbol = table.resolve_expression(module_scope, statement.value.callee) + if isinstance(statement, nodes.Assign) and isinstance(statement.target, nodes.Name): + symbol = _denoted(table, module_scope, statement.value) if symbol is not None: instances[statement.target.identifier] = symbol - return SymbolTable(scopes, imports, instances) + # Later bindings resolve through this one, in statement order. + table = SymbolTable(scopes, imports, instances) + return table + + +def _denoted(table: SymbolTable, scope_id: ScopeId, node: nodes.Expression) -> SymbolId | None: + """What a value denotes, as a function body derives it (``derive_symbols``): a + resolved name or an attribute of one, the result of calling one, or an item of one, + which carries its container's symbol (``os.environ['HOME']``).""" + + if isinstance(node, nodes.Subscript): + return _denoted(table, scope_id, node.value) + if isinstance(node, nodes.Attribute): + parent = _denoted(table, scope_id, node.value) + return parent.attribute(node.name) if parent is not None else None + if isinstance(node, nodes.Call): + return _denoted(table, scope_id, node.callee) + return table.resolve_expression(scope_id, node) class SymbolAnalysis(Analysis[SymbolTable]): From 4bf453ee4bcb2fc92ffa47276ca771f4cce0f0e6 Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 26 Sep 2026 00:21:20 +0200 Subject: [PATCH 3/6] test: expect a Members model to type what an object gives by name --- tests/test_members.py | 206 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 tests/test_members.py diff --git a/tests/test_members.py b/tests/test_members.py new file mode 100644 index 0000000..444cc37 --- /dev/null +++ b/tests/test_members.py @@ -0,0 +1,206 @@ +"""A ``Members`` model says what the attributes and items of a class's instances give. + +Some libraries give objects by names the project chooses: a pymongo client gives the +database it is dotted or indexed with, a database gives a collection. Symbols follow +attribute paths, so those names used to end up in the symbol (``Client.shop.users.find``) +or, for an item, to vanish into the container's symbol (``Client.find``), and no model +could list them. A ``Members`` model describes the instances of a class: + +- ``typed`` maps an attribute to the class of what it gives, called or not + (``get_database``, or a ``db`` property); +- ``dynamic``, when set, is the class of every item and of every attribute outside + ``defined`` and ``typed``. + +Every path to a collection then denotes the collection class, the one an annotated +parameter denotes, and a single sink on ``Collection.find`` covers them all. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.analysis import AnalysisManager +from coretrace_python.frontend import build_hir +from coretrace_python.hir import nodes +from coretrace_python.interprocedural import CallGraphAnalysis, ExternalSymbol +from coretrace_python.semantic.symbols import SymbolId +from coretrace_python.source import SourceManager +from coretrace_python.taint import ( + Members, + SecurityModelRegistry, + Sink, + Source, + TaintAnalysis, + TaintKind, +) + +CLIENT = SymbolId("python.docstore.Client") +DATABASE = SymbolId("python.docstore.database.Database") +COLLECTION = SymbolId("python.docstore.collection.Collection") +FIND = COLLECTION.attribute("find") +MEMBERS = ( + Members(CLIENT, DATABASE, defined=("close", "list_database_names"), typed=(("get_database", DATABASE),)), + Members( + DATABASE, + COLLECTION, + defined=("command", "list_collection_names"), + typed=(("get_collection", COLLECTION), ("client", CLIENT)), + ), + # A collection gives its sub-collections by name. + Members(COLLECTION, COLLECTION, defined=("delete_many", "find", "find_one", "insert_one", "name")), + # Attributes typed without dynamic members, like Flask-PyMongo's ``db``. + Members(SymbolId("python.docstore.flask.Store"), typed=(("db", DATABASE),)), +) + + +def manager_for(source_text: str, *models: object) -> AnalysisManager: + registry = SecurityModelRegistry() + registry.register(*MEMBERS, *models) # type: ignore[arg-type] + return engine.build_manager(build_hir(SourceManager().add_source("app.py", source_text)), registry) + + +def targets(source_text: str, function: str) -> list[SymbolId | None]: + graph = manager_for(source_text).get(CallGraphAnalysis) + return [s.target.symbol if isinstance(s.target, ExternalSymbol) else None for s in graph.sites(function)] + + +APP = ( + "import docstore\nfrom docstore.flask import Store\n\n" + "def f(q):\n" + " client = docstore.Client()\n" + " store = Store()\n" + " return {call}\n" +) + + +@pytest.mark.parametrize( + "call, symbol", + [ + ("client.shop.users.find(q)", FIND), + ("client['shop']['users'].find(q)", FIND), + ("client.shop['users'].find(q)", FIND), + ("client['shop'].users.find(q)", FIND), + ("client.get_database('shop').get_collection('users').find(q)", FIND), + ("client.shop.users.archive.find(q)", FIND), + ("store.db.users.find(q)", FIND), + ("client.shop.command(q)", DATABASE.attribute("command")), + ("client.shop.client.close()", CLIENT.attribute("close")), + ("client.close()", CLIENT.attribute("close")), + # What a member method returns is no member: a document's ``str.find``. + ("client.shop.users.find_one(q)['name'].find(q)", COLLECTION.attribute("find_one").attribute("find")), + ], +) +def test_attributes_and_items_give_the_class_the_model_names(call: str, symbol: SymbolId) -> None: + assert targets(APP.format(call=call), "f")[-1] == symbol + + +def test_module_level_names_give_the_same_classes() -> None: + source = ( + "import docstore\n\n" + "client = docstore.Client()\n" + "db = client.shop\n" + "users = db['users']\n" + "orders = client.get_database('shop').orders\n\n" + "def f(q):\n" + " users.find(q)\n" + " orders.find_one(q)\n" + " db.users.delete_many(q)\n" + ) + + assert targets(source, "f") == [FIND, COLLECTION.attribute("find_one"), COLLECTION.attribute("delete_many")] + + +def test_annotated_parameters_and_inherited_attributes_give_the_same_classes() -> None: + source = ( + "import docstore\nfrom docstore.database import Database\n\n" + "def f(db: Database, q):\n" + " return db.users.find(q)\n\n" + "class Shop(docstore.Client):\n" + " def orders(self, q):\n" + " return self.shop.orders.find(q)\n" + ) + + assert targets(source, "f") == [FIND] + assert targets(source, "Shop.orders") == [FIND] + + +def test_one_sink_on_the_member_class_covers_every_path() -> None: + source = ( + "import docstore\n\n" + "client = docstore.Client()\n" + "db = client['shop']\n\n" + "def by_attribute():\n client.shop.users.find(input())\n\n" + "def by_item():\n db['users'].find(input())\n\n" + "def by_getter():\n client.get_database('shop').get_collection('users').find(input())\n" + ) + manager = manager_for( + source, Source(SymbolId("python.builtins.input"), "stdin"), Sink(FIND, TaintKind.NOSQL) + ) + + sinks = [ + (function.name, str(flow.sink.symbol)) + for function in manager.module.body + if isinstance(function, nodes.Function) + for flow in manager.get(TaintAnalysis, function).flows + ] + assert sinks == [("by_attribute", str(FIND)), ("by_item", str(FIND)), ("by_getter", str(FIND))] + + +def test_defined_names_are_kept_sorted_so_the_models_repr_is_stable() -> None: + members = Members(CLIENT, DATABASE, defined=frozenset({"list_database_names", "close"})) # type: ignore[arg-type] + + assert members.defined == ("close", "list_database_names") + assert members == Members(CLIENT, DATABASE, defined=("list_database_names", "close")) + + +PLUGIN_MANIFEST = ( + 'name = "docstore-models"\nversion = "1.0.0"\nplugin_api = ">=1,<2"\nrequires = []\n' + 'provides = ["model.docstore"]\n\n[entrypoint]\nmodule = "docstore_models"\nclass = "DocstoreModels"\n' +) +PLUGIN = ''' +from typing import ClassVar + +from coretrace_python.plugins import ModelPlugin +from coretrace_python.semantic.symbols import SymbolId +from coretrace_python.taint import Members, Model, Sink, TaintKind + +CLIENT = SymbolId("python.docstore.Client") +DATABASE = SymbolId("python.docstore.database.Database") +COLLECTION = SymbolId("python.docstore.collection.Collection") + + +class DocstoreModels(ModelPlugin): + name: ClassVar[str] = "docstore-models" + models: ClassVar[tuple[Model, ...]] = ( + Members(CLIENT, DATABASE), + Members(DATABASE, COLLECTION, defined=("command",)), + Members(COLLECTION, COLLECTION, defined=("find",)), + # Any rule shows the flow reaching the sink; SQL has one. + Sink(COLLECTION.attribute("find"), TaintKind.SQL), + ) +''' + + +@pytest.mark.parametrize("jobs", [1, 2]) +def test_a_plugin_models_members_in_a_project_analysis(tmp_path: Path, jobs: int) -> None: + plugin = tmp_path / "plugins" / "docstore_models" + plugin.mkdir(parents=True) + (plugin / "plugin.toml").write_text(PLUGIN_MANIFEST, encoding="utf-8") + (plugin / "docstore_models.py").write_text(PLUGIN, encoding="utf-8") + root = tmp_path / "project" + root.mkdir() + for name in ("app.py", "admin.py"): + (root / name).write_text( + "import docstore\n\ndb = docstore.Client()['shop']\n\ndef run():\n return db.users.find(input())\n", + encoding="utf-8", + ) + + analysis = engine.analyze_project(root, [engine.BUNDLED_PLUGINS, tmp_path / "plugins"], jobs=jobs) + + found = sorted( + (Path(str(f.span.source_id)).name, f.span.start_line) for f in analysis.findings if f.rule_id == "sql-injection" + ) + assert found == [("admin.py", 6), ("app.py", 6)] From 93d3cf66d847487dc710735143ef91e3167bd01c Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 26 Sep 2026 00:21:20 +0200 Subject: [PATCH 4/6] feat: type what an object gives by attribute or item with a Members model Symbol resolution reads the Members models the engine provides as the semantic.members input, so SymbolAnalysis now depends on MembersAnalysis besides scopes and imports. --- CHANGELOG.md | 4 + docs/plugins.md | 1 + src/coretrace_python/engine.py | 8 +- .../interprocedural/callgraph.py | 26 ++--- src/coretrace_python/ir/lowering.py | 2 +- src/coretrace_python/semantic/__init__.py | 6 +- src/coretrace_python/semantic/symbols.py | 102 +++++++++++++++--- src/coretrace_python/taint/__init__.py | 2 + src/coretrace_python/taint/models.py | 12 ++- tests/test_analysis_manager.py | 5 +- 10 files changed, 135 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 722ab03..c29aa5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - A module-level name bound to an attribute, an item or a call of another module-level name resolves in functions, as the same value does in a function body. `cursor = conn.cursor()` after `conn = sqlite3.connect(...)`, `db = client.shop` and `run = os.system` used to leave the calls made through them unknown, so a query built from request data and executed through a module-level cursor was not reported. Names resolve in statement order. The regression corpus is unchanged (#155). +### Plugins + +- A `Members(symbol, dynamic=None, defined=(), typed=())` model says what the attributes and items of a class's instances give, for libraries whose objects give others by names the project chooses. A pymongo client gives the database it is dotted or indexed with, a database gives a collection. The names used to end up in the symbol (`MongoClient.shop.users.find`), or an item vanished into its container's symbol (`MongoClient.find`), so no model could list them. With the model, every path to a collection denotes the collection class, the one a parameter annotated `Collection` denotes: `client.shop.users`, `client["shop"]["users"]`, `client.get_database("shop").get_collection("users")`, a module-level `db = client.shop`, an attribute inherited from the client class. A sink on `Collection.find` covers them all. Symbol resolution now depends on these models, which the engine provides to every module and worker (#157). + ## 0.11.0 (2026-09-25) ### Detection diff --git a/docs/plugins.md b/docs/plugins.md index eac5fa4..d8d0d78 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -152,6 +152,7 @@ class BottleModels(ModelPlugin): | `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. | | `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. | Two plugins may describe the same symbol. An identical model is registered once; two `Sink` models merge their kinds, positions and keywords, so a plugin can add `SQL` to a sink diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index 49066da..416d881 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -109,7 +109,7 @@ from coretrace_python.semantic import SEMANTIC_ANALYSES from coretrace_python.semantic.imports import ImportAnalysis, ImportResolutionError, ImportTable from coretrace_python.semantic.scopes import ScopeAnalysis, ScopeError -from coretrace_python.semantic.symbols import SymbolAnalysis, SymbolId +from coretrace_python.semantic.symbols import MembersAnalysis, SymbolAnalysis, SymbolId from coretrace_python.source import SourceFile, SourceId, SourceManager, SourceSpan, decode_text from coretrace_python.taint import ( EntryPoint, @@ -257,6 +257,7 @@ def build_manager( manager = _register_all(module) table = (models or SecurityModelRegistry()).freeze() manager.provide(SecurityModelAnalysis, table) + manager.provide(MembersAnalysis, table.members_by_class()) manager.provide(ClearingAnalysis, table.clearing()) manager.provide(ProjectSummaries, SummaryIndex()) manager.provide(RegisteredRoutes, _routes_of(manager)) @@ -311,6 +312,7 @@ def analyze_file(source: SourceFile, plugin_roots: Sequence[Path]) -> FileAnalys registry = load_plugins(plugin_roots, manager) table = plugin_models(loaded.plugin for loaded in registry) manager.provide(SecurityModelAnalysis, table) + manager.provide(MembersAnalysis, table.members_by_class()) manager.provide(ClearingAnalysis, table.clearing()) manager.provide(ProjectSummaries, SummaryIndex()) manager.provide(RegisteredRoutes, _routes_of(manager)) @@ -402,8 +404,10 @@ def analyze_project( advisories, origins = _merge_advisories(_contributions(registry, plugins, root, file_advisories)) affected = affected_symbols(dependencies, advisories) models = plugin_models(all_plugins, root).extended(*advisory_sinks(affected)) + members = models.members_by_class() for manager in managers.values(): manager.provide(SecurityModelAnalysis, models) + manager.provide(MembersAnalysis, members) manager.provide(DependencyAnalysis, dependencies) imports: dict[str, ImportTable] = {} @@ -698,8 +702,10 @@ def _analyse_batch(batch: _Batch) -> dict[str, dict[str, Any]]: models = plugin_models(all_plugins, batch.root).extended(*advisory_sinks(affected)) routes = _decode_routes(batch.routes) escaped = frozenset(batch.escaped) + members = models.members_by_class() for manager in managers.values(): manager.provide(SecurityModelAnalysis, models) + manager.provide(MembersAnalysis, members) manager.provide(DependencyAnalysis, dependencies) manager.provide(RegisteredRoutes, routes) manager.provide(EscapedTemplates, escaped) diff --git a/src/coretrace_python/interprocedural/callgraph.py b/src/coretrace_python/interprocedural/callgraph.py index c93e810..de41d84 100644 --- a/src/coretrace_python/interprocedural/callgraph.py +++ b/src/coretrace_python/interprocedural/callgraph.py @@ -176,12 +176,12 @@ def callers(self, name: str) -> frozenset[str]: def derive_symbols( - function: FunctionIR, initial: Mapping[Value, SymbolId] | None = None + function: FunctionIR, table: SymbolTable, initial: Mapping[Value, SymbolId] | None = None ) -> dict[Value, SymbolId]: - """Symbols of values: ``Symbol`` results, attributes and items of symbol values, - results of calling a symbol (``sqlite3.connect(p)`` denotes ``python.sqlite3.connect``) - and the values a ``with`` on such a result binds. Known functions derive nothing; - parameters only through ``initial``, their annotated classes.""" + """Symbols of values: ``Symbol`` results, attributes and items of symbol values, as + ``table`` types them, results of calling a symbol (``sqlite3.connect(p)`` denotes + ``python.sqlite3.connect``) and the values a ``with`` on such a result binds. Known + functions derive nothing; parameters only through ``initial``, their annotated classes.""" symbols: dict[Value, SymbolId] = dict(initial or {}) changed = True @@ -195,11 +195,9 @@ def derive_symbols( if isinstance(instruction, Symbol): symbol = instruction.symbol_id elif isinstance(instruction, GetAttr) and instruction.object in symbols: - symbol = symbols[instruction.object].attribute(instruction.attribute) + symbol = table.attribute(symbols[instruction.object], instruction.attribute) elif isinstance(instruction, GetItem) and instruction.object in symbols: - # An item of a symbol-denoted container (``request.files['f']``) - # carries the container's symbol, so its methods resolve. - symbol = symbols[instruction.object] + symbol = table.item(symbols[instruction.object]) elif isinstance(instruction, Call | WithEnter): origin = ( instruction.callee if isinstance(instruction, Call) else instruction.context @@ -263,6 +261,8 @@ def resolve_targets( owner: str | None = None, typed: Mapping[Value, str] | None = None, bases: Mapping[str, SymbolId] | None = None, + *, + table: SymbolTable, ) -> tuple[dict[Value, Target], dict[Value, SymbolId]]: """Map every callee value of ``function`` to its target, and every symbol value. ``nested`` names the functions defined inside this one (``outer.inner``); @@ -271,7 +271,8 @@ def resolve_targets( call to ``App.__init__``, ``app.run()`` and ``self.run()`` calls to ``App.run``. ``bases`` maps a module class to the symbol of its base: an attribute the class does not define is inherited, so ``self.get_argument`` in a ``RequestHandler`` subclass - denotes ``tornado.web.RequestHandler.get_argument``.""" + denotes ``tornado.web.RequestHandler.get_argument``. ``table`` says what the + attributes and items of a class a ``Members`` model describes give.""" module = scopes.module_scope classes = classes or {} @@ -294,8 +295,8 @@ def resolve_targets( class_name = instance_of[instruction.object] base = (bases or {}).get(class_name) if base is not None and instruction.attribute not in classes.get(class_name, ()): - inherited[instruction.result] = base.attribute(instruction.attribute) - symbols = derive_symbols(function, inherited) + inherited[instruction.result] = table.attribute(base, instruction.attribute) + symbols = derive_symbols(function, table, inherited) targets: dict[Value, Target] = { value: ExternalSymbol(symbol) for value, symbol in symbols.items() } @@ -427,6 +428,7 @@ def compute(cls, ctx: AnalysisContext) -> CallGraph: owner, _typed_with_module_classes(function, ssa, classes), bases, + table=table, ) defs = {i.result: i for block in ssa.blocks for i in block.instructions if i.result is not None} found: list[CallSite] = [] diff --git a/src/coretrace_python/ir/lowering.py b/src/coretrace_python/ir/lowering.py index 3899fa9..0d6dde0 100644 --- a/src/coretrace_python/ir/lowering.py +++ b/src/coretrace_python/ir/lowering.py @@ -141,7 +141,7 @@ def imported_symbol(self, node: nodes.Expression) -> SymbolId | None: return self.symbols.resolve(self.scope.id, node.identifier) if isinstance(node, nodes.Attribute): parent = self.imported_symbol(node.value) - return parent.attribute(node.name) if parent is not None else None + return self.symbols.attribute(parent, node.name) if parent is not None else None return None def expression(self, node: nodes.Expression) -> Value: diff --git a/src/coretrace_python/semantic/__init__.py b/src/coretrace_python/semantic/__init__.py index 3806215..c651be2 100644 --- a/src/coretrace_python/semantic/__init__.py +++ b/src/coretrace_python/semantic/__init__.py @@ -2,8 +2,8 @@ from coretrace_python.semantic.imports import ImportAnalysis from coretrace_python.semantic.scopes import ScopeAnalysis -from coretrace_python.semantic.symbols import SymbolAnalysis +from coretrace_python.semantic.symbols import MembersAnalysis, SymbolAnalysis -SEMANTIC_ANALYSES = (ScopeAnalysis, ImportAnalysis, SymbolAnalysis) +SEMANTIC_ANALYSES = (ScopeAnalysis, ImportAnalysis, MembersAnalysis, SymbolAnalysis) -__all__ = ["SEMANTIC_ANALYSES", "ImportAnalysis", "ScopeAnalysis", "SymbolAnalysis"] +__all__ = ["SEMANTIC_ANALYSES", "ImportAnalysis", "MembersAnalysis", "ScopeAnalysis", "SymbolAnalysis"] diff --git a/src/coretrace_python/semantic/symbols.py b/src/coretrace_python/semantic/symbols.py index ed065bf..3eec67b 100644 --- a/src/coretrace_python/semantic/symbols.py +++ b/src/coretrace_python/semantic/symbols.py @@ -4,6 +4,7 @@ import builtins from collections.abc import Mapping +from dataclasses import dataclass from types import MappingProxyType from typing import ClassVar @@ -16,20 +17,69 @@ BUILTIN_NAMES = frozenset(name for name in dir(builtins) if not name.startswith("_")) +@dataclass(frozen=True) +class Members: + """What the attributes and items of an instance of ``symbol`` give, for a class whose + instances give other objects by name, as a pymongo client gives the database it is + dotted or indexed with (``client.shop``, ``client['shop']``). + + ``typed`` maps an attribute to the class of what it gives, called or not, since a + call denotes its callee: ``get_database`` gives a database, as a ``db`` property does. + ``dynamic``, when set, is the class of every item and of every attribute outside + ``defined`` and ``typed``. Otherwise an attribute keeps its path and an item its + container's symbol, as for any other class.""" + + symbol: SymbolId + dynamic: SymbolId | None = None + defined: tuple[str, ...] = () + typed: tuple[tuple[str, SymbolId], ...] = () + + def __post_init__(self) -> None: + # Sorted: the models' repr is part of the cache key, the same in every process. + object.__setattr__(self, "defined", tuple(sorted(set(self.defined)))) + + def attribute(self, name: str) -> SymbolId: + for attribute, gives in self.typed: + if attribute == name: + return gives + if self.dynamic is not None and name not in self.defined: + return self.dynamic + return self.symbol.attribute(name) + + def item(self) -> SymbolId: + return self.symbol if self.dynamic is None else self.dynamic + + class SymbolTable: """Resolve names to canonical symbols through scopes, imports, builtins and the module-level names bound to a value denoting a symbol (``app = Flask(__name__)``, - ``cursor = conn.cursor()``).""" + ``cursor = conn.cursor()``). ``members`` holds the ``Members`` models by class.""" def __init__( self, scopes: ScopeTable, imports: ImportTable, instances: Mapping[str, SymbolId] | None = None, + members: Mapping[SymbolId, Members] | None = None, ) -> None: self._scopes = scopes self._imports = imports self._instances = MappingProxyType(dict(instances or {})) + self._members = MappingProxyType(dict(members or {})) + + def attribute(self, symbol: SymbolId, name: str) -> SymbolId: + """The symbol of attribute ``name`` of what ``symbol`` denotes: its path, unless + a ``Members`` model describes the class.""" + + members = self._members.get(symbol) + return symbol.attribute(name) if members is None else members.attribute(name) + + def item(self, symbol: SymbolId) -> SymbolId: + """The symbol of an item of what ``symbol`` denotes: the container's own, so its + methods resolve (``request.files['f'].save``), unless a ``Members`` model says.""" + + members = self._members.get(symbol) + return symbol if members is None else members.item() def resolve(self, scope_id: ScopeId, name: str) -> SymbolId | None: resolution = self._scopes.resolve(scope_id, name) @@ -50,16 +100,19 @@ def resolve_expression(self, scope_id: ScopeId, node: nodes.Expression) -> Symbo return self.resolve(scope_id, node.identifier) if isinstance(node, nodes.Attribute): parent = self.resolve_expression(scope_id, node.value) - return parent.attribute(node.name) if parent is not None else None + return self.attribute(parent, node.name) if parent is not None else None if isinstance(node, nodes.Call): return self.resolve_expression(scope_id, node.callee) return None def analyze_symbols( - scopes: ScopeTable, imports: ImportTable, module: nodes.Module | None = None + scopes: ScopeTable, + imports: ImportTable, + module: nodes.Module | None = None, + members: Mapping[SymbolId, Members] | None = None, ) -> SymbolTable: - table = SymbolTable(scopes, imports) + table = SymbolTable(scopes, imports, members=members) if module is None: return table instances: dict[str, SymbolId] = {} @@ -70,32 +123,55 @@ def analyze_symbols( if symbol is not None: instances[statement.target.identifier] = symbol # Later bindings resolve through this one, in statement order. - table = SymbolTable(scopes, imports, instances) + table = SymbolTable(scopes, imports, instances, members) return table def _denoted(table: SymbolTable, scope_id: ScopeId, node: nodes.Expression) -> SymbolId | None: """What a value denotes, as a function body derives it (``derive_symbols``): a resolved name or an attribute of one, the result of calling one, or an item of one, - which carries its container's symbol (``os.environ['HOME']``).""" + which carries its container's symbol (``os.environ['HOME']``) unless a ``Members`` + model says what its items are.""" if isinstance(node, nodes.Subscript): - return _denoted(table, scope_id, node.value) + container = _denoted(table, scope_id, node.value) + return table.item(container) if container is not None else None if isinstance(node, nodes.Attribute): parent = _denoted(table, scope_id, node.value) - return parent.attribute(node.name) if parent is not None else None + return table.attribute(parent, node.name) if parent is not None else None if isinstance(node, nodes.Call): return _denoted(table, scope_id, node.callee) return table.resolve_expression(scope_id, node) +class MembersAnalysis(Analysis[Mapping[SymbolId, Members]]): + """The ``Members`` models by class, provided by the engine from the security models; + none on its own, so every attribute keeps its path.""" + + name: ClassVar[str] = "semantic.members" + + @classmethod + def compute(cls, ctx: AnalysisContext) -> Mapping[SymbolId, Members]: + return MappingProxyType({}) + + class SymbolAnalysis(Analysis[SymbolTable]): name: ClassVar[str] = "semantic.symbols" - requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({ScopeAnalysis, ImportAnalysis}) + requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({ScopeAnalysis, ImportAnalysis, MembersAnalysis}) @classmethod def compute(cls, ctx: AnalysisContext) -> SymbolTable: - return analyze_symbols(ctx.get(ScopeAnalysis), ctx.get(ImportAnalysis), ctx.module) - - -__all__ = ["BUILTIN_NAMES", "SymbolAnalysis", "SymbolId", "SymbolTable", "analyze_symbols"] + return analyze_symbols( + ctx.get(ScopeAnalysis), ctx.get(ImportAnalysis), ctx.module, ctx.get(MembersAnalysis) + ) + + +__all__ = [ + "BUILTIN_NAMES", + "Members", + "MembersAnalysis", + "SymbolAnalysis", + "SymbolId", + "SymbolTable", + "analyze_symbols", +] diff --git a/src/coretrace_python/taint/__init__.py b/src/coretrace_python/taint/__init__.py index c97f8ff..9a0329f 100644 --- a/src/coretrace_python/taint/__init__.py +++ b/src/coretrace_python/taint/__init__.py @@ -1,5 +1,6 @@ """Security models and the global multi-kind taint engine (architecture §16, §17).""" +from coretrace_python.semantic.symbols import Members from coretrace_python.taint.engine import ( EntryPointAnalysis, Taint, @@ -36,6 +37,7 @@ "EntryPoint", "EntryPointAnalysis", "EscapedTemplates", + "Members", "Model", "ModelError", "ModelTable", diff --git a/src/coretrace_python/taint/models.py b/src/coretrace_python/taint/models.py index 77c9a6a..7534086 100644 --- a/src/coretrace_python/taint/models.py +++ b/src/coretrace_python/taint/models.py @@ -10,6 +10,7 @@ import hashlib import re +from collections.abc import Mapping from dataclasses import dataclass, field from enum import Flag, auto from types import MappingProxyType @@ -17,7 +18,7 @@ from coretrace_python.analysis import Analysis, AnalysisContext, MissingInputError from coretrace_python.interprocedural import Clearing -from coretrace_python.semantic.symbols import SymbolId +from coretrace_python.semantic.symbols import Members, SymbolId class TaintKind(Flag): @@ -221,6 +222,7 @@ class AuthorizationGuard: | SuffixSink | SafeArgument | TemplateRender + | Members ) @@ -238,6 +240,7 @@ class ModelTable: suffix_sinks: tuple[SuffixSink, ...] = () safe_arguments: tuple[SafeArgument, ...] = () template_renders: tuple[TemplateRender, ...] = () + members: tuple[Members, ...] = () _by_symbol: dict[type[Model], dict[SymbolId, Model]] = field( init=False, repr=False, compare=False ) @@ -321,6 +324,7 @@ def extended(self, *sinks: Sink) -> ModelTable: self.suffix_sinks, self.safe_arguments, self.template_renders, + self.members, ) def sanitizer(self, symbol: SymbolId) -> Sanitizer | None: @@ -335,6 +339,11 @@ def template_render(self, symbol: SymbolId) -> TemplateRender | None: found = self._by_symbol[TemplateRender].get(symbol) return found if isinstance(found, TemplateRender) else None + def members_by_class(self) -> Mapping[SymbolId, Members]: + """The ``Members`` models by class, as symbol resolution reads them.""" + + return MappingProxyType({m.symbol: m for m in self.members}) + def clearing(self, escaped: frozenset[str] = frozenset()) -> Clearing: """What calls clear from the data they return: every sanitizer its kinds, every template render ``HTML`` when it names one of the ``escaped`` templates.""" @@ -393,6 +402,7 @@ def freeze(self) -> ModelTable: suffix_sinks=tuple(m for m in models if isinstance(m, SuffixSink)), 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)), ) diff --git a/tests/test_analysis_manager.py b/tests/test_analysis_manager.py index 2366df8..e16705e 100644 --- a/tests/test_analysis_manager.py +++ b/tests/test_analysis_manager.py @@ -45,7 +45,7 @@ from coretrace_python.semantic import SEMANTIC_ANALYSES from coretrace_python.semantic.imports import ImportAnalysis from coretrace_python.semantic.scopes import ScopeAnalysis - from coretrace_python.semantic.symbols import SymbolAnalysis + from coretrace_python.semantic.symbols import MembersAnalysis, SymbolAnalysis except ImportError as error: # pragma: no cover - red until the analysis manager lands MISSING = error else: @@ -225,7 +225,8 @@ def compute(cls, ctx: AnalysisContext) -> int: def test_transitive_dependencies_are_exposed() -> None: engine = manager(*SEMANTIC_ANALYSES) - assert engine.dependencies(SymbolAnalysis) == frozenset({ScopeAnalysis, ImportAnalysis}) + # Symbol resolution also reads the Members models the engine provides. + assert engine.dependencies(SymbolAnalysis) == frozenset({ScopeAnalysis, ImportAnalysis, MembersAnalysis}) assert engine.dependencies(ScopeAnalysis) == frozenset() From 079f901edd3de413ea3ba9e01f14d9b2b39ecfeb Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 26 Sep 2026 01:23:00 +0200 Subject: [PATCH 5/6] test: specify the nosql-injection rule for structured attacker input Text sources (form fields, headers, URL parameters, argv, the environment, input()) no longer carry NOSQL, so the operator source kinds are now expressed with TEXT_KINDS. --- tests/test_detectors.py | 3 +- tests/test_django_http_clients.py | 1 + tests/test_docs.py | 2 +- tests/test_nosql_injection.py | 206 ++++++++++++++++++++++++++++++ tests/test_operator_sources.py | 10 +- tests/test_packaging.py | 4 +- 6 files changed, 218 insertions(+), 8 deletions(-) create mode 100644 tests/test_nosql_injection.py diff --git a/tests/test_detectors.py b/tests/test_detectors.py index dc370ee..74b0b8d 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -182,6 +182,7 @@ def test_shipped_plugins_load_with_their_manifests() -> None: "insecure-temp-file", "insecure-tls", "missing-timeout", + "nosql-injection", "open-redirect", "path-traversal", "plaintext-credentials", @@ -199,7 +200,7 @@ def test_shipped_plugins_load_with_their_manifests() -> None: "weak-crypto", "xss", } - for rule in ("sql-injection", "command-injection", "code-injection", "path-traversal", "ssrf", "xss"): + for rule in ("sql-injection", "command-injection", "code-injection", "nosql-injection", "path-traversal", "ssrf", "xss"): assert by_name[rule].requires == ("taint.flows", "findings.refutation") assert by_name[rule].provides == (f"vulnerability.{rule}",) assert by_name["python-stdlib-models"].provides == ("model.python-stdlib",) diff --git a/tests/test_django_http_clients.py b/tests/test_django_http_clients.py index 644a1c8..f766abc 100644 --- a/tests/test_django_http_clients.py +++ b/tests/test_django_http_clients.py @@ -295,6 +295,7 @@ def test_detectors_stay_generic() -> None: "code_injection", "command_injection", "insecure_deserialization", + "nosql_injection", "open_redirect", "path_traversal", "plaintext_credentials", diff --git a/tests/test_docs.py b/tests/test_docs.py index 25979ae..d2c299a 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -90,7 +90,7 @@ def test_every_bundled_plugin_is_named_in_the_guides() -> None: re.search(r'^name = "([a-z-]+)"', manifest.read_text(encoding="utf-8"), re.MULTILINE).group(1) # type: ignore[union-attr] for manifest in engine.BUNDLED_PLUGINS.rglob("plugin.toml") ] - assert len(names) == 36 + assert len(names) == 37 assert all(f"`{name}`" in text for name in names), [n for n in names if f"`{n}`" not in text] diff --git a/tests/test_nosql_injection.py b/tests/test_nosql_injection.py new file mode 100644 index 0000000..dd29831 --- /dev/null +++ b/tests/test_nosql_injection.py @@ -0,0 +1,206 @@ +"""NoSQL injection: attacker input that may hold a structure reaching a NOSQL sink. + +MongoDB operator injection needs a structure: a JSON body giving +``{"password": {"$ne": null}}`` makes ``find_one({"name": name, "password": password})`` +match any password. A string cannot hold an operator, so only input that may hold a +structure carries ``NOSQL``: JSON and the raw bodies a ``json.loads`` decodes, files, +WebSocket messages, standard input. Text input carries ``TEXT_KINDS``, every kind but +``NOSQL``: form fields, query parameters, headers, cookies, URL parameters, the command +line and the environment. An entry-point parameter annotated with a scalar type is +text too, since FastAPI validates it before the handler runs. + +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. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.findings import Finding, Severity +from coretrace_python.source import SourceManager +from coretrace_python.taint import TEXT_KINDS, TaintKind + +MANIFEST = ( + 'name = "docstore-sinks"\nversion = "1.0.0"\nplugin_api = ">=1,<2"\nrequires = []\n' + 'provides = ["model.docstore"]\n\n[entrypoint]\nmodule = "docstore_sinks"\nclass = "DocstoreSinks"\n' +) +PLUGIN = ''' +from typing import ClassVar + +from coretrace_python.plugins import ModelPlugin +from coretrace_python.semantic.symbols import SymbolId +from coretrace_python.taint import Model, Sink, TaintKind + + +class DocstoreSinks(ModelPlugin): + name: ClassVar[str] = "docstore-sinks" + models: ClassVar[tuple[Model, ...]] = ( + Sink(SymbolId("python.docstore.find"), TaintKind.NOSQL, ((TaintKind.NOSQL, (0,)),)), + ) +''' + + +@pytest.fixture(scope="module") +def plugins(tmp_path_factory: pytest.TempPathFactory) -> list[Path]: + root = tmp_path_factory.mktemp("plugins") + plugin = root / "docstore_sinks" + plugin.mkdir() + (plugin / "plugin.toml").write_text(MANIFEST, encoding="utf-8") + (plugin / "docstore_sinks.py").write_text(PLUGIN, encoding="utf-8") + return [engine.BUNDLED_PLUGINS, root] + + +def check(plugins: list[Path], text: str) -> tuple[Finding, ...]: + return engine.check(SourceManager().add_source("app.py", text), plugins) + + +def injected(plugins: list[Path], text: str) -> list[int]: + return [f.span.start_line for f in check(plugins, text) if f.rule_id == "nosql-injection"] + + +def test_text_kinds_are_every_kind_but_nosql() -> None: + assert TEXT_KINDS == TaintKind.ALL & ~TaintKind.NOSQL + + +FLASK = ( + "import json\n\nimport docstore\nfrom flask import Flask, request\n\n" + "app = Flask(__name__)\n\n" + "@app.route('/users/')\n" + "def users(name):\n" + " return str(docstore.find({query}))\n" +) + + +@pytest.mark.parametrize( + "query", + [ + '{"name": request.json["name"], "password": request.json["password"]}', + "request.get_json()", + "json.loads(request.data)", + "json.loads(request.get_data())", + "json.load(request.files['filter'])", + ], +) +def test_a_structure_from_the_request_in_a_filter_is_a_nosql_injection(plugins: list[Path], query: str) -> None: + findings = [f for f in check(plugins, FLASK.format(query=query)) if f.rule_id == "nosql-injection"] + + assert [f.span.start_line for f in findings] == [10] + assert findings[0].severity is Severity.HIGH + assert findings[0].metadata["source_label"] == "http" + + +@pytest.mark.parametrize( + "query", + [ + '{"name": request.form["name"]}', + '{"name": request.args.get("name")}', + '{"name": request.values["name"], "tag": request.cookies["tag"]}', + '{"agent": request.headers["User-Agent"], "path": request.path}', + '{"name": name}', + ], +) +def test_text_from_the_request_cannot_hold_an_operator(plugins: list[Path], query: str) -> None: + assert injected(plugins, FLASK.format(query=query)) == [] + + +def test_text_sources_still_carry_the_other_kinds(plugins: list[Path]) -> None: + text = ( + "import sqlite3\nfrom flask import Flask, request\n\napp = Flask(__name__)\n\n" + "@app.route('/users/')\n" + "def users(name):\n" + " sqlite3.connect('app.db').execute(\"SELECT * FROM users WHERE name = '\" + request.form['q'] + \"'\")\n" + " sqlite3.connect('app.db').execute(\"SELECT * FROM users WHERE name = '\" + name + \"'\")\n" + ) + + assert sorted(f.span.start_line for f in check(plugins, text) if f.rule_id == "sql-injection") == [8, 9] + + +FASTAPI = ( + "from typing import Annotated, Optional, Union\nfrom uuid import UUID\n\n" + "import docstore\nfrom fastapi import FastAPI, Query\nfrom pydantic import BaseModel\n\n" + "class Filter(BaseModel):\n name: str\n\n" + "app = FastAPI()\n\n" + "@app.get('/users')\n" + "def users({parameter}):\n" + " return docstore.find({{'name': q}})\n" +) + + +@pytest.mark.parametrize( + "parameter", + ["q: dict", "q", "q: Filter", "q: list[dict]", "q: Optional[dict] = None"], +) +def test_a_parameter_that_may_hold_a_structure_is_structured_input(plugins: list[Path], parameter: str) -> None: + assert injected(plugins, FASTAPI.format(parameter=parameter)) == [15] + + +@pytest.mark.parametrize( + "parameter", + [ + "q: str", + "q: int = 0", + "q: Annotated[str, Query()]", + "q: Optional[int] = None", + "q: str | None = None", + "q: Union[str, None] = None", + "q: Annotated[Optional[str], Query(max_length=20)] = None", + "q: UUID", + # A container of scalars holds no operator either. + "q: list[str] = Query()", + ], +) +def test_a_parameter_annotated_with_a_scalar_type_is_text(plugins: list[Path], parameter: str) -> None: + assert injected(plugins, FASTAPI.format(parameter=parameter)) == [] + + +@pytest.mark.parametrize( + "body, reported", + [ + (" return docstore.find(json.loads(sys.stdin.read()))\n", True), + (" return docstore.find({'name': input()})\n", False), + (" return docstore.find({'name': sys.argv[1]})\n", False), + (" return docstore.find({'name': os.environ['USER']})\n", False), + ], +) +def test_standard_input_is_a_payload_and_the_command_line_text(plugins: list[Path], body: str, reported: bool) -> None: + text = f"import json\nimport os\nimport sys\n\nimport docstore\n\ndef run():\n{body}" + + assert injected(plugins, text) == ([8] if reported else []) + + +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" +) + + +@pytest.mark.parametrize( + "query", + [ + "json.loads(request.body)", + # Limit: the request object is tainted as a whole, its text fields too. + '{"name": request.GET["name"]}', + ], +) +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_parameters_of_one_entry_point_remain_one_origin(plugins: list[Path]) -> None: + text = ( + "from fastapi import FastAPI\nfrom fastapi.responses import HTMLResponse\n\n" + "app = FastAPI()\n\n" + "@app.get('/users')\n" + "def users(q, n: int):\n" + " return HTMLResponse(q + str(n))\n" + ) + + assert [f.span.start_line for f in check(plugins, text) if f.rule_id == "xss"] == [8] diff --git a/tests/test_operator_sources.py b/tests/test_operator_sources.py index a9cad30..1fb0a58 100644 --- a/tests/test_operator_sources.py +++ b/tests/test_operator_sources.py @@ -22,7 +22,7 @@ from coretrace_python.findings import Finding from coretrace_python.semantic.symbols import SymbolId from coretrace_python.source import SourceManager -from coretrace_python.taint import TaintKind +from coretrace_python.taint import TEXT_KINDS, TaintKind REPO = Path(__file__).resolve().parent.parent PLUGINS = REPO / "src" / "coretrace_python" / "bundled" @@ -52,10 +52,12 @@ def rules(findings: tuple[Finding, ...]) -> list[str]: def test_source_kinds_reflect_who_controls_them() -> None: - assert source_kinds("os.environ") == TaintKind.ALL & ~(TaintKind.COMMAND | TaintKind.PATH) + # The environment, argv and a line of input are text, which holds no query operator + # (#158); process output is a payload. + assert source_kinds("os.environ") == TEXT_KINDS & ~(TaintKind.COMMAND | TaintKind.PATH) assert source_kinds("subprocess.check_output") == TaintKind.ALL & ~TaintKind.PATH - assert source_kinds("sys.argv") == TaintKind.ALL & ~TaintKind.PATH - assert source_kinds("builtins.input") == TaintKind.ALL + assert source_kinds("sys.argv") == TEXT_KINDS & ~TaintKind.PATH + assert source_kinds("builtins.input") == TEXT_KINDS def test_environment_does_not_inject_commands_or_paths_but_still_reaches_urls_and_statements() -> None: diff --git a/tests/test_packaging.py b/tests/test_packaging.py index 10143ae..faecb0a 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -64,7 +64,7 @@ def test_bundled_plugins_live_inside_the_package() -> None: assert bundled == Path(coretrace_python.__file__).resolve().parent / "bundled" assert bundled.is_dir() manifests = sorted(p.relative_to(bundled).as_posix() for p in bundled.rglob("plugin.toml")) - assert len(manifests) == 36 + assert len(manifests) == 37 assert "syntax/dangerous_eval/plugin.toml" in manifests assert not (REPO / "plugins").exists() @@ -108,7 +108,7 @@ def test_the_wheel_carries_the_bundled_plugins(tmp_path: Path) -> None: assert "coretrace_python/bundled/syntax/dangerous_eval/plugin.toml" in names assert "coretrace_python/bundled/syntax/dangerous_eval/dangerous_eval.py" in names - assert sum(1 for n in names if n.endswith("/plugin.toml")) == 36 + assert sum(1 for n in names if n.endswith("/plugin.toml")) == 37 assert not any(n.startswith("tests/") or "tests-project" in n for n in names) From caa400644b35657d2353798cd5dcf9ab8fb122f7 Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 26 Sep 2026 01:23:00 +0200 Subject: [PATCH 6/6] feat: report structured attacker input reaching NoSQL sinks Only input that may hold a structure carries NOSQL: text sources carry TEXT_KINDS, and an entry-point parameter annotated with scalars loses NOSQL. A flow names each origin once, whatever kinds the parameters of one entry point carry. --- CHANGELOG.md | 3 + docs/plugins.md | 2 +- docs/usage.md | 1 + .../bundled/models/bottle/bottle_models.py | 16 ++-- .../bundled/models/cli/cli_models.py | 8 +- .../bundled/models/flask/flask_models.py | 21 +++--- .../models/python_stdlib/python_stdlib.py | 8 +- .../bundled/models/tornado/tornado_models.py | 15 ++-- .../nosql_injection/nosql_injection.py | 20 +++++ .../security/nosql_injection/plugin.toml | 9 +++ src/coretrace_python/taint/__init__.py | 2 + src/coretrace_python/taint/engine.py | 75 +++++++++++++++++-- src/coretrace_python/taint/models.py | 5 ++ 13 files changed, 151 insertions(+), 34 deletions(-) create mode 100644 src/coretrace_python/bundled/security/nosql_injection/nosql_injection.py create mode 100644 src/coretrace_python/bundled/security/nosql_injection/plugin.toml diff --git a/CHANGELOG.md b/CHANGELOG.md index c29aa5c..28a29fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,12 @@ ### Detection - A module-level name bound to an attribute, an item or a call of another module-level name resolves in functions, as the same value does in a function body. `cursor = conn.cursor()` after `conn = sqlite3.connect(...)`, `db = client.shop` and `run = os.system` used to leave the calls made through them unknown, so a query built from request data and executed through a module-level cursor was not reported. Names resolve in statement order. The regression corpus is unchanged (#155). +- `nosql-injection` reports attacker input that may hold a structure reaching a NoSQL sink, such as a MongoDB filter, where a JSON body giving `{"$ne": null}` matches any value. Only input that may hold a structure carries `NOSQL`: JSON, the raw bodies a `json.loads` decodes, files, WebSocket messages and standard input. Text cannot hold an operator. The text sources of Flask, bottle and tornado, the URL parameters of their routes, the command line, the environment and `input()` no longer carry `NOSQL`. Neither does an entry-point parameter annotated with scalars (`str`, `Optional[int]`, `Annotated[str, Query()]`, `list[str]`, …), which FastAPI validates. Two limits remain. Django and aiohttp request objects stay tainted as a whole. The keys of a dict are not followed, so a string concatenated into `$where` is not reported. The engine ships no NoSQL sink: model plugins declare them (#158). ### Plugins +- `TEXT_KINDS` holds every kind but `NOSQL`: what text input carries. A model plugin declares its text sources with it (#158). + - A `Members(symbol, dynamic=None, defined=(), typed=())` model says what the attributes and items of a class's instances give, for libraries whose objects give others by names the project chooses. A pymongo client gives the database it is dotted or indexed with, a database gives a collection. The names used to end up in the symbol (`MongoClient.shop.users.find`), or an item vanished into its container's symbol (`MongoClient.find`), so no model could list them. With the model, every path to a collection denotes the collection class, the one a parameter annotated `Collection` denotes: `client.shop.users`, `client["shop"]["users"]`, `client.get_database("shop").get_collection("users")`, a module-level `db = client.shop`, an attribute inherited from the client class. A sink on `Collection.find` covers them all. Symbol resolution now depends on these models, which the engine provides to every module and worker (#157). ## 0.11.0 (2026-09-25) diff --git a/docs/plugins.md b/docs/plugins.md index d8d0d78..30379c3 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -140,7 +140,7 @@ class BottleModels(ModelPlugin): | Model | Declares | |---|---| -| `Source(symbol, label, kinds=ALL)` | A call or attribute whose value is attacker-controlled; `label` names it in messages (`http`, `stdin`, `argv`, `http-response`). `kinds` restricts what it can inject. | +| `Source(symbol, label, kinds=ALL)` | A call or attribute whose value is attacker-controlled; `label` names it in messages (`http`, `stdin`, `argv`, `http-response`). `kinds` restricts what it can inject. Text input, such as a form field, a header or a URL parameter, carries `TEXT_KINDS`, every kind but `NOSQL`: a string cannot hold a query operator. | | `Sink(symbol, kinds, positions=(), keywords=())` | A call dangerous for `kinds`. `positions` limits a kind to the given argument indexes, and `keywords` to the given keyword names; a restricted kind reaches only the arguments listed there. SQL sinks read the statement only, and the SSRF sinks of HTTP clients read the URL, by position or as `url=`. A starred argument or `**kwargs` reaches no restricted kind. | | `Sanitizer(symbol, kinds)` | A call whose result is safe for `kinds`, including when the call is inside a project function the data goes through. | | `EntryPoint(symbol, label, kinds=ALL)` | A decorator or a class base whose functions receive their parameters as `label` input, such as `flask.Flask.route`. | diff --git a/docs/usage.md b/docs/usage.md index 56b1678..a39f17e 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -112,6 +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. | | `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/bottle/bottle_models.py b/src/coretrace_python/bundled/models/bottle/bottle_models.py index 2936074..b622de9 100644 --- a/src/coretrace_python/bundled/models/bottle/bottle_models.py +++ b/src/coretrace_python/bundled/models/bottle/bottle_models.py @@ -8,13 +8,15 @@ from coretrace_python.plugins import ModelPlugin from coretrace_python.semantic.symbols import SymbolId -from coretrace_python.taint import EntryPoint, Model, Sanitizer, Sink, Source, TaintKind +from coretrace_python.taint import TEXT_KINDS, EntryPoint, Model, Sanitizer, Sink, Source, TaintKind _METHODS = ("route", "get", "post", "put", "delete", "patch") -_REQUEST_ATTRIBUTES = ( - "query", "forms", "params", "json", "body", "headers", "cookies", "GET", "POST", "files", +# Text, which cannot hold a query operator; JSON, raw bodies and files may hold a structure. +_TEXT_ATTRIBUTES = ( + "query", "forms", "params", "headers", "cookies", "GET", "POST", "url", "path", "fullpath", "query_string", "get_cookie", "get_header", ) +_PAYLOAD_ATTRIBUTES = ("json", "body", "files") _FIRST_ONLY = lambda kind: ((kind, (0,)),) @@ -25,9 +27,11 @@ def _sym(path: str) -> SymbolId: class BottleModels(ModelPlugin): name: ClassVar[str] = "bottle-models" models: ClassVar[tuple[Model, ...]] = ( - *(EntryPoint(_sym(f"bottle.{method}"), "http") for method in _METHODS), - *(EntryPoint(_sym(f"bottle.Bottle.{method}"), "http") for method in _METHODS), - *(Source(_sym(f"bottle.request.{attribute}"), "http") for attribute in _REQUEST_ATTRIBUTES), + # A route's parameters are segments of its URL. + *(EntryPoint(_sym(f"bottle.{method}"), "http", TEXT_KINDS) for method in _METHODS), + *(EntryPoint(_sym(f"bottle.Bottle.{method}"), "http", TEXT_KINDS) for method in _METHODS), + *(Source(_sym(f"bottle.request.{attribute}"), "http", TEXT_KINDS) for attribute in _TEXT_ATTRIBUTES), + *(Source(_sym(f"bottle.request.{attribute}"), "http") for attribute in _PAYLOAD_ATTRIBUTES), Sink(_sym("bottle.redirect"), TaintKind.REDIRECT, _FIRST_ONLY(TaintKind.REDIRECT)), # ``static_file(filename, root)``: the root is the application's own. Sink(_sym("bottle.static_file"), TaintKind.PATH, _FIRST_ONLY(TaintKind.PATH)), diff --git a/src/coretrace_python/bundled/models/cli/cli_models.py b/src/coretrace_python/bundled/models/cli/cli_models.py index 72ac98f..e6561f5 100644 --- a/src/coretrace_python/bundled/models/cli/cli_models.py +++ b/src/coretrace_python/bundled/models/cli/cli_models.py @@ -6,11 +6,11 @@ from coretrace_python.plugins import ModelPlugin from coretrace_python.semantic.symbols import SymbolId -from coretrace_python.taint import EntryPoint, Model, TaintKind +from coretrace_python.taint import TEXT_KINDS, EntryPoint, Model, TaintKind -# A command-line tool is expected to open the paths it is given: argv input carries every -# kind but PATH. -ARGV_KINDS = TaintKind.ALL & ~TaintKind.PATH +# A command-line tool is expected to open the paths it is given: argv input is text, +# without PATH. +ARGV_KINDS = TEXT_KINDS & ~TaintKind.PATH _COMMANDS = ( "click.command", diff --git a/src/coretrace_python/bundled/models/flask/flask_models.py b/src/coretrace_python/bundled/models/flask/flask_models.py index 765cf20..2d33563 100644 --- a/src/coretrace_python/bundled/models/flask/flask_models.py +++ b/src/coretrace_python/bundled/models/flask/flask_models.py @@ -7,6 +7,7 @@ from coretrace_python.plugins import ModelPlugin from coretrace_python.semantic.symbols import SymbolId from coretrace_python.taint import ( + TEXT_KINDS, AuthorizationGuard, EntryPoint, Model, @@ -17,10 +18,10 @@ TaintKind, ) -_REQUEST_ATTRIBUTES = ( - "args", "form", "values", "json", "data", "cookies", "headers", "files", - "get_json", "get_data", "url", "full_path", "path", "query_string", "stream", -) +# Strings, or mappings of strings: text, which cannot hold a query operator. +_TEXT_ATTRIBUTES = ("args", "form", "values", "cookies", "headers", "url", "full_path", "path", "query_string") +# JSON, and the raw bodies and files a ``json.loads`` may decode into a structure. +_PAYLOAD_ATTRIBUTES = ("json", "get_json", "data", "get_data", "stream", "files") _TARGET_ONLY = ((TaintKind.REDIRECT, (0,)),) @@ -33,11 +34,13 @@ def _sym(path: str) -> SymbolId: class FlaskModels(ModelPlugin): name: ClassVar[str] = "flask-models" models: ClassVar[tuple[Model, ...]] = ( - *(Source(_sym(f"flask.request.{attribute}"), "http") for attribute in _REQUEST_ATTRIBUTES), - EntryPoint(_sym("flask.Flask.route"), "http"), - EntryPoint(_sym("flask.Blueprint.route"), "http"), - RouteRegistrar(_sym("flask.Flask.add_url_rule"), 2, "http", keyword="view_func"), - RouteRegistrar(_sym("flask.Blueprint.add_url_rule"), 2, "http", keyword="view_func"), + *(Source(_sym(f"flask.request.{attribute}"), "http", TEXT_KINDS) for attribute in _TEXT_ATTRIBUTES), + *(Source(_sym(f"flask.request.{attribute}"), "http") for attribute in _PAYLOAD_ATTRIBUTES), + # A route's parameters are segments of its URL. + EntryPoint(_sym("flask.Flask.route"), "http", TEXT_KINDS), + EntryPoint(_sym("flask.Blueprint.route"), "http", TEXT_KINDS), + RouteRegistrar(_sym("flask.Flask.add_url_rule"), 2, "http", TEXT_KINDS, keyword="view_func"), + RouteRegistrar(_sym("flask.Blueprint.add_url_rule"), 2, "http", TEXT_KINDS, keyword="view_func"), Sink(_sym("flask.render_template_string"), TaintKind.HTML), Sink(_sym("flask.make_response"), TaintKind.HTML), Sink(_sym("flask.Response"), TaintKind.HTML), diff --git a/src/coretrace_python/bundled/models/python_stdlib/python_stdlib.py b/src/coretrace_python/bundled/models/python_stdlib/python_stdlib.py index ec2bbb5..e0a52e6 100644 --- a/src/coretrace_python/bundled/models/python_stdlib/python_stdlib.py +++ b/src/coretrace_python/bundled/models/python_stdlib/python_stdlib.py @@ -7,6 +7,7 @@ from coretrace_python.plugins import ModelPlugin from coretrace_python.semantic.symbols import SymbolId from coretrace_python.taint import ( + TEXT_KINDS, Model, SafeArgument, Sanitizer, @@ -16,7 +17,7 @@ Validator, ) -_ENVIRONMENT_KINDS = TaintKind.ALL & ~(TaintKind.COMMAND | TaintKind.PATH) +_ENVIRONMENT_KINDS = TEXT_KINDS & ~(TaintKind.COMMAND | TaintKind.PATH) # SafeLoader and BaseLoader build plain data only, in Python and in C, under every spelling. _SAFE_YAML_LOADERS = ( "python.yaml.SafeLoader", @@ -38,13 +39,14 @@ def _sym(path: str) -> SymbolId: class PythonStdlibModels(ModelPlugin): name: ClassVar[str] = "python-stdlib-models" models: ClassVar[tuple[Model, ...]] = ( - Source(_sym("builtins.input"), "stdin"), + # A line of text; the stream may be a payload ``json.load`` decodes. + Source(_sym("builtins.input"), "stdin", TEXT_KINDS), Source(_sym("sys.stdin"), "stdin"), # Operator-controlled inputs. A command-line tool is expected to open the paths # it is given; the environment is set by whoever runs the program, so a command # or a path built from it is not an injection; the output of a local process is # not a path either, but a downloaded script piped into a shell is a real flaw. - Source(_sym("sys.argv"), "argv", TaintKind.ALL & ~TaintKind.PATH), + Source(_sym("sys.argv"), "argv", TEXT_KINDS & ~TaintKind.PATH), Source(_sym("os.environ"), "environment", _ENVIRONMENT_KINDS), Source(_sym("subprocess.run.stdout"), "process-output", _PROCESS_OUTPUT_KINDS), Source(_sym("subprocess.check_output"), "process-output", _PROCESS_OUTPUT_KINDS), diff --git a/src/coretrace_python/bundled/models/tornado/tornado_models.py b/src/coretrace_python/bundled/models/tornado/tornado_models.py index 6960d14..918e216 100644 --- a/src/coretrace_python/bundled/models/tornado/tornado_models.py +++ b/src/coretrace_python/bundled/models/tornado/tornado_models.py @@ -8,12 +8,13 @@ from coretrace_python.plugins import ModelPlugin from coretrace_python.semantic.symbols import SymbolId -from coretrace_python.taint import EntryPoint, Model, Sanitizer, Sink, Source, TaintKind +from coretrace_python.taint import TEXT_KINDS, EntryPoint, Model, Sanitizer, Sink, Source, TaintKind _HANDLERS = ("tornado.web.RequestHandler", "tornado.websocket.WebSocketHandler") -_REQUEST_METHODS = ( +# Text, which cannot hold a query operator; the request object holds the raw body too. +_TEXT_METHODS = ( "get_argument", "get_arguments", "get_query_argument", "get_query_arguments", - "get_body_argument", "get_body_arguments", "get_cookie", "request", "path_args", "path_kwargs", + "get_body_argument", "get_body_arguments", "get_cookie", "path_args", "path_kwargs", ) _CLIENTS = ("tornado.httpclient.AsyncHTTPClient.fetch", "tornado.httpclient.HTTPClient.fetch") _TARGET_ONLY = ((TaintKind.REDIRECT, (0,)),) @@ -26,8 +27,12 @@ def _sym(path: str) -> SymbolId: class TornadoModels(ModelPlugin): name: ClassVar[str] = "tornado-models" models: ClassVar[tuple[Model, ...]] = ( - *(EntryPoint(_sym(handler), "http") for handler in _HANDLERS), - *(Source(_sym(f"{handler}.{method}"), "http") for handler in _HANDLERS for method in _REQUEST_METHODS), + # A request handler's parameters are segments of its URL; a WebSocket handler also + # receives the messages, raw payloads a ``json.loads`` may decode. + EntryPoint(_sym("tornado.web.RequestHandler"), "http", TEXT_KINDS), + 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), *(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/bundled/security/nosql_injection/nosql_injection.py b/src/coretrace_python/bundled/security/nosql_injection/nosql_injection.py new file mode 100644 index 0000000..fda5452 --- /dev/null +++ b/src/coretrace_python/bundled/security/nosql_injection/nosql_injection.py @@ -0,0 +1,20 @@ +"""NoSQL injection: attacker input that may hold a structure reaching a NOSQL sink, such +as a MongoDB filter, where a JSON body giving ``{"$ne": null}`` matches any value. Text +input carries no ``NOSQL``: a string cannot hold an operator. The engine ships no NOSQL +sink; model plugins declare them.""" + +from __future__ import annotations + +from typing import ClassVar + +from coretrace_python.findings import Severity +from coretrace_python.plugins import TaintDetector +from coretrace_python.taint import TaintKind + + +class NoSqlInjectionPlugin(TaintDetector): + name: ClassVar[str] = "nosql-injection" + rule_id: ClassVar[str] = "nosql-injection" + kind: ClassVar[TaintKind] = TaintKind.NOSQL + severity: ClassVar[Severity] = Severity.HIGH + title: ClassVar[str] = "NoSQL injection" diff --git a/src/coretrace_python/bundled/security/nosql_injection/plugin.toml b/src/coretrace_python/bundled/security/nosql_injection/plugin.toml new file mode 100644 index 0000000..85bada1 --- /dev/null +++ b/src/coretrace_python/bundled/security/nosql_injection/plugin.toml @@ -0,0 +1,9 @@ +name = "nosql-injection" +version = "1.0.0" +plugin_api = ">=1,<2" +requires = ["taint.flows", "findings.refutation"] +provides = ["vulnerability.nosql-injection"] + +[entrypoint] +module = "nosql_injection" +class = "NoSqlInjectionPlugin" diff --git a/src/coretrace_python/taint/__init__.py b/src/coretrace_python/taint/__init__.py index 9a0329f..860dbbc 100644 --- a/src/coretrace_python/taint/__init__.py +++ b/src/coretrace_python/taint/__init__.py @@ -10,6 +10,7 @@ propagate_taint, ) from coretrace_python.taint.models import ( + TEXT_KINDS, AuthorizationGuard, EntryPoint, Model, @@ -33,6 +34,7 @@ from coretrace_python.taint.templates import EscapedTemplates, escaped_templates __all__ = [ + "TEXT_KINDS", "AuthorizationGuard", "EntryPoint", "EntryPointAnalysis", diff --git a/src/coretrace_python/taint/engine.py b/src/coretrace_python/taint/engine.py index 21f9a45..22bedcf 100644 --- a/src/coretrace_python/taint/engine.py +++ b/src/coretrace_python/taint/engine.py @@ -70,7 +70,7 @@ Value, ) from coretrace_python.ir.ssa import SSAAnalysis -from coretrace_python.semantic.scopes import ScopeAnalysis, ScopeTable +from coretrace_python.semantic.scopes import ScopeAnalysis, ScopeId, ScopeTable from coretrace_python.semantic.symbols import SymbolAnalysis, SymbolId, SymbolTable from coretrace_python.source import SourceSpan from coretrace_python.taint.models import ( @@ -551,7 +551,12 @@ def report( reaching = taint.kinds & sink.kinds_at(position, keyword) if not reaching: return - for source in sorted(taint.sources, key=lambda s: str(s.symbol)): + # One flow per origin: the parameters of one entry point share its source, with + # the kinds each can carry (a scalar one no NOSQL); the flow's kinds are the taint's. + origins: dict[tuple[SymbolId, str], Source] = {} + for source in sorted(taint.sources, key=lambda s: (str(s.symbol), s.label, s.kinds.value)): + origins.setdefault((source.symbol, source.label), source) + for source in origins.values(): flows.append( TaintFlow( source, @@ -770,17 +775,22 @@ def parameter_sources( ) -> Mapping[int, Source]: """The attacker-controlled parameters of ``function``, by index: every parameter of an entry point (``self`` excepted for a method), including one registered elsewhere - (``routes``), and every parameter annotated with a typed-parameter symbol.""" + (``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.""" owner = _owner(module, function) sources: dict[int, Source] = {} + scope = scopes.scope_for(function) + enclosing = scope.parent if scope.parent is not None else scope.id 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 for index in range(first, len(function.parameters)): - sources[index] = Source(entry.symbol, entry.label, entry.kinds) - scope = scopes.scope_for(function) - enclosing = scope.parent if scope.parent is not None else scope.id + 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 + sources[index] = Source(entry.symbol, entry.label, kinds) for index, parameter in enumerate(function.parameters): if parameter.annotation is None: continue @@ -800,6 +810,59 @@ def parameter_sources( return sources +def _typing(*names: str) -> frozenset[SymbolId]: + return frozenset(SymbolId(f"python.{module}.{name}") for module in ("typing", "typing_extensions") for name in names) + + +_SCALARS = frozenset( + SymbolId(f"python.{name}") + for name in ( + "builtins.str", + "builtins.int", + "builtins.float", + "builtins.bool", + "builtins.bytes", + "uuid.UUID", + "datetime.date", + "datetime.datetime", + "datetime.time", + "datetime.timedelta", + "decimal.Decimal", + ) +) +_CONTAINERS = frozenset( + SymbolId(f"python.builtins.{name}") for name in ("list", "set", "frozenset", "tuple") +) | _typing("List", "Set", "FrozenSet", "Tuple", "Sequence") +_UNIONS = _typing("Optional", "Union") +_LITERAL = _typing("Literal") +_ANNOTATED = _typing("Annotated") + + +def _scalar(annotation: nodes.Expression, symbols: SymbolTable, scope: ScopeId) -> bool: + """Whether a value annotated with ``annotation`` holds scalars only: a scalar class, + a literal, a container of scalars, a union of them with ``None``, or one of them + ``Annotated`` with metadata. Anything unknown may hold a structure.""" + + if isinstance(annotation, nodes.Constant): + return annotation.value is None or annotation.value is Ellipsis + if isinstance(annotation, nodes.BinaryOp): + return annotation.operator == "bit_or" and all( + _scalar(side, symbols, scope) for side in (annotation.left, annotation.right) + ) + if isinstance(annotation, nodes.Subscript): + origin = symbols.resolve_expression(scope, annotation.value) + key = annotation.key + arguments = key.elements if isinstance(key, nodes.Tuple) else (key,) + if origin in _LITERAL: + return True + if origin in _ANNOTATED: + return _scalar(arguments[0], symbols, scope) + return (origin in _UNIONS or origin in _CONTAINERS) and all( + _scalar(argument, symbols, scope) for argument in arguments + ) + return symbols.resolve_expression(scope, annotation) in _SCALARS + + def propagate_taint( name: str, function: FunctionIR, diff --git a/src/coretrace_python/taint/models.py b/src/coretrace_python/taint/models.py index 7534086..afa2670 100644 --- a/src/coretrace_python/taint/models.py +++ b/src/coretrace_python/taint/models.py @@ -42,6 +42,11 @@ class TaintKind(Flag): PII = auto() +# What text input carries: a string cannot hold a query operator, only a structure can, +# so a form field, a header or a URL parameter carries every kind but NOSQL. +TEXT_KINDS = TaintKind.ALL & ~TaintKind.NOSQL + + class ModelError(Exception): """Two models of the same kind claim the same symbol."""