From 0a56b5429fad6724cb2371648a9c3b5af45730fd Mon Sep 17 00:00:00 2001 From: pranav-afk Date: Sun, 12 Jul 2026 12:25:05 +0530 Subject: [PATCH] fix: reject submit-only XSS false positives Strengthen XSS validation in skills and system prompt so form HTTP 200 is not treated as confirmed XSS. Guard create_vulnerability_report with a submit-only heuristic. Closes #745. --- strix/agents/prompts/system_prompt.jinja | 7 ++ strix/skills/vulnerabilities/xss.md | 29 ++++++- strix/tools/reporting/tool.py | 104 +++++++++++++++++++++++ tests/test_xss_report_guard.py | 50 +++++++++++ 4 files changed, 186 insertions(+), 4 deletions(-) create mode 100644 tests/test_xss_report_guard.py diff --git a/strix/agents/prompts/system_prompt.jinja b/strix/agents/prompts/system_prompt.jinja index bfd0ee762..1325e8095 100644 --- a/strix/agents/prompts/system_prompt.jinja +++ b/strix/agents/prompts/system_prompt.jinja @@ -193,6 +193,13 @@ VALIDATION REQUIREMENTS: - A vulnerability is ONLY considered reported when a reporting agent uses create_vulnerability_report (or create_dependency_report for known-CVE dependency/supply-chain findings) with full details. Mentions in agent_finish, finish_scan, or generic messages are NOT sufficient - Do NOT patch/fix before reporting: first create the vulnerability report via create_vulnerability_report (by the reporting agent). Only after reporting is completed should fixing/patching proceed - DEDUPLICATION: The create_vulnerability_report tool uses LLM-based deduplication. If it rejects your report as a duplicate, DO NOT attempt to re-submit the same vulnerability. Accept the rejection and move on to testing other areas. The vulnerability has already been reported by another agent + +XSS / STORED XSS (STRICT — common false positive): +- Do NOT report XSS solely because a form or API accepted a payload (HTTP 200, "success", "request sent", stored without error). +- XSS requires a confirmed sink: the payload is rendered unsafely into a page/DOM the victim (or you as the victim role) can open, with evidence of script execution or an unambiguous executable context. +- Stored XSS: you MUST open/fetch the view that displays the stored data (admin queue, message detail, profile page, etc.). Speculating that "a manager will open the admin panel" without accessing that view is a false positive — do not file it. +- Black-box without access to the render path: log as unconfirmed candidate / continue recon; do not create_vulnerability_report for XSS until the render path is verified. +- When filing XSS, technical_analysis and poc_description MUST state the observed sink URL/view, how the payload appeared in HTML/DOM, and how execution was confirmed (browser, agent-browser, or response DOM proof). diff --git a/strix/skills/vulnerabilities/xss.md b/strix/skills/vulnerabilities/xss.md index f9e3dd087..23c88ab38 100644 --- a/strix/skills/vulnerabilities/xss.md +++ b/strix/skills/vulnerabilities/xss.md @@ -170,17 +170,38 @@ Keep a compact set tuned per context: ## Validation -1. Provide minimal payload and context (sink type) with before/after DOM or network evidence -2. Demonstrate cross-browser execution where relevant or explain parser-specific behavior -3. Show bypass of stated defenses (sanitizer settings, CSP/Trusted Types) with proof -4. Quantify impact beyond alert: data accessed, action performed, persistence achieved +XSS is only confirmed when a **sink** executes (or would execute) attacker-controlled script +in a victim browser context. Input acceptance alone is never enough. + +**Required evidence (at least one):** + +1. **Reflected XSS** — response body / DOM contains the payload unescaped in a scriptable + context, **and** browser/agent-browser shows script execution (or a solid DOM proof of + an executable sink, e.g. unescaped payload inside a scriptable attribute/`innerHTML` path). +2. **Stored XSS** — you **open a page that renders the stored value** (victim view, admin + panel, notification UI, email preview, etc.) and observe the same: unsafe reflection + + execution evidence. Do **not** invent “when an admin opens X” without accessing that view. +3. **DOM XSS** — instrument or step through source→sink; show the source value reaches a + dangerous sink without encoding. + +Document: minimal payload, sink type/context, request + response (or screenshot/DOM), and +impact beyond `alert` when possible. + +**Not sufficient as confirmation:** + +- HTTP 200 / “success” / “request sent” after POSTing a payload into a form +- Server storing the string without ever viewing a render path that includes it +- Black-box speculation that staff UIs “probably” render HTML +- Payload present only in JSON/logs with proper encoding or `Content-Type` that is not HTML ## False Positives +- **Form accepted HTML/JS payload (HTTP 200) with no observed render/execution path** - Reflected content safely encoded in the exact context - CSP with nonces/hashes and no inline/event handlers - Trusted Types enforced on sinks; DOMPurify in strict mode with URI allowlists - Scriptable contexts disabled (no HTML pass-through, safe URL schemes enforced) +- Content stored but only shown escaped, or only to the same attacker-controlled client ## Impact diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index b6f284ca9..1569205ba 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -154,6 +154,92 @@ def _calculate_cvss(breakdown: dict[str, str]) -> tuple[float, str, str]: _VALID_FIX_EFFORT = frozenset({"trivial", "low", "medium", "high"}) +# Heuristic guard for the common false positive in #745: treating form/API +# acceptance of a script payload as confirmed XSS without a render sink. +_XSS_CLAIM_MARKERS = ( + "xss", + "cross-site scripting", + "cross site scripting", + "cwe-79", + "stored xss", + "reflected xss", + "dom xss", +) +_XSS_SUBMIT_ONLY_MARKERS = ( + "successfully submitting", + "successfully submitted", + "confirmed by successfully", + "responded with http 200", + "responded with 200", + "http 200", + "status 200", + "success message", + "request sent", + "accepted the payload", + "accepted and stores", + "accepts and stores", + "without any input sanitization", +) +_XSS_SINK_EVIDENCE_MARKERS = ( + "script execution", + "executed in", + "execution confirmed", + "onerror fired", + "payload executed", + "browser confirmed", + "agent-browser", + "opened the page", + "opened the admin", + "opened the view", + "viewed the stored", + "viewed the admin", + "response body contained", + "page source contained", + "dom contained", + "reflected unescaped", + "rendered unescaped", + "appeared unescaped", + "innerhtml", + "document.write", +) + + +def _looks_like_xss_submit_only_false_positive( + *, + title: str, + description: str, + technical_analysis: str, + poc_description: str, + evidence: str = "", + cwe: str | None, +) -> str | None: + """Return an error message when XSS is claimed from submit-success alone.""" + blob = " ".join( + [ + title, + description, + technical_analysis, + poc_description, + evidence, + cwe or "", + ] + ).lower() + claims_xss = any(marker in blob for marker in _XSS_CLAIM_MARKERS) + if not claims_xss: + return None + submit_only = any(marker in blob for marker in _XSS_SUBMIT_ONLY_MARKERS) + has_sink = any(marker in blob for marker in _XSS_SINK_EVIDENCE_MARKERS) + if submit_only and not has_sink: + return ( + "XSS not confirmed: evidence only shows form/API acceptance " + "(e.g. HTTP 200 / success after submit), not a render sink. " + "Open the page that displays the stored/reflected value, confirm " + "unsafe reflection or script execution, then re-file with that " + "sink URL/view and observation. Do not report speculated admin " + "impact without accessing that view." + ) + return None + async def _do_create( # noqa: PLR0912 *, @@ -224,6 +310,17 @@ async def _do_create( # noqa: PLR0912 if cwe_err: errors.append(cwe_err) + xss_fp = _looks_like_xss_submit_only_false_positive( + title=title, + description=description, + technical_analysis=technical_analysis, + poc_description=poc_description, + evidence=evidence, + cwe=cwe, + ) + if xss_fp: + errors.append(xss_fp) + if errors: return {"success": False, "error": "Validation failed", "errors": errors} @@ -355,6 +452,11 @@ async def create_vulnerability_report( dynamically PoC'd — a vulnerable dependency version pinned in a lockfile/manifest that matches a published advisory. File those with ``create_dependency_report`` instead, never with this tool. + - **XSS / stored XSS** where the only evidence is that a form or API + accepted a script payload (HTTP 200 / success message). That is not + XSS. You must have opened a page/view that **renders** the data and + confirmed unsafe reflection or script execution. Do not invent + admin-panel impact you never observed. Automatic LLM-based **deduplication** rejects reports that describe the same root cause on the same asset as an existing report. If you @@ -461,6 +563,8 @@ async def create_vulnerability_report( target: Affected URL / domain / repository. technical_analysis: The mechanism and root cause. poc_description: Step-by-step reproduction (steps only, no code). + For XSS: include the sink URL/view opened and how execution was + confirmed — not only the submit request. poc_script_code: Working PoC (Python preferred). remediation_steps: Specific, actionable fix (prose, no code). evidence: Concrete proof the issue is real and exploitable — diff --git a/tests/test_xss_report_guard.py b/tests/test_xss_report_guard.py new file mode 100644 index 000000000..6f0ffa167 --- /dev/null +++ b/tests/test_xss_report_guard.py @@ -0,0 +1,50 @@ +"""Tests for XSS submit-only false-positive rejection in reporting.""" + +from __future__ import annotations + +from strix.tools.reporting.tool import _looks_like_xss_submit_only_false_positive + + +def test_rejects_submit_success_as_stored_xss() -> None: + err = _looks_like_xss_submit_only_false_positive( + title="Stored XSS in contact form", + description="The form accepts raw HTML without sanitization.", + technical_analysis=( + "The vulnerability was confirmed by successfully submitting a request " + "with payloads. The server responded with " + "HTTP 200 and a success message: Request sent." + ), + poc_description="POST the payload to the form and receive success.", + cwe="CWE-79", + ) + assert err is not None + assert "not confirmed" in err.lower() + + +def test_allows_xss_with_sink_observation() -> None: + err = _looks_like_xss_submit_only_false_positive( + title="Stored XSS in admin request detail", + description="Payload executes when admin opens the request.", + technical_analysis=( + "Submitted payload via contact form (HTTP 200). Opened the admin " + "request detail page; page source contained the payload unescaped " + "and agent-browser confirmed script execution via onerror." + ), + poc_description=( + "1. Submit payload. 2. Open /admin/requests/123. " + "3. Observed payload appeared unescaped and script execution." + ), + cwe="CWE-79", + ) + assert err is None + + +def test_non_xss_findings_unaffected() -> None: + err = _looks_like_xss_submit_only_false_positive( + title="SQL Injection in login", + description="Auth bypass via SQLi.", + technical_analysis="Server responded with HTTP 200 and dumped rows.", + poc_description="Successfully submitted ' OR 1=1--", + cwe="CWE-89", + ) + assert err is None