From b078ff3c0925fd41afb8b4f82bd2560d3567bf47 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 8 Oct 2026 02:02:14 +0200 Subject: [PATCH] feat(sarif): export the injection kind of command-injection results --- docs/usage.md | 2 +- src/coretrace_python/reporters/sarif.py | 6 +++ tests/test_sarif_injection.py | 58 +++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 tests/test_sarif_injection.py diff --git a/docs/usage.md b/docs/usage.md index ec318ad..1bb0417 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -115,7 +115,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) 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`. | +| `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 the only 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. | | `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. | diff --git a/src/coretrace_python/reporters/sarif.py b/src/coretrace_python/reporters/sarif.py index 9e32abf..fe17ea7 100644 --- a/src/coretrace_python/reporters/sarif.py +++ b/src/coretrace_python/reporters/sarif.py @@ -19,6 +19,9 @@ Severity.LOW: "note", Severity.INFO: "note", } +# The kinds of injection a finding may carry, the only metadata a result exports: other +# metadata, such as evidence, may quote what the analysed code holds. +_INJECTIONS = frozenset({"option", "command"}) def _artifact(report: Report, path: str) -> dict[str, str]: @@ -53,6 +56,9 @@ def _result( "message": {"text": finding.message}, "locations": [location], } + injection = finding.metadata.get("injection") + if injection in _INJECTIONS: + result["properties"] = {"injection": injection} if suppressed: result["suppressions"] = [{"kind": "inSource"}] if baseline_state is not None: diff --git a/tests/test_sarif_injection.py b/tests/test_sarif_injection.py new file mode 100644 index 0000000..ffc5a8f --- /dev/null +++ b/tests/test_sarif_injection.py @@ -0,0 +1,58 @@ +"""Acceptance tests: the kind of a command injection in the SARIF log (#220). + +A SARIF result carries the ``injection`` of its finding in its ``properties`` when the +rule established it, ``option`` or ``command``, and no ``injection`` when it could not. +No other metadata of a finding is exported there: some, such as evidence, may quote +what the analysed code holds. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from coretrace_python.cli import main + +CALLS = { + "option.py": 'subprocess.run(["ls", sys.argv[1]])', + "command.py": 'subprocess.run(["sh", "-c", sys.argv[1]])', + "unknown.py": 'subprocess.run(["python", "-m", sys.argv[1]])', + "shell.py": "os.system(sys.argv[1])", +} + + +@pytest.fixture +def results( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> dict[str, dict[str, object]]: + root = tmp_path / "proj" + root.mkdir() + for name, call in CALLS.items(): + (root / name).write_text( + f"import os\nimport subprocess\nimport sys\n\n\ndef run():\n {call}\n", + encoding="utf-8", + ) + monkeypatch.chdir(tmp_path) + main(["--check", "proj", "--format", "sarif"]) + log = json.loads(capsys.readouterr().out) + return { + r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"]: r + for r in log["runs"][0]["results"] + if r["ruleId"] == "command-injection" + } + + +def test_a_result_carries_the_injection_its_rule_established( + results: dict[str, dict[str, object]], +) -> None: + assert results["option.py"]["properties"] == {"injection": "option"} + assert results["command.py"]["properties"] == {"injection": "command"} + + +def test_a_result_whose_injection_is_not_established_has_none( + results: dict[str, dict[str, object]], +) -> None: + assert "properties" not in results["unknown.py"] + assert "properties" not in results["shell.py"]