Skip to content

Commit f2a39cf

Browse files
fix: eliminate exception-derived text from API responses (CodeQL py/stack-trace-exposure #23, #25, #26)
- SigmaValidator: static YAML parse-failure message, no library detail - Wazuh backend: replace NotImplementedError signaling with structured {supported: false, reason} results - app.py: module-level static message map keyed by reason code - cli.py: update call sites for new convert() contract - tests: update two cases that asserted the prior intentional behavior; tighten convert_matrix assertion to name offending template/backend - README: correct three stale references to the old raise contract
1 parent aaccdb2 commit f2a39cf

5 files changed

Lines changed: 125 additions & 61 deletions

File tree

README.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ SigmaForge is a detection rule authoring tool that generates, validates, and con
3434
**Aggregation support:** Templates using aggregation conditions
3535
(`count() by ... > N`) emit real queries on Splunk SPL and QRadar AQL only.
3636
Elastic KQL and Elastic EQL do not implement aggregation and return
37-
placeholder text; Wazuh raises `NotImplementedError` (documented below under
38-
Wazuh backend specifics). Affected templates: `windows_logon_brute_force`,
37+
placeholder text; Wazuh returns a structured `{"supported": False, "reason":
38+
"aggregation_condition"}` result instead of a query string (documented below
39+
under Wazuh backend specifics). Affected templates: `windows_logon_brute_force`,
3940
`firewall_port_scan`, `brute_force_by_username`.
4041

4142
**Index and sourcetype defaults:** generated queries use conventional index
@@ -164,7 +165,7 @@ Methods:
164165
- Valid field modifiers: `contains`, `startswith`, `endswith`, `base64`, `base64offset`, `utf16le`, `utf16be`, `wide`, `re`, `cidr`, `all`, `gt`, `gte`, `lt`, `lte`, `fieldref`, `expand`, `windash`
165166

166167
**`SIEMConverter` (static)**
167-
- `convert(rule_yaml, backend, rule_id=100001, group_name="sigma_rules") → str`
168+
- `convert(rule_yaml, backend, rule_id=100001, group_name="sigma_rules") → str | dict``dict` only for the wazuh backend's known-unsupported cases (`{"supported": False, "reason": <code>}`)
168169
- `_build_field_query(field_name, values, backend, negate, field_map)` — translates a single field with modifiers to the backend's syntax
169170
- `_parse_condition(condition, selections, backend)` — resolves selection references, handles boolean operators and aggregation conditions (`count() by field > N`)
170171
- `_build_aggregation(base_query, count_field, group_field, operator, threshold, backend)` — generates `stats`/`summarize`/aggregation syntax per backend
@@ -174,7 +175,7 @@ Methods:
174175
- `WAZUH_FIELD_MAP` — decoder-scoped field maps: `windows_security`, `windows_sysmon`, `windows_eventchannel`, `linux_auth`, `linux_audit`, `linux_syslog`
175176
- Emits `<group>``<rule>``<field>` elements; OR conditions produce multiple `<rule>` siblings; NOT conditions produce `negate="yes"` on `<field>`
176177
- `<mitre><id>` block requires Wazuh 4.2+
177-
- Aggregation conditions (conditions containing `|`) raise `NotImplementedError` — Wazuh does not support them natively
178+
- Aggregation conditions (conditions containing `|`), parenthesised sub-expressions, and conditions resolving to no field selections are not supported natively — `convert()` returns `{"supported": False, "reason": <code>}` instead of a query string, and `app.py`/`cli.py` map the reason code to a static, human-written message (never exception text — see CodeQL py/stack-trace-exposure)
178179
- `rule_id` clamped to `1–999,999`; `group_name` validated against `^[A-Za-z0-9._-]{1,64}$`
179180

180181
**Helper functions:**
@@ -310,7 +311,7 @@ pip install -r requirements-dev.txt
310311
python -m pytest
311312
```
312313

313-
`tests/test_conversion.py` parametrizes every `RULE_TEMPLATES` key across all seven backends (`splunk`, `elastic`, `eql`, `sentinel`, `wazuh`, `qradar`, `dac_json`), asserting `SIEMConverter.convert()` returns a non-empty string — except Wazuh on the three aggregation-condition templates (`windows_logon_brute_force`, `firewall_port_scan`, `brute_force_by_username`), where it asserts `NotImplementedError` is raised. It also asserts every template validates via `SigmaValidator`, and locks in the aggregation-handling fix with regression checks against the Splunk/Sentinel output of those three templates.
314+
`tests/test_conversion.py` parametrizes every `RULE_TEMPLATES` key across all seven backends (`splunk`, `elastic`, `eql`, `sentinel`, `wazuh`, `qradar`, `dac_json`), asserting `SIEMConverter.convert()` returns a non-empty string — except Wazuh on the three aggregation-condition templates (`windows_logon_brute_force`, `firewall_port_scan`, `brute_force_by_username`), where it asserts a `{"supported": False, "reason": "aggregation_condition"}` result is returned instead. It also asserts every template validates via `SigmaValidator`, and locks in the aggregation-handling fix with regression checks against the Splunk/Sentinel output of those three templates.
314315

315316
---
316317

app.py

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,26 +93,57 @@ def _safe_library_path(filename: str) -> str:
9393
return filepath
9494

9595

96+
# Static, human-written explanations for known Wazuh backend syntax gaps.
97+
# SIEMConverter.convert()/_wazuh_build_rule() return {"supported": False,
98+
# "reason": <key>} for these cases instead of raising, so the text shown to
99+
# the user always comes from this literal dict — never from an exception
100+
# object (CodeQL py/stack-trace-exposure).
101+
_WAZUH_UNSUPPORTED_MESSAGES = {
102+
"aggregation_condition": (
103+
"Wazuh backend does not support aggregation conditions "
104+
"(e.g. 'selection | count() by field > N'). "
105+
"Aggregation support is planned for a future phase."
106+
),
107+
"parenthesized_condition": (
108+
"Wazuh backend (Phase 1) does not support parenthesised "
109+
"sub-expressions. Rewrite the condition as a flat AND/OR/NOT expression."
110+
),
111+
"no_field_selection": (
112+
"Wazuh backend: the condition did not resolve to explicit field "
113+
"selections (e.g. '1 of selection*' or 'all of them' are not "
114+
"supported). Use explicit selection names."
115+
),
116+
"_default": "Wazuh backend cannot convert this rule due to unsupported condition syntax.",
117+
}
118+
119+
120+
def _wazuh_unsupported_message(reason: str) -> str:
121+
return _WAZUH_UNSUPPORTED_MESSAGES.get(reason, _WAZUH_UNSUPPORTED_MESSAGES["_default"])
122+
123+
96124
def _convert_backend_safe(rule_yaml: str, backend: str, **kwargs) -> str:
97125
"""
98126
Convert rule_yaml to the given backend for display in the multi-backend
99127
conversion panel, without ever putting a raw exception message into the
100128
client response (CodeQL py/stack-trace-exposure).
101129
102-
NotImplementedError is raised deliberately by SIEMConverter for known,
103-
documented syntax gaps (e.g. Wazuh's lack of aggregation-condition
104-
support) and carries a message written for the end user, so it is safe
105-
to surface verbatim. Any other exception is logged with a full traceback
130+
Known, documented syntax gaps (e.g. Wazuh's lack of aggregation-condition
131+
support) are signaled by SIEMConverter as a structured
132+
{"supported": False, "reason": ...} result, not an exception, and are
133+
rendered here from the static _WAZUH_UNSUPPORTED_MESSAGES dict. Any
134+
genuinely unexpected exception is logged with a full traceback
106135
server-side and replaced with a generic message client-side.
107136
"""
108137
try:
109-
return SIEMConverter.convert(rule_yaml, backend, **kwargs)
110-
except NotImplementedError as e:
111-
return f"Conversion error: {e}"
138+
result = SIEMConverter.convert(rule_yaml, backend, **kwargs)
112139
except Exception:
113140
logging.exception("Unexpected error converting rule to backend %s", backend)
114141
return "Conversion error: An internal error occurred while converting to this backend."
115142

143+
if isinstance(result, dict) and result.get("supported") is False:
144+
return f"Conversion error: {_wazuh_unsupported_message(result.get('reason'))}"
145+
return result
146+
116147

117148
# ─────────────────────────────────────────────
118149
# Web Routes
@@ -276,6 +307,13 @@ def api_convert():
276307
rule_id=rule_id, group_name=group_name)
277308
else:
278309
query = SIEMConverter.convert(rule_yaml, backend)
310+
311+
if isinstance(query, dict) and query.get("supported") is False:
312+
return jsonify({
313+
"success": False,
314+
"error": _wazuh_unsupported_message(query.get("reason")),
315+
}), 400
316+
279317
return jsonify({"success": True, "query": query, "backend": backend})
280318
except Exception as e:
281319
logging.exception("Unexpected error in api_convert for backend %s", backend)

cli.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,10 @@ def cmd_generate(args):
166166
rule_yaml, backend,
167167
rule_id=args.rule_id, group_name=args.group_name,
168168
)
169-
print(output)
169+
if isinstance(output, dict) and output.get("supported") is False:
170+
print_error(f"Wazuh conversion unsupported: {output.get('reason')}")
171+
else:
172+
print(output)
170173
elif backend in ("dac_json", "qradar"):
171174
print(SIEMConverter.convert(rule_yaml, backend))
172175
else:
@@ -224,7 +227,10 @@ def cmd_convert(args):
224227
rule_yaml, backend,
225228
rule_id=args.rule_id, group_name=args.group_name,
226229
)
227-
print(output)
230+
if isinstance(output, dict) and output.get("supported") is False:
231+
print_error(f"Wazuh conversion unsupported: {output.get('reason')}")
232+
else:
233+
print(output)
228234
elif backend in ("dac_json", "qradar"):
229235
print(SIEMConverter.convert(rule_yaml, backend))
230236
else:

src/sigma_engine.py

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -990,9 +990,12 @@ def validate(rule_yaml: str) -> dict:
990990
# Parse YAML
991991
try:
992992
rule = yaml.safe_load(rule_yaml)
993-
except yaml.YAMLError as e:
993+
except yaml.YAMLError:
994994
result["valid"] = False
995-
result["errors"].append(f"YAML parse error: {str(e)}")
995+
result["errors"].append(
996+
"YAML parse error: the rule document could not be parsed. "
997+
"Check indentation, colons after keys, and matching brackets/quotes."
998+
)
996999
return result
9971000

9981001
if not isinstance(rule, dict):
@@ -1396,13 +1399,16 @@ def _wazuh_build_rule(
13961399
rule: dict,
13971400
rule_id: int = 100001,
13981401
group_name: str = "sigma_rules",
1399-
) -> str:
1402+
) -> str | dict:
14001403
"""
14011404
Assemble a Wazuh XML rule group from a parsed Sigma rule dict.
14021405
OR conditions produce multiple <rule> elements (one per branch);
14031406
AND/NOT conditions produce a single <rule> with negated <field> elements.
1404-
Phase 1 supports flat AND/OR/NOT conditions only — parenthesised
1405-
sub-expressions (other than a single outer wrapper) raise NotImplementedError.
1407+
Phase 1 supports flat AND/OR/NOT conditions only — unsupported syntax
1408+
(parenthesised sub-expressions, or a condition that resolves to no
1409+
field selections) returns {"supported": False, "reason": ...} instead
1410+
of raising, so callers never have to derive user-facing text from an
1411+
exception object (CodeQL py/stack-trace-exposure).
14061412
"""
14071413

14081414
# ── Helpers ───────────────────────────────────────────────────────
@@ -1549,11 +1555,7 @@ def _split_top(expr: str, operator: str) -> list:
15491555
# Phase 1 supports flat AND/OR/NOT only.
15501556
condition_stripped = _unwrap(condition.strip())
15511557
if "(" in condition_stripped or ")" in condition_stripped:
1552-
raise NotImplementedError(
1553-
f"Wazuh backend (Phase 1) does not support parenthesised "
1554-
f"sub-expressions: {condition!r}. "
1555-
f"Rewrite as a flat AND/OR/NOT condition."
1556-
)
1558+
return {"supported": False, "reason": "parenthesized_condition"}
15571559

15581560
or_branches = _split_top(condition_stripped, "or")
15591561
branch_specs = []
@@ -1613,11 +1615,7 @@ def _split_top(expr: str, operator: str) -> list:
16131615
# Guard: a branch with no positive or negative selections means
16141616
# the condition used unsupported syntax (1 of selection*, all of them, etc.)
16151617
if not spec["positives"] and not spec["negatives"]:
1616-
raise NotImplementedError(
1617-
f"Wazuh backend: condition branch produced no field selections "
1618-
f"from condition {condition!r}. Unsupported syntax — likely "
1619-
f"'1 of selection*' or 'all of them'. Use explicit selection names."
1620-
)
1618+
return {"supported": False, "reason": "no_field_selection"}
16211619

16221620
rid = rule_id + i
16231621
label_parts = [n for n, _ in spec["positives"]]
@@ -1679,7 +1677,7 @@ def _split_top(expr: str, operator: str) -> list:
16791677

16801678
@staticmethod
16811679
def convert(rule_yaml: str, backend: str,
1682-
rule_id: int = 100001, group_name: str = "sigma_rules") -> str:
1680+
rule_id: int = 100001, group_name: str = "sigma_rules") -> str | dict:
16831681
"""
16841682
Convert a Sigma rule YAML string to a SIEM query or structured output.
16851683
@@ -1693,17 +1691,20 @@ def convert(rule_yaml: str, backend: str,
16931691
'dac_json' — Detection-as-Code normalized JSON (no query translation)
16941692
16951693
rule_id and group_name are wazuh-only kwargs (ignored by all other backends).
1694+
1695+
Returns a query string on success. For the wazuh backend, known
1696+
unsupported condition syntax (aggregation conditions, parenthesised
1697+
sub-expressions, conditions with no resolvable field selections)
1698+
returns {"supported": False, "reason": <code>} instead of raising —
1699+
callers map the reason code to a static, human-written message
1700+
(see app.py) rather than deriving response text from an exception.
16961701
"""
16971702
rule = yaml.safe_load(rule_yaml)
16981703

16991704
if backend == "wazuh":
17001705
_condition = rule.get("detection", {}).get("condition", "")
17011706
if "|" in _condition:
1702-
raise NotImplementedError(
1703-
f"Wazuh backend does not support aggregation conditions "
1704-
f"(condition contains '|'): {_condition!r}. "
1705-
f"Aggregation support is planned for a future phase."
1706-
)
1707+
return {"supported": False, "reason": "aggregation_condition"}
17071708
return SIEMConverter._wazuh_build_rule(rule, rule_id=rule_id, group_name=group_name)
17081709

17091710
if backend == "dac_json":

tests/test_conversion.py

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,15 @@
1919
SigmaValidator,
2020
build_rule_from_template,
2121
)
22-
from app import _convert_backend_safe
22+
from app import _convert_backend_safe, _wazuh_unsupported_message
2323

2424
BACKENDS = ["splunk", "elastic", "eql", "sentinel", "wazuh", "qradar", "dac_json"]
2525

2626
# Templates whose condition is an aggregation ("selection | count(...) by field > N").
27-
# Wazuh's backend does not support aggregation conditions and raises NotImplementedError.
27+
# Wazuh's backend does not support aggregation conditions and returns a
28+
# structured {"supported": False, "reason": "aggregation_condition"} result
29+
# instead of raising (CodeQL py/stack-trace-exposure: response text must
30+
# never be derived from a caught exception object).
2831
AGGREGATION_TEMPLATES = {
2932
"windows_logon_brute_force",
3033
"firewall_port_scan",
@@ -41,38 +44,47 @@
4144
@pytest.mark.parametrize("key", TEMPLATE_KEYS)
4245
def test_convert_matrix(key, backend):
4346
rule_yaml = _RULE_YAML[key]
44-
expect_not_implemented = backend == "wazuh" and key in AGGREGATION_TEMPLATES
47+
expect_unsupported = backend == "wazuh" and key in AGGREGATION_TEMPLATES
4548

46-
if expect_not_implemented:
47-
with pytest.raises(NotImplementedError):
48-
SIEMConverter.convert(rule_yaml, backend)
49+
result = SIEMConverter.convert(rule_yaml, backend)
50+
51+
if expect_unsupported:
52+
assert isinstance(result, dict)
53+
assert result.get("supported") is False
54+
assert result.get("reason") == "aggregation_condition"
4955
return
5056

51-
result = SIEMConverter.convert(rule_yaml, backend)
57+
assert not isinstance(result, dict), (
58+
f"{key!r}/{backend!r} unexpectedly returned a structured "
59+
f"unsupported result: {result!r}"
60+
)
5261
assert isinstance(result, str)
5362
assert result.strip() != ""
5463

5564

56-
def test_convert_backend_safe_preserves_notimplementederror_text():
57-
"""NotImplementedError is raised deliberately by SIEMConverter (e.g. the
58-
Wazuh backend's lack of aggregation-condition support) with a message
59-
written for the end user, so _convert_backend_safe() must surface it
60-
verbatim rather than genericizing it."""
65+
def test_convert_backend_safe_uses_static_message_for_unsupported_wazuh_condition():
66+
"""Known Wazuh syntax gaps (e.g. aggregation conditions) are signaled by
67+
SIEMConverter as a structured {"supported": False, "reason": ...} result,
68+
never an exception, and _convert_backend_safe() must render the
69+
corresponding STATIC message from _WAZUH_UNSUPPORTED_MESSAGES — not any
70+
text derived from the rule content or a caught exception object
71+
(CodeQL py/stack-trace-exposure)."""
6172
rule_yaml = _RULE_YAML["brute_force_by_username"]
6273
result = _convert_backend_safe(
6374
rule_yaml, "wazuh", rule_id=100001, group_name="sigma_rules"
6475
)
65-
assert result.startswith("Conversion error:")
76+
assert result == f"Conversion error: {_wazuh_unsupported_message('aggregation_condition')}"
6677
assert "aggregation conditions" in result
67-
assert "TargetUserName" in result # condition text from the real NotImplementedError message
78+
assert "TargetUserName" not in result # no rule-derived content in the message
6879

6980

7081
def test_convert_backend_safe_genericizes_other_exceptions(caplog):
71-
"""Any exception other than NotImplementedError (here: malformed YAML
72-
raising a yaml.YAMLError deep in SIEMConverter.convert()) must not leak
73-
its message, parser detail, or exception class name to the client — only
74-
the fixed generic message, with full detail logged server-side
75-
(CodeQL py/stack-trace-exposure)."""
82+
"""Any actual exception (here: malformed YAML raising a yaml.YAMLError
83+
deep in SIEMConverter.convert()) — as opposed to the structured
84+
{"supported": False, ...} result used for known Wazuh syntax gaps — must
85+
not leak its message, parser detail, or exception class name to the
86+
client. Only the fixed generic message reaches the response, with full
87+
detail logged server-side (CodeQL py/stack-trace-exposure)."""
7688
with caplog.at_level(logging.ERROR):
7789
result = _convert_backend_safe("not: [valid yaml structure", "splunk")
7890

@@ -119,15 +131,21 @@ def test_sentinel_aggregation_has_no_orphan_comment_or_duplicate_where(key):
119131
)
120132

121133

122-
def test_validator_surfaces_yaml_parse_error_text():
123-
"""Intentional behavior, not a bug: CodeQL py/stack-trace-exposure alert #24
124-
on app.py's /api/validate route was dismissed because that endpoint exists
125-
specifically so a user can paste arbitrary Sigma YAML and be told why it
126-
fails to parse. SigmaValidator.validate() deliberately includes the
127-
yaml.YAMLError text in its errors list for exactly this reason — lock it
128-
in so a future "fix" for the CodeQL alert doesn't quietly break it."""
134+
def test_validator_yaml_parse_error_is_static_not_exception_derived():
135+
"""CodeQL py/stack-trace-exposure alerts #23/#25/#26 flagged the SUCCESS
136+
return path in app.py because SigmaValidator.validate()'s result (which
137+
is always included in those responses) embedded str(e) from the caught
138+
yaml.YAMLError. The previous "alert #24 dismissed" design (surfacing the
139+
raw yaml.YAMLError text) is superseded: the validator must now build its
140+
own static parse-failure message and never reach into the exception
141+
object, so no yaml library/module/parser detail can reach an HTTP
142+
response."""
129143
malformed_yaml = "title: Broken Rule\ndetection: [unclosed\n"
130144
result = SigmaValidator.validate(malformed_yaml)
131145

132146
assert result["valid"] is False
133147
assert any("YAML parse error" in err for err in result["errors"])
148+
joined_errors = " ".join(result["errors"])
149+
assert "yaml." not in joined_errors.lower()
150+
assert "line " not in joined_errors.lower()
151+
assert "column" not in joined_errors.lower()

0 commit comments

Comments
 (0)