mirror of
https://github.com/CyberMind-FR/secubox-deb.git
synced 2026-07-28 21:17:36 +00:00
fix(cve-triage): dedup must regex-match live WAF patterns, not exact-string
The generator only rejected a candidate when re.escape(path) exactly equaled a stored pattern key, so it never matched the live cve_2024 category's hand-written regexes (literal hyphens, \d classes) — the dedup feature was inert against real WAF rules. Now a candidate is covered when any existing pattern, treated as a regex (as the WAF engine does), matches the raw path; invalid stored regexes are skipped, never raised. Also harden existing_patterns() against valid-but-non-object JSON (e.g. "[]"/"null") which previously raised AttributeError on data.get(...), breaking the read-only preview/dry-run best-effort contract. Co-Authored-By: Gerald KERMA <devel@cybermind.fr>
This commit is contained in:
parent
4e0ec8fd05
commit
58afb624ff
|
|
@ -37,6 +37,8 @@ def existing_patterns(rules_path: Path) -> dict[str, str]:
|
|||
data = json.loads(Path(rules_path).read_text(encoding="utf-8"))
|
||||
except (OSError, json.JSONDecodeError):
|
||||
return {}
|
||||
if not isinstance(data, dict):
|
||||
return {}
|
||||
cats = data.get("categories", {})
|
||||
if not isinstance(cats, dict):
|
||||
return {}
|
||||
|
|
|
|||
|
|
@ -22,6 +22,20 @@ from .extract import Candidate, Rejection, extract, is_kev
|
|||
from .presence import is_appliance, should_generate
|
||||
|
||||
|
||||
def _covered_by(path: str, existing: dict[str, str]) -> str | None:
|
||||
"""Nom de la catégorie dont un pattern (traité comme regex, ce que fait le
|
||||
moteur WAF) matche `path` — donc la sonde detect y serait redondante. Un
|
||||
pattern stocké qui n'est pas une regex valide ne peut pas matcher : on le
|
||||
saute (jamais d'erreur). None si aucune couverture."""
|
||||
for pattern, category in existing.items():
|
||||
try:
|
||||
if re.search(pattern, path):
|
||||
return category
|
||||
except re.error:
|
||||
continue
|
||||
return None
|
||||
|
||||
|
||||
def generate(subset_dir: Path, present: set[str], present_complete: bool,
|
||||
*, existing: dict[str, str] | None = None
|
||||
) -> tuple[list[Candidate], list[tuple[str, str]]]:
|
||||
|
|
@ -59,9 +73,9 @@ def generate(subset_dir: Path, present: set[str], present_complete: bool,
|
|||
rejected.append((p.name, why))
|
||||
continue
|
||||
|
||||
pattern = re.escape(result.path)
|
||||
if pattern in existing:
|
||||
rejected.append((p.name, f"already covered by {existing[pattern]}"))
|
||||
covered_by = _covered_by(result.path, existing)
|
||||
if covered_by is not None:
|
||||
rejected.append((p.name, f"already covered by {covered_by}"))
|
||||
continue
|
||||
|
||||
kept.append(result)
|
||||
|
|
|
|||
|
|
@ -90,7 +90,6 @@ def test_apply_with_incomplete_inventory_returns_3_and_leaves_rules_untouched(tm
|
|||
|
||||
|
||||
def test_cli_dedups_against_existing_rules(tmp_path, monkeypatch, capsys):
|
||||
import re
|
||||
from api.wafgen import cli
|
||||
subset = tmp_path / "subset"
|
||||
subset.mkdir()
|
||||
|
|
@ -102,11 +101,10 @@ def test_cli_dedups_against_existing_rules(tmp_path, monkeypatch, capsys):
|
|||
' tags: cve,kev\n'
|
||||
'http:\n - method: GET\n path: ["{{BaseURL}}/api/v1/totp/user-backup"]\n')
|
||||
rules = tmp_path / "waf-rules.json"
|
||||
# json.dumps (not raw string concat) so re.escape()'s backslash before the
|
||||
# hyphen in "user-backup" is properly JSON-escaped — see test_emit.py's
|
||||
# equivalent fixture, which hits the same "\-" pitfall.
|
||||
# Literal hyphen (unescaped) — this is the real live cve_2024 rule form,
|
||||
# a hand-written regex, NOT re.escape() output.
|
||||
rules.write_text(json.dumps({"categories": {"cve_2024": {"patterns": [
|
||||
{"pattern": re.escape("/api/v1/totp/user-backup")}]}}}))
|
||||
{"pattern": "/api/v1/totp/user-backup"}]}}}))
|
||||
monkeypatch.setattr(cli, "gather_present", lambda: (set(), True))
|
||||
|
||||
rc = cli.main(["waf-rules", "generate",
|
||||
|
|
|
|||
|
|
@ -165,3 +165,15 @@ def test_existing_patterns_missing_or_corrupt_file_is_empty(tmp_path):
|
|||
bad = tmp_path / "bad.json"
|
||||
bad.write_text("{ not json")
|
||||
assert existing_patterns(bad) == {}
|
||||
|
||||
|
||||
def test_existing_patterns_non_object_json_is_empty_not_a_raise(tmp_path):
|
||||
# Valid JSON but not a top-level object ([] / null / ...) must not raise
|
||||
# AttributeError on `.get` — best-effort contract: no dedup, never a crash.
|
||||
from api.wafgen.emit import existing_patterns
|
||||
empty_list = tmp_path / "list.json"
|
||||
empty_list.write_text("[]")
|
||||
assert existing_patterns(empty_list) == {}
|
||||
null_json = tmp_path / "null.json"
|
||||
null_json.write_text("null")
|
||||
assert existing_patterns(null_json) == {}
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@
|
|||
# Copyright (c) 2026 CyberMind — Gérald Kerma <devel@cybermind.fr>
|
||||
# Source-Disclosed License — All rights reserved except as expressly granted.
|
||||
# See LICENCE-CMSD-1.0.md for terms.
|
||||
import re
|
||||
import textwrap
|
||||
|
||||
from api.wafgen.generate import generate
|
||||
|
|
@ -107,14 +106,57 @@ def test_candidate_already_covered_by_another_category_is_rejected(tmp_path):
|
|||
- method: GET
|
||||
path: ["{{BaseURL}}/api/v1/totp/user-backup"]
|
||||
""")
|
||||
# cve_2024 already covers this exact path (stored re.escape'd)
|
||||
existing = {re.escape("/api/v1/totp/user-backup"): "cve_2024"}
|
||||
# cve_2024 already covers this path — stored as the live WAF file actually
|
||||
# would: a literal hand-written pattern (hyphen unescaped), NOT re.escape().
|
||||
existing = {"/api/v1/totp/user-backup": "cve_2024"}
|
||||
kept, rejected = generate(subset, present=set(), present_complete=True,
|
||||
existing=existing)
|
||||
assert kept == []
|
||||
assert any("already covered by cve_2024" in reason for _, reason in rejected)
|
||||
|
||||
|
||||
def test_candidate_covered_by_a_regex_variant_pattern_is_rejected(tmp_path):
|
||||
# The engine treats stored patterns as regexes: a hand-written pattern
|
||||
# using `\d` must still dedup the concrete candidate path it matches.
|
||||
subset = tmp_path / "nuclei-subset"
|
||||
subset.mkdir()
|
||||
_write(subset, "ivanti.yaml", """
|
||||
id: CVE-2024-21887
|
||||
info:
|
||||
severity: critical
|
||||
classification: {cve-id: CVE-2024-21887, cpe: "cpe:2.3:a:ivanti:connect_secure:*"}
|
||||
metadata: {vendor: ivanti, product: connect_secure}
|
||||
tags: cve,kev
|
||||
http:
|
||||
- method: GET
|
||||
path: ["{{BaseURL}}/api/v1/totp/user-backup"]
|
||||
""")
|
||||
existing = {r"/api/v\d/totp/user-backup": "cve_2024"}
|
||||
kept, rejected = generate(subset, present=set(), present_complete=True,
|
||||
existing=existing)
|
||||
assert kept == []
|
||||
assert any("already covered by cve_2024" in reason for _, reason in rejected)
|
||||
|
||||
|
||||
def test_invalid_existing_regex_is_skipped_not_raised(tmp_path):
|
||||
subset = tmp_path / "nuclei-subset"
|
||||
subset.mkdir()
|
||||
_write(subset, "ivanti.yaml", """
|
||||
id: CVE-2024-21887
|
||||
info:
|
||||
severity: critical
|
||||
classification: {cve-id: CVE-2024-21887, cpe: "cpe:2.3:a:ivanti:connect_secure:*"}
|
||||
metadata: {vendor: ivanti, product: connect_secure}
|
||||
tags: cve,kev
|
||||
http:
|
||||
- method: GET
|
||||
path: ["{{BaseURL}}/api/v1/totp/user-backup"]
|
||||
""")
|
||||
existing = {"[unclosed(": "weird"}
|
||||
kept, _ = generate(subset, present=set(), present_complete=True, existing=existing)
|
||||
assert [c.cve for c in kept] == ["CVE-2024-21887"]
|
||||
|
||||
|
||||
def test_no_dedup_when_existing_absent_keeps_candidate(tmp_path):
|
||||
subset = tmp_path / "nuclei-subset"
|
||||
subset.mkdir()
|
||||
|
|
@ -150,6 +192,6 @@ def test_dedup_does_not_over_reject_unrelated_existing_patterns(tmp_path):
|
|||
- method: GET
|
||||
path: ["{{BaseURL}}/api/v1/totp/user-backup"]
|
||||
""")
|
||||
existing = {re.escape("/some/other/unrelated/path"): "cve_2024"}
|
||||
existing = {"/some/other/unrelated/path": "cve_2024"}
|
||||
kept, _ = generate(subset, present=set(), present_complete=True, existing=existing)
|
||||
assert [c.cve for c in kept] == ["CVE-2024-21887"]
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user