diff --git a/strix/report/sarif.py b/strix/report/sarif.py index 49824d758..04ab59a35 100644 --- a/strix/report/sarif.py +++ b/strix/report/sarif.py @@ -1037,8 +1037,10 @@ def _primary_fingerprint( # keying on the title alone would collapse them onto one primary # fingerprint. _class_token resolves to the discriminating # structured field when present. + # A titled finding always carried this segment, even with an empty + # keyword, so keep it there to leave existing fingerprints unchanged. token = _class_token(report) - if token: + if token or _string_value(report.get("title")): parts.append(f"synth_class:{token}") composite = "|".join(parts) diff --git a/strix/skills/vulnerabilities/client_side_path_traversal.md b/strix/skills/vulnerabilities/client_side_path_traversal.md index 636b13feb..afecf4b31 100644 --- a/strix/skills/vulnerabilities/client_side_path_traversal.md +++ b/strix/skills/vulnerabilities/client_side_path_traversal.md @@ -41,7 +41,7 @@ Minimal shape of the primitive — a profile viewer whose id comes from the rout ```text app route: /users/ app builds: fetch(`/api/users/${id}`) -attacker id: ../../admin/settings (or ..%2f..%2fadmin%2fsettings) +attacker id: ../admin/settings (or ..%2fadmin%2fsettings) final req: GET /api/admin/settings <- traversal survived into the sink ``` @@ -109,7 +109,7 @@ Prove the impact-class-specific fact before calling it exploitable: File a confirmed CSPT as a vulnerability report with `finding_class="client_side_path_traversal"` and `cwe="CWE-22"`. It shares CWE-22 with server-side traversal, so the class is what keeps it a separate finding. -- Give it a discriminator: `endpoint` set to the traversed target path the victim's request reached (e.g. `/api/admin/keys`) plus its `method`, or a `code_location` with a repo-relative `file` and `start_line` for the client sink. +- Give it a discriminator: `endpoint` set to the page route that carries the attacker-controlled source (e.g. `/invoice` for `/invoice?doc=`) plus its `method`, or a `code_location` with a repo-relative `file` and `start_line` for the client sink. Keep the destination your payload reached out of `endpoint`, so the bug keeps one identity whichever destination you demonstrate. - Use this class when page code builds the redirected request from browser input, including a request that lands on a BFF route that forwards it. When the server builds the traversed path itself from a parameter, it is server-side traversal or SSRF: leave `finding_class` as `dynamic`. - Put the source value, the final request (method + path), the paired control, and the observed effect in the PoC and evidence. @@ -117,7 +117,7 @@ File a confirmed CSPT as a vulnerability report with `finding_class="client_side - Decode/normalization of `../` with no request sink downstream — the value never shapes a request. - A sink whose redirected request has no security-relevant effect (static asset, idempotent read of already-authorized data, dead/404 path). -- Same-origin-only redirection to an equally-authorized endpoint — the principal could already call it directly. +- Redirection to an endpoint where the victim's session adds nothing: a public or idempotent read whose response the attacker never sees. A state-changing request sent with the victim's session is still a finding, even though the victim could call it directly. - A framework that re-encodes or segment-normalizes before the sink, so the traversal does not survive into the final request. ## Pro Tips diff --git a/strix/tools/reporting/tool.py b/strix/tools/reporting/tool.py index 7a9e54210..bb7ac98de 100644 --- a/strix/tools/reporting/tool.py +++ b/strix/tools/reporting/tool.py @@ -872,7 +872,8 @@ async def _do_create( errors.append( "finding_class 'client_side_path_traversal' needs a discriminator SARIF " "can anchor, or distinct CSPT findings collapse onto one fingerprint: set " - "endpoint (the traversed target path, e.g. '/admin/keys') and method, or a " + "endpoint (the page route that carries the attacker-controlled source, e.g. " + "'/invoice', not the path the payload reached) and method, or a " "code_location with a repo-relative file and start line (not an absolute or " "drive path, which SARIF drops)." ) @@ -1453,9 +1454,10 @@ async def create_vulnerability_report( class; setting this keeps it separated in the report artifacts and SARIF (do NOT use it for server-side traversal — leave those ``dynamic``). A CSPT report must - also carry a discriminator: ``endpoint`` (the traversed - target path) and ``method``, or a ``code_location`` for the - client sink. See the ``client_side_path_traversal`` skill. + also carry a discriminator: ``endpoint`` (the page route + that carries the attacker-controlled source, not the path + the payload reached) and ``method``, or a ``code_location`` + for the client sink. See the ``client_side_path_traversal`` skill. """ ( http_exchange_ids, diff --git a/tests/test_cspt_finding_class.py b/tests/test_cspt_finding_class.py index 5081dd368..0913bfdfe 100644 --- a/tests/test_cspt_finding_class.py +++ b/tests/test_cspt_finding_class.py @@ -8,6 +8,7 @@ pure-Python, no browser or network, so they run in ordinary CI. from __future__ import annotations +import hashlib from typing import TYPE_CHECKING, Any import pytest @@ -62,7 +63,7 @@ async def _create(**overrides: Any) -> dict[str, Any]: "severity_change_conditions": "A CSRF token on the sink would lower it.", "fix_effort": "low", "cvss_breakdown": _CVSS, - "endpoint": "/api/admin/keys", + "endpoint": "/invoice", "method": "GET", "cve": None, "cwe": "CWE-22", @@ -145,7 +146,7 @@ async def test_create_accepts_a_cspt_finding_with_an_endpoint(report_state: Repo assert result["success"] is True, result stored = report_state.vulnerability_reports[0] assert stored["finding_class"] == "client_side_path_traversal" - assert stored["endpoint"] == "/api/admin/keys" + assert stored["endpoint"] == "/invoice" async def test_create_reserves_dependency_cve_for_the_dependency_tool( @@ -181,18 +182,18 @@ async def test_cspt_finding_takes_proof_revisions_like_a_dynamic_one( result = _do_update( report_id=report_id, - update_reason="The traversal reaches the key-rotation endpoint, not the listing.", + update_reason="The source is the billing invoice route, not the legacy one.", fields={ - "endpoint": "/api/admin/keys/rotate", - "method": "POST", - "poc_script_code": "POST /invoice?doc=..%2Fadmin%2Fkeys%2Frotate", + "endpoint": "/billing/invoice", + "method": "GET", + "poc_script_code": "GET /billing/invoice?doc=..%2Fadmin%2Fkeys", "cvss_breakdown": {**_CVSS, "availability": "H"}, }, ) assert result["success"] is True, result stored = report_state.vulnerability_reports[0] - assert stored["endpoint"] == "/api/admin/keys/rotate" + assert stored["endpoint"] == "/billing/invoice" assert stored["cvss_breakdown"]["availability"] == "H" assert stored["finding_class"] == "client_side_path_traversal" assert "dependency_metadata" not in stored @@ -220,3 +221,12 @@ def test_dedupe_comparison_includes_finding_class() -> None: ) assert cleaned.get("finding_class") == "client_side_path_traversal" + + +def test_untitled_keyword_keeps_the_pre_class_synthetic_fingerprint() -> None: + """A titled finding whose title yields no keyword (e.g. written in Japanese) + keeps the fingerprint it had before finding_class existed.""" + report = {"title": "反射型クロスサイトスクリプティング", "finding_class": "dynamic"} + expected = hashlib.sha256(b"rule:CWE-79|uri:SECURITY.md|synth_class:").hexdigest() + + assert _primary_fingerprint("CWE-79", report, _SECURITY_MD, is_synthetic=True) == expected