mirror of
https://github.com/usestrix/strix.git
synced 2026-11-14 09:01:25 +00:00
fix(sarif): canonicalise vulnerability class for CWE rule ids
Greptile P1 on #1618: rewordings of the same finding diverged in rule identity. 'SQL Injection in get_user' qualified to CWE-89/sql-injection, while 'SQLi via string-formatted query in get_user' matched no curated keyword and fell back to bare CWE-89, so a consumer keying alerts on (ruleId, location) would treat the re-scan as a new alert and lose dismissal carryover (partialFingerprints already key on the bare CWE, so they could not preserve rule identity). _curated_class_keyword() now resolves equivalent class names to one canonical keyword via a closed _CLASS_ALIASES map (sqli -> sql injection, idor -> insecure direct object reference, directory traversal -> path traversal, redos -> regex denial of service, cross-site scripting -> xss, and other unambiguous acronym/full-form synonyms), with aliases matched at their canonical keyword's catalogue position and titles normalised over hyphens/underscores. 'regex denial of service' now precedes 'denial of service' in the catalogue, as the precise-before-fuzzy ordering intends. Near-synonyms that may denote distinct sub-classes (bola vs insecure direct object reference, weak random vs insecure random, auth bypass vs missing authentication) are deliberately not aliased, so distinct classes sharing a CWE still get distinct rules. The same canonicalisation also aligns class fingerprints for reworded titles. AI assistance disclosure: this change was implemented with AI assistance; the bot's claim was reproduced before editing, and the new regression tests plus the existing SARIF/report suites were run against the change. Signed-off-by: Bharath Vemula <vemula.sai8@gmail.com>
This commit is contained in:
Bharath Vemula
parent
3b7b7e4e7e
commit
32b1acf6bd
2 changed files with 143 additions and 14 deletions
|
|
@ -874,7 +874,9 @@ def _rule_id(report: dict[str, Any]) -> str:
|
|||
vulnerability-class slug (``CWE-639/insecure-direct-object-reference``)
|
||||
so distinct classes that share one CWE do not collapse into a single
|
||||
rule. The slug comes from the curated class-keyword catalogue only —
|
||||
never from the raw LLM title — and findings with no curated keyword
|
||||
never from the raw LLM title — and equivalent class names resolve
|
||||
to one canonical keyword (``_CLASS_ALIASES``), so rewordings of the
|
||||
same class keep the same rule id. Findings with no curated keyword
|
||||
keep the bare ``CWE-NNN`` id, so rule ids stay stable across
|
||||
title rewordings.
|
||||
"""
|
||||
|
|
@ -929,7 +931,9 @@ def _class_rule_slug(report: dict[str, Any]) -> str | None:
|
|||
Uses only the closed class-keyword catalogue (the same notion of
|
||||
class ``_class_fingerprint`` uses), never the raw title: a title
|
||||
with no curated keyword yields None and the rule keeps its bare
|
||||
CWE id rather than gaining a prose-dependent suffix.
|
||||
CWE id rather than gaining a prose-dependent suffix. Equivalent
|
||||
names map to their canonical keyword first (``_CLASS_ALIASES``),
|
||||
so e.g. "SQLi" yields the "sql injection" slug, not a bare id.
|
||||
"""
|
||||
title = _string_value(report.get("title")) or ""
|
||||
keyword = _curated_class_keyword(title)
|
||||
|
|
@ -941,7 +945,8 @@ def _class_rule_slug(report: dict[str, Any]) -> str | None:
|
|||
# Vulnerability-class keywords for the file-independent class
|
||||
# fingerprint. Order matters — first match wins, so precise terms
|
||||
# come before fuzzy ones (e.g. "broken access control" before
|
||||
# "access control"). Keep this list closed and curated; a future
|
||||
# "access control", "regex denial of service" before
|
||||
# "denial of service"). Keep this list closed and curated; a future
|
||||
# maintainer adding sloppy entries could collapse distinct findings
|
||||
# to the same class hash.
|
||||
_VULN_CLASS_KEYWORDS = (
|
||||
|
|
@ -958,7 +963,6 @@ _VULN_CLASS_KEYWORDS = (
|
|||
"session fixation",
|
||||
"open redirect",
|
||||
"path traversal",
|
||||
"directory traversal",
|
||||
"command injection",
|
||||
"sql injection",
|
||||
"code injection",
|
||||
|
|
@ -978,7 +982,6 @@ _VULN_CLASS_KEYWORDS = (
|
|||
"toctou",
|
||||
"information disclosure",
|
||||
"insecure direct object reference",
|
||||
"idor",
|
||||
"bola",
|
||||
"bfla",
|
||||
"cross-tenant",
|
||||
|
|
@ -986,20 +989,61 @@ _VULN_CLASS_KEYWORDS = (
|
|||
"tenant bypass",
|
||||
"auth bypass",
|
||||
"rate limiting",
|
||||
"rate limit",
|
||||
"weak cryptography",
|
||||
"weak hash",
|
||||
"weak random",
|
||||
"insecure random",
|
||||
"tls verification",
|
||||
"certificate verification",
|
||||
"denial of service",
|
||||
"regex denial of service",
|
||||
"redos",
|
||||
"denial of service",
|
||||
"supply chain",
|
||||
)
|
||||
|
||||
|
||||
# Equivalent names for a canonical class, keyed by that canonical
|
||||
# keyword. A rule id (and a class fingerprint) must be a property of
|
||||
# the vulnerability class, not of the LLM's phrasing of it: "SQLi",
|
||||
# "directory traversal", "IDOR", "ReDoS" and "cross-site scripting"
|
||||
# are the same classes as "sql injection", "path traversal",
|
||||
# "insecure direct object reference", "regex denial of service" and
|
||||
# "xss", so they must resolve to the same canonical keyword — and
|
||||
# hence the same rule id — rather than to a bare CWE id or a
|
||||
# different slug. Only unambiguous acronyms / full-form synonyms
|
||||
# belong here. Near-synonyms that may denote distinct sub-classes
|
||||
# (e.g. "bola" vs "insecure direct object reference", "weak random"
|
||||
# vs "insecure random", "auth bypass" vs "missing authentication")
|
||||
# are deliberately NOT aliased, so distinct classes sharing one CWE
|
||||
# still get distinct rules.
|
||||
_CLASS_ALIASES: dict[str, tuple[str, ...]] = {
|
||||
"sql injection": ("sqli",),
|
||||
"command injection": ("cmd injection",),
|
||||
"template injection": ("ssti",),
|
||||
"xss": ("cross site scripting",),
|
||||
"csrf": ("cross site request forgery",),
|
||||
"ssrf": ("server side request forgery",),
|
||||
"xxe": ("xml external entity", "xml external entities"),
|
||||
"toctou": ("time of check time of use", "time of check to time of use"),
|
||||
"insecure direct object reference": ("idor",),
|
||||
"path traversal": ("directory traversal",),
|
||||
"rate limiting": ("rate limit",),
|
||||
"regex denial of service": ("redos",),
|
||||
"open redirect": ("unvalidated redirect",),
|
||||
"information disclosure": ("info disclosure",),
|
||||
}
|
||||
|
||||
|
||||
# (pattern, canonical keyword) pairs in match order: each canonical
|
||||
# keyword's aliases sit at that keyword's own position, so an alias
|
||||
# and its canonical form have identical priority against every other
|
||||
# class. First match wins, as in the catalogue itself.
|
||||
_CLASS_MATCH_PATTERNS: tuple[tuple[str, str], ...] = tuple(
|
||||
(pattern, canonical)
|
||||
for canonical in _VULN_CLASS_KEYWORDS
|
||||
for pattern in (*_CLASS_ALIASES.get(canonical, ()), canonical)
|
||||
)
|
||||
|
||||
|
||||
def _primary_fingerprint(
|
||||
rule_id: str,
|
||||
report: dict[str, Any],
|
||||
|
|
@ -1109,18 +1153,24 @@ def _class_fingerprint(rule_id: str, report: dict[str, Any]) -> str | None:
|
|||
|
||||
|
||||
def _curated_class_keyword(title: str) -> str:
|
||||
"""First matching curated class keyword in ``title``, or "".
|
||||
"""Canonical curated class keyword for ``title``, or "".
|
||||
|
||||
The catalogue is closed and ordered (first match wins); this is
|
||||
the stable notion of "the finding's class" shared by fingerprints
|
||||
and CWE rule-id qualification.
|
||||
and CWE rule-id qualification. Equivalent names resolve to their
|
||||
canonical keyword (``_CLASS_ALIASES``), so rewordings of the same
|
||||
class — "SQL Injection" vs "SQLi", "IDOR" vs "insecure direct
|
||||
object reference" — return the same value instead of diverging
|
||||
into a qualified id and a bare CWE id. Titles are normalised
|
||||
(hyphens/underscores become spaces) before matching, so
|
||||
"sql-injection" and "cross-site scripting" match too.
|
||||
"""
|
||||
if not title:
|
||||
return ""
|
||||
lower = title.lower()
|
||||
for kw in _VULN_CLASS_KEYWORDS:
|
||||
if kw in lower:
|
||||
return kw
|
||||
normalised = re.sub(r"\s+", " ", title.lower().replace("-", " ").replace("_", " ")).strip()
|
||||
for pattern, canonical in _CLASS_MATCH_PATTERNS:
|
||||
if pattern in normalised:
|
||||
return canonical
|
||||
return ""
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -458,3 +458,82 @@ def test_class_qualified_rule_keeps_bare_cwe_fingerprints(tmp_path: Path) -> Non
|
|||
fp_a = _read(a)["runs"][0]["results"][0]["partialFingerprints"]
|
||||
fp_b = _read(b)["runs"][0]["results"][0]["partialFingerprints"]
|
||||
assert fp_a == fp_b
|
||||
|
||||
|
||||
def _rule_and_location(run_dir: Path) -> tuple[str, dict[str, Any], dict[str, Any]]:
|
||||
result = _read(run_dir)["runs"][0]["results"][0]
|
||||
loc = result["locations"][0]["physicalLocation"]
|
||||
return result["ruleId"], loc, result["partialFingerprints"]
|
||||
|
||||
|
||||
def test_title_rewording_keeps_same_rule_id(tmp_path: Path) -> None:
|
||||
# Greptile P1 on #1618: "SQL Injection in get_user" and "SQLi via
|
||||
# string-formatted query in get_user" are the same bug, but the
|
||||
# first qualified to CWE-89/sql-injection while the second fell
|
||||
# back to bare CWE-89, so a consumer keying alerts on
|
||||
# (ruleId, location) would treat the re-scan as a new alert.
|
||||
# Both rewordings must now resolve to the same canonical class.
|
||||
a = tmp_path / "a"
|
||||
b = tmp_path / "b"
|
||||
a.mkdir()
|
||||
b.mkdir()
|
||||
write_sarif(a, [_finding(title="SQL Injection in get_user")])
|
||||
write_sarif(b, [_finding(title="SQLi via string-formatted query in get_user")])
|
||||
|
||||
rule_a, loc_a, fp_a = _rule_and_location(a)
|
||||
rule_b, loc_b, fp_b = _rule_and_location(b)
|
||||
|
||||
assert rule_a == rule_b == "CWE-89/sql-injection"
|
||||
assert (
|
||||
loc_a
|
||||
== loc_b
|
||||
== {
|
||||
"artifactLocation": {"uri": "app.py"},
|
||||
"region": {"startLine": 4},
|
||||
}
|
||||
)
|
||||
assert fp_a == fp_b
|
||||
|
||||
|
||||
def test_class_alias_rewordings_share_one_rule_id(tmp_path: Path) -> None:
|
||||
# The fix is a canonical-class mapping, not a one-off for "SQLi":
|
||||
# every equivalent-name pair below must share one rule id.
|
||||
pairs = [
|
||||
(
|
||||
"IDOR via order parameter",
|
||||
"Insecure Direct Object Reference via order parameter",
|
||||
"CWE-639",
|
||||
),
|
||||
("Cross-Site Scripting in search", "XSS in search", "CWE-79"),
|
||||
("Directory traversal in download", "Path traversal in download", "CWE-22"),
|
||||
("ReDoS in search regex", "Regex denial of service in search regex", "CWE-1333"),
|
||||
("SQL-injection in login", "SQL Injection in login", "CWE-89"),
|
||||
]
|
||||
for i, (alias_title, canonical_title, cwe) in enumerate(pairs):
|
||||
a = tmp_path / f"a{i}"
|
||||
b = tmp_path / f"b{i}"
|
||||
a.mkdir()
|
||||
b.mkdir()
|
||||
write_sarif(a, [_finding(title=alias_title, cwe=cwe)])
|
||||
write_sarif(b, [_finding(title=canonical_title, cwe=cwe)])
|
||||
rule_a, _, _ = _rule_and_location(a)
|
||||
rule_b, _, _ = _rule_and_location(b)
|
||||
assert rule_a == rule_b, (alias_title, canonical_title, rule_a, rule_b)
|
||||
|
||||
|
||||
def test_distinct_class_still_gets_distinct_rule_id(tmp_path: Path) -> None:
|
||||
# Negative control: canonicalisation must not collapse genuinely
|
||||
# different classes. Same CWE, different class -> different rule.
|
||||
write_sarif(
|
||||
tmp_path,
|
||||
[
|
||||
_finding(id="vuln-0001", title="SQL Injection in get_user", cwe="CWE-89"),
|
||||
_finding(id="vuln-0002", title="Command Injection in get_user", cwe="CWE-89"),
|
||||
],
|
||||
)
|
||||
run = _read(tmp_path)["runs"][0]
|
||||
|
||||
assert [result["ruleId"] for result in run["results"]] == [
|
||||
"CWE-89/sql-injection",
|
||||
"CWE-89/command-injection",
|
||||
]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue