fix(report): key a CSPT finding on its source page, not the payload target

The CSPT discriminator told agents to set endpoint to the path their
payload reached. One bug demonstrated against two destinations then
got two SARIF identities. Point endpoint at the page route carrying the
attacker-controlled source, in the validator message, the tool
docstring and the skill.

Also in the skill: the worked example used two ../ segments, which
resolve to /admin/settings rather than /api/admin/settings, and a
false-positive rule would have closed a state-changing request sent
with the victim's session.

SARIF: a titled locationless finding whose title yields no class
keyword (e.g. a non-English title) kept an empty synth_class segment on
main; keep it so its fingerprint does not change.
This commit is contained in:
Abdelrahman Mostafa 2026-10-22 16:17:43 +03:00
parent c804111e06
commit d49130e764
4 changed files with 29 additions and 15 deletions

View file

@ -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)

View file

@ -41,7 +41,7 @@ Minimal shape of the primitive — a profile viewer whose id comes from the rout
```text
app route: /users/<id>
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

View file

@ -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,

View file

@ -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