feat(code-injection): report import directories chosen by input

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`. |
+| `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. |
 | `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

@@ -74,6 +74,12 @@ class PythonStdlibModels(ModelPlugin):
                 ("runpy.run_path", "path_name"),
             )
         ),
+        # The directory imports are looked up in, and the site directory whose ``.pth``
+        # files run; the index of ``insert`` is data (#240).
+        Sink(_sym("sys.path.insert"), TaintKind.CODE, ((TaintKind.CODE, (1,)),), selects=True),
+        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),
         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,13 +1,17 @@
 """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, or the file ``runpy.run_path`` runs. ``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, 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.
 
 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
 how far it goes depends on what the input can reach. Their findings are titled for
 that. A constant package prefix (``"app.plugins." + name``) confines an import to the
-modules of that package: a hotspot."""
+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."""
 
 from __future__ import annotations
 
@@ -28,6 +32,8 @@
     for name in ("importlib.import_module", "builtins.__import__", "importlib.__import__", "runpy.run_module")
 )
 _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")
 
 
 class CodeInjectionPlugin(TaintDetector):
@@ -44,6 +50,24 @@ def assess(
         symbol = flow.sink.symbol
         if symbol == _RUN_PATH:
             return Assessment(verdict, self.severity, "Untrusted script execution")
+        if symbol in _IMPORT_PATHS:
+            if verdict.status is Status.VULNERABILITY:
+                verdict = Verdict(
+                    flow,
+                    Status.HOTSPOT,
+                    "the input chooses a directory imports are looked up in: code runs only when a later "
+                    "import resolves to a module the attacker placed there",
+                )
+            return Assessment(verdict, self.severity, "Untrusted import path")
+        if symbol == _SITE_DIRECTORY:
+            if verdict.status is Status.VULNERABILITY:
+                verdict = Verdict(
+                    flow,
+                    Status.VULNERABILITY,
+                    f"{verdict.evidence}; the import lines of the .pth files of the directory run during the "
+                    "call: exploitable when the input chooses a directory holding files the attacker controls",
+                )
+            return Assessment(verdict, self.severity, "Untrusted site directory")
         if symbol not in _IMPORTS:
             return Assessment(verdict, self.severity)
         if verdict.status is Status.VULNERABILITY and flow.through is None:

src/coretrace_python/taint/models.py

@@ -66,9 +66,9 @@ class Sink:
     ``keywords`` restrict some kinds to the arguments they list: a SQL statement is the
     first argument of ``execute``, its parameter tuple is not a statement; the target of
     ``requests.get`` is its first argument or ``url=``, its body is no destination.
-    ``selects`` marks a sink whose argument chooses by name the code that runs, a module
-    or a file: only a check that limits the value to constants proves it, not a check of
-    its shape (#240)."""
+    ``selects`` marks a sink whose argument chooses by name the code that runs, a module,
+    a file or a directory code is loaded from: only a check that limits the value to
+    constants proves it, not a check of its shape (#240)."""
 
     symbol: SymbolId
     kinds: TaintKind

tests/test_import_path_sinks.py

@@ -0,0 +1,108 @@
+"""Acceptance tests for issue #240, part 2: the directories imports are looked up in.
+
+``sys.path.insert``, ``append`` and ``extend`` let the input choose a directory imports
+are looked up in: code runs only when a later import resolves to the attacker's module
+there, so the finding is a hotspot, ``Untrusted import path``. ``site.addsitedir`` runs
+the import lines of the directory's ``.pth`` files during the call: a vulnerability,
+``Untrusted site directory``, exploitable when that directory holds files the attacker
+controls, which the finding states. As for the import sinks, only a check that limits
+the directory to constants proves it, and the environment and the command line reach
+these sinks as they reach ``eval``. Writes to ``sys.path`` (slice assignment, ``+=``,
+rebinding) are not calls and are not covered.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from coretrace_python import engine
+from coretrace_python.findings import Finding
+from coretrace_python.source import SourceManager
+
+HEADER = "import os\nimport site\nimport sys\nfrom sys import path\n\n\ndef run():\n    d = input()\n"
+
+
+def injections(body: str) -> list[Finding]:
+    text = HEADER + "".join(f"    {line}\n" for line in body.splitlines())
+    findings = engine.check(SourceManager().add_source("app.py", text), [engine.BUNDLED_PLUGINS])
+    return [f for f in findings if f.rule_id == "code-injection"]
+
+
+def verdict(finding: Finding) -> tuple[str, str, str]:
+    return finding.metadata["verdict"], finding.confidence.value, finding.message.split(":", 1)[0]
+
+
+@pytest.mark.parametrize(
+    "call",
+    [
+        "sys.path.insert(0, d)",
+        "sys.path.append(d)",
+        "sys.path.extend([d])",
+        "path.insert(0, d)",
+    ],
+)
+def test_an_import_directory_the_input_chooses_is_a_hotspot(call: str) -> None:
+    (finding,) = injections(call)
+
+    assert verdict(finding) == ("hotspot", "medium", "Untrusted import path")
+    assert "later import" in finding.metadata["evidence"]
+
+
+@pytest.mark.parametrize("call", ["site.addsitedir(d)", "site.addsitedir(sitedir=d)"])
+def test_a_site_directory_the_input_chooses_is_a_vulnerability_stating_its_condition(call: str) -> None:
+    (finding,) = injections(call)
+
+    assert verdict(finding) == ("vulnerability", "high", "Untrusted site directory")
+    assert ".pth" in finding.metadata["evidence"]
+    assert "attacker" in finding.metadata["evidence"]
+
+
+@pytest.mark.parametrize(
+    "call",
+    [
+        # The index is not a directory.
+        "sys.path.insert(int(d), '/srv/lib')",
+        "sys.path.insert(len(d), '/srv/lib')",
+        # Constants and the project's own directories.
+        "sys.path.insert(0, '/srv/lib')",
+        "sys.path.append(os.path.join(os.path.dirname(__file__), 'lib'))",
+        "site.addsitedir('/srv/site')",
+    ],
+)
+def test_a_directory_the_input_does_not_choose_is_not_reported(call: str) -> None:
+    assert injections(call) == []
+
+
+@pytest.mark.parametrize(
+    ("call", "expected"),
+    [
+        ("sys.path.insert(0, d)", ("hotspot", "medium", "Untrusted import path")),
+        ("site.addsitedir(d)", ("hotspot", "medium", "Untrusted site directory")),
+    ],
+)
+@pytest.mark.parametrize("guard", ["d.isalnum()", "d.isidentifier()"])
+def test_a_check_of_the_shape_of_the_directory_does_not_prove_it(
+    call: str, expected: tuple[str, str, str], guard: str
+) -> None:
+    (finding,) = injections(f"if {guard}:\n    {call}")
+
+    assert verdict(finding) == expected
+
+
+@pytest.mark.parametrize("call", ["sys.path.insert(0, d)", "site.addsitedir(d)"])
+@pytest.mark.parametrize("guard", ["d in ('/srv/a', '/srv/b')", "d == '/srv/a'"])
+def test_a_check_against_constants_proves_the_directory(call: str, guard: str) -> None:
+    assert injections(f"if {guard}:\n    {call}") == []
+
+
+@pytest.mark.parametrize(
+    ("body", "expected"),
+    [
+        ("sys.path.insert(0, os.environ['PLUGIN_DIR'])", ("hotspot", "medium", "Untrusted import path")),
+        ("site.addsitedir(sys.argv[1])", ("vulnerability", "high", "Untrusted site directory")),
+    ],
+)
+def test_the_environment_and_the_command_line_reach_these_sinks(body: str, expected: tuple[str, str, str]) -> None:
+    (finding,) = injections(body)
+
+    assert verdict(finding) == expected