feat(code-injection): report attributes chosen by input and then called

docs/plugins.md

@@ -169,7 +169,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. 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. |
+| `Sink(symbol, kinds, positions=(), keywords=(), selects=False, requires_called_result=NONE)` | 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. `selects` marks an argument that chooses by name the code that runs (a module, a file, a directory, an attribute): only a check limiting it to constants proves it. `requires_called_result` lists the kinds that apply only when the call's result is itself called, as for `getattr`'s name: the engine reports them at each invocation of the result, the other kinds at the call. |
 | `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, inputs=None)` | A decorator or a class base whose functions receive their parameters as `label` input, such as `flask.Flask.route`; or a project function or class named by its own symbol (`python.app.handlers.main`), as an integration reading a deployment declaration names a Lambda handler. `inputs` limits the parameters that are input to those positions, after `self` for a method: a handler's event, not its context. |
 | `TypedParameter(symbol, label, kinds=ALL)` | A class whose annotated parameters carry `label` input, such as Django's `HttpRequest`. |
@@ -187,7 +187,9 @@ class BottleModels(ModelPlugin):
 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
 another plugin declared for `COMMAND`; any other difference is a conflict that stops
-the analysis with a `ModelError` naming both plugins.
+the analysis with a `ModelError` naming both plugins. A merged sink `selects` when either
+does, and a kind requires the called result only when every declaration carrying it
+does: an unconditional declaration is never weakened, whatever the order.
 
 A plugin may also read models from the project it analyses: `project_models(self, root)`
 returns them, from a file at `root` for instance. It is called once per directory check,

docs/usage.md

@@ -121,7 +121,7 @@ variable is not treated that way yet.
 | Rule | Reached sink |
 |---|---|
 | `command-injection` | Shell or process execution (`os.system`, `subprocess` with a string, …), judged on what the input controls in the command. A string a shell runs, a string naming the program, the program itself, or an operand the program runs as a command (after the host for `ssh`, the program of `sudo`, `env`, `xargs`, `timeout`…, any argument of a batch file) is `high`. The script an interpreter runs (`bash run.sh`, `python tool.py`, its first operand when no option such as `-c` gives the code), or the module of `python -m`, is `high` and kept for review: execution is possible, and its exploitability depends on the file the input chooses. The arguments after a script or code are the script's own: options of unknown meaning, reported `medium` (a hotspot after an option of the script), refuted behind a constant prefix. Otherwise an element of an argument list run without a shell can only be read as an option: `medium`, or `high` when a documented option of the program runs a command it chooses (`git push --receive-pack=…`, `ssh -o ProxyCommand=…`). A value a documented option takes is judged by what the option does with it, whatever its prefix: run it (`sh -c`, `cmd /c`, `python -c`: `high`), keep it as data (`git commit -m`: refuted). After an option the rule does not know, the finding stays a hotspot to review. A constant prefix that keeps a standalone input from being an option (`f"v{version}"`), or an operand after `--` for a program that honours it, is refuted. Every case keeps the rule id `command-injection`, so suppressions and baselines match whatever the case. When the rule reads the command element by element and establishes what the input is there, the finding's `injection` metadata (in the JSON report, and as a property of its SARIF result) says so: `command` when the input runs or chooses what runs (a shell string, the program, an operand run as a command, the value of an option that runs it); `option` when it can only reach the program as an option or as the value of an option not established to run it, and the message is then titled `Option injection`. When that cannot be established (an option whose effect is to review, an unknown program or element before the input, a command that is not a list or tuple display, a sink such as `os.system` the rule does not read element by element), the metadata is absent and the title stays `Command injection`. |
-| `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. Also the module `importlib.import_module`, `__import__` or `runpy.run_module` imports, and the file `runpy.run_path` runs, by position or by keyword (`name`, `mod_name`, `path_name`); the package of a relative name, the globals and the run name are data. An import runs the top-level code of a module the input chooses among those that can be found, a script path the file it chooses: execution is possible, and how far it goes depends on what the input can reach, so these findings are titled `Untrusted module import` and `Untrusted script execution`. A constant package prefix (`"app.plugins." + name`) confines an import to that package: a hotspot. The environment and the command line reach these sinks as they reach `eval`. Also the directory `sys.path.insert` (its second argument), `sys.path.append` and `sys.path.extend` add to the import path, and the directory `site.addsitedir` adds (`sitedir`), judged the same way. A directory on the import path runs code only when a later import resolves to a module the attacker placed there: a hotspot, titled `Untrusted import path`. `site.addsitedir` runs the import lines of the directory's `.pth` files during the call: a vulnerability, titled `Untrusted site directory`, exploitable when the directory holds files the attacker controls, as the finding's evidence states. Coverage of the import path is partial: writes to `sys.path` that are not calls, a slice assignment (`sys.path[:0] = [d]`), `sys.path += [d]` or rebinding `sys.path`, are not reported. |
+| `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. Also the module `importlib.import_module`, `__import__` or `runpy.run_module` imports, and the file `runpy.run_path` runs, by position or by keyword (`name`, `mod_name`, `path_name`); the package of a relative name, the globals and the run name are data. An import runs the top-level code of a module the input chooses among those that can be found, a script path the file it chooses: execution is possible, and how far it goes depends on what the input can reach, so these findings are titled `Untrusted module import` and `Untrusted script execution`. A constant package prefix (`"app.plugins." + name`) confines an import to that package: a hotspot. The environment and the command line reach these sinks as they reach `eval`. Also the directory `sys.path.insert` (its second argument), `sys.path.append` and `sys.path.extend` add to the import path, and the directory `site.addsitedir` adds (`sitedir`), judged the same way. A directory on the import path runs code only when a later import resolves to a module the attacker placed there: a hotspot, titled `Untrusted import path`. `site.addsitedir` runs the import lines of the directory's `.pth` files during the call: a vulnerability, titled `Untrusted site directory`, exploitable when the directory holds files the attacker controls, as the finding's evidence states. Coverage of the import path is partial: writes to `sys.path` that are not calls, a slice assignment (`sys.path[:0] = [d]`), `sys.path += [d]` or rebinding `sys.path`, are not reported. Also the name `getattr(obj, name)` receives, when the attribute it selects is called: the finding, titled `Untrusted attribute call`, is placed at each call of the result, one per call, with the lines of the selection and of the call in its metadata (`selected_line`, `invoked_line`), and through project functions at the call that reaches them. What runs depends on what the object exposes, so it is a hotspot, on a module too and behind a constant prefix; a check proves the name only when it limits the value `getattr` received to constants, where `getattr` selects (even past a join) or before the call, and a check of a name rebound since proves nothing; a check before the call does not prove a callable selected in an earlier iteration of a loop, which reaches the call across the loop's back edge: only a check where it was selected does (even when the name never changes, which this version does not establish); nothing is reported when the selection or the call cannot run, in the function and inside a project function. Calling the result of a call, or an item of it, calls what the call returned: its arguments are not read as the call's, so `getattr(self, 'factory')(None, data)` or `eval('lambda x: x')(data)` reports nothing. A `getattr` whose result is only read is not reported, nor, in this version, one whose result is returned to the caller, stored in an attribute or a container, or passed on before it is called, nor `operator.attrgetter`, `operator.methodcaller`, `obj.__getattribute__(name)` or `vars(obj)[name]`; checks made inside a project function are not seen from its callers, as for every sink. |
 | `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. 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`, …). |

src/coretrace_python/bundled/models/python_stdlib/python_stdlib.py

@@ -80,6 +80,15 @@ class PythonStdlibModels(ModelPlugin):
         Sink(_sym("sys.path.append"), TaintKind.CODE, ((TaintKind.CODE, (0,)),), selects=True),
         Sink(_sym("sys.path.extend"), TaintKind.CODE, ((TaintKind.CODE, (0,)),), selects=True),
         Sink(_sym("site.addsitedir"), TaintKind.CODE, ((TaintKind.CODE, (0,)),), ((TaintKind.CODE, ("sitedir",)),), selects=True),
+        # The name of the attribute ``getattr`` selects, when what it selects is called;
+        # the object and the default are data (#240).
+        Sink(
+            _sym("builtins.getattr"),
+            TaintKind.CODE,
+            ((TaintKind.CODE, (1,)),),
+            selects=True,
+            requires_called_result=TaintKind.CODE,
+        ),
         Sink(_sym("builtins.open"), TaintKind.PATH),
         Sink(_sym("os.remove"), TaintKind.PATH),
         Sink(_sym("os.unlink"), TaintKind.PATH),

src/coretrace_python/bundled/security/code_injection/code_injection.py

@@ -1,8 +1,9 @@
 """Code injection: attacker-controlled input reaching a CODE sink, such as the code
 ``eval`` or ``exec`` runs, the module ``importlib.import_module``, ``__import__`` or
-``runpy.run_module`` imports, the file ``runpy.run_path`` runs, or the directory
-``sys.path`` and ``site.addsitedir`` load code from. ``dangerous-eval`` reports every
-``eval`` or ``exec``; this rule reports the calls attacker input reaches.
+``runpy.run_module`` imports, the file ``runpy.run_path`` runs, the directory
+``sys.path`` and ``site.addsitedir`` load code from, or the attribute ``getattr`` selects
+when it is called. ``dangerous-eval`` reports every ``eval`` or ``exec``; this rule reports
+the calls attacker input reaches.
 
 An import runs the top-level code of a module the input chooses among those that can be
 found, and a script path runs the file the input chooses: execution is possible, and
@@ -11,7 +12,9 @@
 modules of that package: a hotspot. A directory added to ``sys.path`` runs code only
 when a later import resolves there: a hotspot. ``site.addsitedir`` runs the import lines
 of the directory's ``.pth`` files during the call: a vulnerability, exploitable when the
-directory holds files the attacker controls, as the finding states."""
+directory holds files the attacker controls, as the finding states. The attribute
+``getattr`` selects runs at each call of its result, where the finding is placed: what
+runs depends on what the object exposes, a hotspot."""
 
 from __future__ import annotations
 
@@ -34,6 +37,7 @@
 _RUN_PATH = SymbolId("python.runpy.run_path")
 _IMPORT_PATHS = frozenset(SymbolId(f"python.sys.path.{name}") for name in ("insert", "append", "extend"))
 _SITE_DIRECTORY = SymbolId("python.site.addsitedir")
+_GETATTR = SymbolId("python.builtins.getattr")
 
 
 class CodeInjectionPlugin(TaintDetector):
@@ -59,6 +63,20 @@ def assess(
                     "import resolves to a module the attacker placed there",
                 )
             return Assessment(verdict, self.severity, "Untrusted import path")
+        if symbol == _GETATTR:
+            if verdict.status is Status.VULNERABILITY:
+                verdict = Verdict(
+                    flow,
+                    Status.HOTSPOT,
+                    "the input chooses the attribute called: what runs depends on what the object exposes",
+                )
+            lines: dict[str, str] = {}
+            if flow.invoked_at is not None and flow.sink_location is not None:
+                lines = {
+                    "selected_line": str(flow.sink_location.start_line),
+                    "invoked_line": str(flow.invoked_at.start_line),
+                }
+            return Assessment(verdict, self.severity, "Untrusted attribute call", lines)
         if symbol == _SITE_DIRECTORY:
             if verdict.status is Status.VULNERABILITY:
                 verdict = Verdict(

src/coretrace_python/cache.py

@@ -42,7 +42,7 @@
 from coretrace_python.semantic.symbols import SymbolId
 from coretrace_python.source import FileLocation, Location, SourceId, SourceSpan
 
-CACHE_FORMAT = 14
+CACHE_FORMAT = 15
 
 
 @dataclass(frozen=True)
@@ -253,11 +253,13 @@ def _encode_call(call: ExternalCall) -> dict[str, Any]:
         "given": _encode_arguments(call.arguments),
         "argument_cleared": [_encode_cleared(c) for c in call.argument_cleared],
         "keyword_cleared": [_encode_cleared(c) for c in call.keyword_cleared],
+        "invoked_at": None if call.invoked_at is None else _encode_span(call.invoked_at),
     }
 
 
 def _decode_call(data: Mapping[str, Any]) -> ExternalCall:
     site = data["call_site"]
+    invoked_at = data["invoked_at"]
     return ExternalCall(
         SymbolId(_string(data["symbol"])),
         tuple(_indices(deps) for deps in data["arguments"]),
@@ -267,6 +269,7 @@ def _decode_call(data: Mapping[str, Any]) -> ExternalCall:
         _decode_arguments(data["given"]),
         tuple(_decode_cleared(c) for c in data["argument_cleared"]),
         tuple(_decode_cleared(c) for c in data["keyword_cleared"]),
+        None if invoked_at is None else _decode_span(invoked_at),
     )