Skip to content

Commit 4d27a6e

Browse files
fzipifzipi
committed
feat: add check for TX.N usage without capture
Signed-off-by: Felipe Zipitria <[email protected]>
1 parent 0f3d209 commit 4d27a6e

File tree

4 files changed

+542
-74
lines changed

4 files changed

+542
-74
lines changed

‎README.md‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,32 +163,47 @@ util/APPROVED_TAGS file.
163163

164164
**Source:** `src/crs_linter/rules/check_capture.py`
165165

166-
Check that every chained rule has a `capture` action if it uses TX.N variable.
166+
Check that rules using TX.N variables have a corresponding `capture` action.
167167

168-
This rule ensures that chained rules using captured transaction variables
169-
(TX:0, TX:1, TX:2, etc.) have a corresponding `capture` action in a
170-
previous rule in the chain.
168+
This rule ensures that captured transaction variables (TX:0, TX:1, TX:2, etc.)
169+
are only used when a `capture` action has been defined in the rule chain.
171170

172-
Example of a passing rule:
173-
SecRule ARGS "@rx attack" \
171+
TX.N variables can be referenced in multiple ways:
172+
1. As a rule target: `SecRule TX:1 "@eq attack"`
173+
2. In action arguments: `msg:'Matched: %{TX.1}'`, `logdata:'Data: %{TX.0}'`
174+
3. In operator arguments: `@rx %{TX.1}`
175+
4. In setvar assignments: `setvar:tx.foo=%{TX.1}`
176+
177+
Example of a passing rule (with capture):
178+
SecRule ARGS "@rx (attack)" \
174179
"id:2,\
175180
phase:2,\
176181
deny,\
177182
capture,\
178-
t:none,\
179-
nolog,\
183+
msg:'Attack detected: %{TX.1}',\
184+
logdata:'Pattern: %{TX.0}',\
180185
chain"
181186
SecRule TX:1 "@eq attack"
182187

183-
Example of a failing rule (missing capture):
188+
Example of a failing rule (missing capture for target):
184189
SecRule ARGS "@rx attack" \
185190
"id:3,\
186191
phase:2,\
187192
deny,\
188193
t:none,\
189194
nolog,\
190195
chain"
191-
SecRule TX:0 "@eq attack" # Fails: uses TX:0 without prior capture
196+
SecRule TX:0 "@eq attack" # Fails: uses TX:0 without capture
197+
198+
Example of a failing rule (missing capture for action argument):
199+
SecRule ARGS "@rx attack" \
200+
"id:4,\
201+
phase:2,\
202+
deny,\
203+
msg:'Matched: %{TX.1}'" # Fails: references TX.1 without capture
204+
205+
This check addresses the issue found in CRS PR #4265 where %{TX.N} was
206+
used in action arguments without verifying that capture was defined.
192207

193208
## CrsTag
194209

‎src/crs_linter/rules/check_capture.py‎

Lines changed: 127 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -4,32 +4,47 @@
44

55

66
class CheckCapture(Rule):
7-
"""Check that every chained rule has a `capture` action if it uses TX.N variable.
7+
"""Check that rules using TX.N variables have a corresponding `capture` action.
88
9-
This rule ensures that chained rules using captured transaction variables
10-
(TX:0, TX:1, TX:2, etc.) have a corresponding `capture` action in a
11-
previous rule in the chain.
9+
This rule ensures that captured transaction variables (TX:0, TX:1, TX:2, etc.)
10+
are only used when a `capture` action has been defined in the rule chain.
1211
13-
Example of a passing rule:
14-
SecRule ARGS "@rx attack" \\
12+
TX.N variables can be referenced in multiple ways:
13+
1. As a rule target: `SecRule TX:1 "@eq attack"`
14+
2. In action arguments: `msg:'Matched: %{TX.1}'`, `logdata:'Data: %{TX.0}'`
15+
3. In operator arguments: `@rx %{TX.1}`
16+
4. In setvar assignments: `setvar:tx.foo=%{TX.1}`
17+
18+
Example of a passing rule (with capture):
19+
SecRule ARGS "@rx (attack)" \\
1520
"id:2,\\
1621
phase:2,\\
1722
deny,\\
1823
capture,\\
19-
t:none,\\
20-
nolog,\\
24+
msg:'Attack detected: %{TX.1}',\\
25+
logdata:'Pattern: %{TX.0}',\\
2126
chain"
2227
SecRule TX:1 "@eq attack"
2328
24-
Example of a failing rule (missing capture):
29+
Example of a failing rule (missing capture for target):
2530
SecRule ARGS "@rx attack" \\
2631
"id:3,\\
2732
phase:2,\\
2833
deny,\\
2934
t:none,\\
3035
nolog,\\
3136
chain"
32-
SecRule TX:0 "@eq attack" # Fails: uses TX:0 without prior capture
37+
SecRule TX:0 "@eq attack" # Fails: uses TX:0 without capture
38+
39+
Example of a failing rule (missing capture for action argument):
40+
SecRule ARGS "@rx attack" \\
41+
"id:4,\\
42+
phase:2,\\
43+
deny,\\
44+
msg:'Matched: %{TX.1}'" # Fails: references TX.1 without capture
45+
46+
This check addresses the issue found in CRS PR #4265 where %{TX.N} was
47+
used in action arguments without verifying that capture was defined.
3348
"""
3449

3550
def __init__(self):
@@ -40,65 +55,118 @@ def __init__(self):
4055
self.error_title = "capture is missing"
4156
self.args = ("data",)
4257

58+
# Regex patterns for detecting TX.N references
59+
self.target_pattern = re.compile(r"^\d$") # For target variables: TX:1
60+
self.expansion_pattern = re.compile(
61+
r"%\{TX[.:](\d)\}", # Matches %{TX.0} or %{TX:1}
62+
re.IGNORECASE
63+
)
64+
65+
# Actions that commonly use variable expansion
66+
self.actions_to_check = {
67+
'msg', 'logdata', 'setvar', 'tag'
68+
}
69+
4370
def check(self, data):
4471
"""
45-
check that every chained rule has a `capture` action if it uses TX.N variable
72+
Check that TX.N variables are only used when capture action is defined.
73+
74+
This checks for TX.N references in:
75+
- Rule targets (existing functionality)
76+
- Action arguments (msg, logdata, setvar, tag)
77+
- Operator arguments
4678
"""
4779
chained = False
4880
ruleid = 0
4981
chainlevel = 0
5082
capture_level = None
51-
re_number = re.compile(r"^\d$")
5283
has_capture = False
5384
use_captured_var = False
85+
use_captured_var_in_expansion = False # Track if TX.N is used in expansion
5486
captured_var_chain_level = 0
87+
5588
for d in data:
5689
# only the SecRule object is relevant
57-
if d["type"].lower() == "secrule":
58-
for v in d["variables"]:
59-
if v["variable"].lower() == "tx" and re_number.match(
60-
v["variable_part"]
61-
):
62-
# only the first occurrence required
63-
if not use_captured_var:
64-
use_captured_var = True
65-
captured_var_chain_level = chainlevel
66-
if "actions" in d:
67-
if not chained:
68-
ruleid = 0
69-
chainlevel = 0
70-
else:
71-
chained = False
72-
for a in d["actions"]:
73-
if a["act_name"] == "id":
74-
ruleid = int(a["act_arg"])
75-
if a["act_name"] == "chain":
76-
chained = True
77-
chainlevel += 1
78-
if a["act_name"] == "capture":
79-
capture_level = chainlevel
80-
has_capture = True
81-
if ruleid > 0 and not chained: # end of chained rule
82-
if use_captured_var:
83-
# we allow if target with TX:N is in the first rule
84-
# of a chained rule without 'capture'
85-
if captured_var_chain_level > 0:
86-
if (
87-
not has_capture
88-
or captured_var_chain_level < capture_level
89-
):
90-
yield LintProblem(
91-
line=a["lineno"],
92-
end_line=a["lineno"],
93-
desc=f"rule uses TX.N without capture; rule id: {ruleid}",
94-
rule="capture",
95-
)
96-
# clear variables
97-
chained = False
98-
chainlevel = 0
99-
has_capture = False
100-
capture_level = 0
101-
captured_var_chain_level = 0
102-
use_captured_var = False
103-
ruleid = 0
90+
if d["type"].lower() != "secrule":
91+
continue
92+
93+
# Check 1: TX.N as target variable (existing check)
94+
for v in d["variables"]:
95+
if (v["variable"].lower() == "tx" and
96+
self.target_pattern.match(v["variable_part"])):
97+
# only the first occurrence required
98+
if not use_captured_var:
99+
use_captured_var = True
100+
captured_var_chain_level = chainlevel
101+
102+
# Check 2: TX.N in operator arguments
103+
if "operator_argument" in d and d["operator_argument"]:
104+
op_arg = d["operator_argument"]
105+
if self.expansion_pattern.search(op_arg):
106+
if not use_captured_var:
107+
use_captured_var = True
108+
use_captured_var_in_expansion = True
109+
captured_var_chain_level = chainlevel
110+
111+
# Check 3: TX.N in action arguments
112+
if "actions" in d:
113+
if not chained:
114+
ruleid = 0
115+
chainlevel = 0
116+
else:
117+
chained = False
118+
119+
for a in d["actions"]:
120+
if a["act_name"] == "id":
121+
ruleid = int(a["act_arg"])
122+
if a["act_name"] == "chain":
123+
chained = True
124+
chainlevel += 1
125+
if a["act_name"] == "capture":
126+
capture_level = chainlevel
127+
has_capture = True
128+
129+
# Check if action argument contains TX.N reference
130+
if (a["act_name"] in self.actions_to_check and
131+
a.get("act_arg")):
132+
if self.expansion_pattern.search(a["act_arg"]):
133+
if not use_captured_var:
134+
use_captured_var = True
135+
use_captured_var_in_expansion = True
136+
captured_var_chain_level = chainlevel
137+
138+
# End of rule/chain - validate
139+
if ruleid > 0 and not chained:
140+
if use_captured_var:
141+
# Rules for requiring capture:
142+
# 1. TX.N as target in chained rule (not first) - require capture (original check)
143+
# 2. TX.N in expansion (%{TX.N}) anywhere - require capture (new check for issue #69)
144+
should_error = False
145+
146+
if captured_var_chain_level > 0:
147+
# TX.N used in a chained rule (not the first)
148+
if not has_capture or captured_var_chain_level < capture_level:
149+
should_error = True
150+
elif use_captured_var_in_expansion and not has_capture:
151+
# TX.N used in expansion (%{TX.N}) without capture
152+
# This is the new check for issue #69
153+
should_error = True
154+
155+
if should_error:
156+
yield LintProblem(
157+
line=a["lineno"],
158+
end_line=a["lineno"],
159+
desc=f"rule uses TX.N without capture; rule id: {ruleid}",
160+
rule="capture",
161+
)
162+
163+
# clear variables
164+
chained = False
165+
chainlevel = 0
166+
has_capture = False
167+
capture_level = 0
168+
captured_var_chain_level = 0
169+
use_captured_var = False
170+
use_captured_var_in_expansion = False
171+
ruleid = 0
104172

‎src/crs_linter/rules/variables_usage.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,16 +87,16 @@ def check(self, data, globtxvars):
8787
# act_arg <- tx.inbound_anomaly_score_threshold
8888
# act_atg_val <- 5
8989
if "act_arg" in a and a["act_arg"] is not None:
90-
val_act = re.findall(r"%\{(tx.[^%]*)}", a["act_arg"], re.I)
90+
val_act = re.findall(r"%\{(tx.[^%]*)}", a["act_arg"], flags=re.I)
9191
# Check act_arg_val for TX variable references in action argument values
9292
# (e.g., the right-hand side of setvar assignments like "setvar:tx.foo=%{tx.bar}")
9393
if "act_arg_val" in a and a["act_arg_val"] is not None:
9494
val_act_arg = re.findall(
95-
r"%\{(tx.[^%]*)}", a["act_arg_val"], re.I
95+
r"%\{(tx.[^%]*)}", a["act_arg_val"], flags=re.I
9696
)
9797
for v in val_act + val_act_arg:
9898
v = v.lower().replace("tx.", "")
99-
if not re.match(r"^\d$", v, re.I):
99+
if not re.match(r"^\d$", v, flags=re.I):
100100
if (
101101
v not in globtxvars
102102
or phase < globtxvars[v]["phase"]
@@ -114,11 +114,11 @@ def check(self, data, globtxvars):
114114
globtxvars[v]["used"] = True
115115

116116
if "operator_argument" in d:
117-
oparg = re.findall(r"%\{(tx.[^%]*)}", d["operator_argument"], re.I)
117+
oparg = re.findall(r"%\{(tx.[^%]*)}", d["operator_argument"], flags=re.I)
118118
if oparg:
119119
for o in oparg:
120120
o = o.lower()
121-
o = re.sub(r"tx\.", "", o, re.I)
121+
o = re.sub(r"tx\.", "", o, flags=re.I)
122122
if (
123123
(
124124
o not in globtxvars

0 commit comments

Comments
 (0)