fix(security): defuse CSV formula injection in multiline exécution cell + HTML-escape Markdown table cells
Finding 1 — CSV multiline formula injection: - Split _format_execution into _format_execution_text (MD/PDF, no sanitization) and _format_execution_csv (CSV, applies _csv_safe to each user-controlled component before join) - Moved _CSV_FORMULA_TRIGGERS + _csv_safe above the format helpers (required by _format_execution_csv) - Outer _csv_safe on the Exécution cell retained as belt-and-braces for the empty-date case - New test: test_render_engagement_csv_defuses_formula_in_inner_execution_lines Finding 2 — Stored XSS in Markdown table: - _cell() in render_engagement_markdown now calls _html_escape() (quote=True, default) before pipe-escaping and \n→<br/> substitution — correct order preserved - New test: test_render_engagement_markdown_escapes_html_in_table_cells 255 → 257 passed, ruff clean, mypy clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -30,7 +30,36 @@ def _creator(obj: object) -> str:
|
||||
return getattr(cb, "username", "") or ""
|
||||
|
||||
|
||||
def _format_execution(sim: Simulation) -> str:
|
||||
# ---------------------------------------------------------------------------
|
||||
# CSV formula-injection defense (defined early — used by _format_execution_csv)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# \t and \r included: Excel auto-trims leading whitespace, so a tab/CR prefix still
|
||||
# reaches the formula parser in some sheet versions.
|
||||
_CSV_FORMULA_TRIGGERS = ("=", "+", "-", "@", "\t", "\r")
|
||||
|
||||
|
||||
def _csv_safe(value: object) -> object:
|
||||
"""Defuse spreadsheet formula injection by prefixing user-controlled cells.
|
||||
|
||||
Excel / LibreOffice / Google Sheets interpret cells starting with =, +, -, @,
|
||||
\\t or \\r as formulas. Since this CSV is the engagement handoff to SOC and is
|
||||
explicitly opened in a spreadsheet app, an authenticated red-team user could
|
||||
craft a simulation field that executes on the SOC analyst's machine. Prefixing
|
||||
with a single apostrophe forces the spreadsheet to treat the cell as text.
|
||||
"""
|
||||
if isinstance(value, str) and value and value[0] in _CSV_FORMULA_TRIGGERS:
|
||||
return "'" + value
|
||||
return value
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Execution cell helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _format_execution_text(sim: Simulation) -> str:
|
||||
"""Canonical 3-part execution concat for Markdown and PDF (no CSV sanitization)."""
|
||||
parts = [
|
||||
sim.executed_at.isoformat() if sim.executed_at else "",
|
||||
sim.commands or "",
|
||||
@@ -39,6 +68,17 @@ def _format_execution(sim: Simulation) -> str:
|
||||
return "\n".join(parts)
|
||||
|
||||
|
||||
def _format_execution_csv(sim: Simulation) -> str:
|
||||
"""Execution concat for CSV: each user-controlled component is formula-defused
|
||||
before joining so that inner lines starting with =, +, -, @ are safe."""
|
||||
parts = [
|
||||
sim.executed_at.isoformat() if sim.executed_at else "",
|
||||
str(_csv_safe(sim.commands or "")),
|
||||
str(_csv_safe(sim.execution_result or "")),
|
||||
]
|
||||
return "\n".join(parts)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Markdown
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -91,11 +131,16 @@ def render_engagement_markdown(
|
||||
lines.append(separator)
|
||||
|
||||
for sim in simulations:
|
||||
execution = _format_execution(sim).replace("\n", "<br/>")
|
||||
|
||||
def _cell(value: str | None) -> str:
|
||||
return (value or "").replace("|", "\\|").replace("\n", "<br/>")
|
||||
# Escape HTML (including quotes) first to prevent stored XSS in MD renderers
|
||||
# that interpret inline HTML, then escape pipe (GFM table syntax),
|
||||
# then fold newlines to <br/> (our own safe markup, inserted after escape).
|
||||
s = _html_escape(value or "")
|
||||
s = s.replace("|", "\\|")
|
||||
s = s.replace("\n", "<br/>")
|
||||
return s
|
||||
|
||||
execution = _format_execution_text(sim)
|
||||
row = "| " + " | ".join([
|
||||
_cell(sim.name),
|
||||
_cell(sim.description),
|
||||
@@ -125,24 +170,6 @@ _CSV_HEADERS = [
|
||||
"Cyber incident",
|
||||
]
|
||||
|
||||
# \t and \r included: Excel auto-trims leading whitespace, so a tab/CR prefix still
|
||||
# reaches the formula parser in some sheet versions.
|
||||
_CSV_FORMULA_TRIGGERS = ("=", "+", "-", "@", "\t", "\r")
|
||||
|
||||
|
||||
def _csv_safe(value: object) -> object:
|
||||
"""Defuse spreadsheet formula injection by prefixing user-controlled cells.
|
||||
|
||||
Excel / LibreOffice / Google Sheets interpret cells starting with =, +, -, @,
|
||||
\\t or \\r as formulas. Since this CSV is the engagement handoff to SOC and is
|
||||
explicitly opened in a spreadsheet app, an authenticated red-team user could
|
||||
craft a simulation field that executes on the SOC analyst's machine. Prefixing
|
||||
with a single apostrophe forces the spreadsheet to treat the cell as text.
|
||||
"""
|
||||
if isinstance(value, str) and value and value[0] in _CSV_FORMULA_TRIGGERS:
|
||||
return "'" + value
|
||||
return value
|
||||
|
||||
|
||||
def render_engagement_csv(
|
||||
_engagement: Engagement, simulations: list[Simulation]
|
||||
@@ -152,13 +179,13 @@ def render_engagement_csv(
|
||||
writer.writerow(_CSV_HEADERS)
|
||||
|
||||
for sim in simulations:
|
||||
execution = _format_execution(sim)
|
||||
execution = _format_execution_csv(sim)
|
||||
writer.writerow([
|
||||
_csv_safe(sim.name or ""),
|
||||
_csv_safe(sim.description or ""),
|
||||
_csv_safe(sim.log_source or ""),
|
||||
_csv_safe(sim.soc_comment or ""),
|
||||
_csv_safe(execution),
|
||||
_csv_safe(execution), # belt-and-braces: outer check covers empty executed_at case
|
||||
_csv_safe(sim.logs or ""),
|
||||
_csv_safe(sim.incident_number or ""),
|
||||
])
|
||||
@@ -217,7 +244,7 @@ def _render_engagement_html(
|
||||
thead = "<thead><tr>" + "".join(f"<th>{h(col)}</th>" for col in _HTML_HEADERS) + "</tr></thead>"
|
||||
parts.append(f"<table>{thead}<tbody>")
|
||||
for sim in simulations:
|
||||
execution_html = h(_format_execution(sim)).replace("\n", "<br/>")
|
||||
execution_html = h(_format_execution_text(sim)).replace("\n", "<br/>")
|
||||
cells = [
|
||||
h(sim.name or ""),
|
||||
h(sim.description or ""),
|
||||
|
||||
@@ -220,6 +220,23 @@ def test_render_engagement_csv_escapes_formula_injection_in_execution(app) -> No
|
||||
assert "HYPERLINK" in cells[4]
|
||||
|
||||
|
||||
def test_render_engagement_csv_defuses_formula_in_inner_execution_lines(app) -> None:
|
||||
"""When executed_at is set, the cell starts with a safe date, but commands
|
||||
line may inject formulas. Each user-controlled component must be defused."""
|
||||
with app.app_context():
|
||||
eng = _make_engagement()
|
||||
sim = _make_sim(
|
||||
executed_at=datetime(2026, 6, 1, 10, 0, tzinfo=UTC),
|
||||
commands="=cmd|'/c calc'!A1",
|
||||
execution_result="@SUM(1)",
|
||||
)
|
||||
result = render_engagement_csv(eng, [sim])
|
||||
cells = list(_csv.reader(_io.StringIO(result)))[1]
|
||||
execution_cell = cells[4] # Exécution column
|
||||
assert "'=cmd|'/c calc'!A1" in execution_cell
|
||||
assert "'@SUM(1)" in execution_cell
|
||||
|
||||
|
||||
def test_render_engagement_csv_does_not_alter_safe_strings(app) -> None:
|
||||
with app.app_context():
|
||||
eng = _make_engagement()
|
||||
@@ -230,6 +247,24 @@ def test_render_engagement_csv_does_not_alter_safe_strings(app) -> None:
|
||||
assert "whoami /all" in cells[4]
|
||||
|
||||
|
||||
def test_render_engagement_markdown_escapes_html_in_table_cells(app) -> None:
|
||||
"""User content in table cells must be HTML-escaped to prevent stored XSS
|
||||
when the .md is opened in a renderer that interprets inline HTML."""
|
||||
with app.app_context():
|
||||
eng = _make_engagement()
|
||||
sim = _make_sim(
|
||||
name="<script>alert(1)</script>",
|
||||
commands='<img src=x onerror="alert(1)">',
|
||||
)
|
||||
result = render_engagement_markdown(eng, [sim])
|
||||
assert "<script>" not in result
|
||||
assert 'onerror="alert' not in result
|
||||
assert "<script>" in result
|
||||
assert "<img" in result
|
||||
# double-quotes in attribute values are also escaped
|
||||
assert """ in result
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# PDF tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user