Skip to content

Commit b9620c2

Browse files
martinhsvmartinhsv
committed
rx:exit after full match; fix TX population after unused group
1 parent a1a8c0f commit b9620c2

File tree

5 files changed

+184
-16
lines changed

5 files changed

+184
-16
lines changed

‎CHANGES

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
v3.x.y - YYYY-MMM-DD (to be released)
22
-------------------------------------
33

4+
- rx: exit after full match (remove /g emulation); ensure capture
5+
groups occuring after unused groups still populate TX vars
6+
[Issue #2336 - @martinhsv]
47
- Correct CHANGES file entry for #2234
58
- Add support to test framework for audit log content verification
69
and add regression tests for issues #2000, #2196

‎src/operators/rx.cc

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ bool Rx::init(const std::string &arg, std::string *error) {
3838

3939
bool Rx::evaluate(Transaction *transaction, RuleWithActions *rule,
4040
const std::string& input, std::shared_ptr<RuleMessage> ruleMessage) {
41-
std::list<SMatch> matches;
4241
Regex *re;
4342

4443
if (m_param.empty() && !m_string->m_containsMacro) {
@@ -52,29 +51,29 @@ bool Rx::evaluate(Transaction *transaction, RuleWithActions *rule,
5251
re = m_re;
5352
}
5453

55-
matches = re->searchAll(input);
54+
std::vector<Utils::SMatchCapture> captures;
55+
re->searchOneMatch(input, captures);
56+
5657
if (rule && rule->hasCaptureAction() && transaction) {
57-
int i = 0;
58-
matches.reverse();
59-
for (const SMatch& a : matches) {
58+
for (const Utils::SMatchCapture& capture : captures) {
59+
const std::string capture_substring(input.substr(capture.m_offset,capture.m_length));
6060
transaction->m_collections.m_tx_collection->storeOrUpdateFirst(
61-
std::to_string(i), a.str());
61+
std::to_string(capture.m_group), capture_substring);
6262
ms_dbg_a(transaction, 7, "Added regex subexpression TX." +
63-
std::to_string(i) + ": " + a.str());
64-
transaction->m_matched.push_back(a.str());
65-
i++;
63+
std::to_string(capture.m_group) + ": " + capture_substring);
64+
transaction->m_matched.push_back(capture_substring);
6665
}
6766
}
6867

69-
for (const auto & i : matches) {
70-
logOffset(ruleMessage, i.offset(), i.str().size());
68+
for (const auto & capture : captures) {
69+
logOffset(ruleMessage, capture.m_offset, capture.m_length);
7170
}
7271

7372
if (m_string->m_containsMacro) {
7473
delete re;
7574
}
7675

77-
if (matches.size() > 0) {
76+
if (captures.size() > 0) {
7877
return true;
7978
}
8079

‎src/utils/regex.cc

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,6 @@
1616
#include "src/utils/regex.h"
1717

1818
#include <pcre.h>
19-
#include <sys/socket.h>
20-
#include <sys/types.h>
21-
#include <netinet/in.h>
22-
#include <arpa/inet.h>
2319
#include <string>
2420
#include <list>
2521

@@ -99,6 +95,26 @@ std::list<SMatch> Regex::searchAll(const std::string& s) const {
9995
return retList;
10096
}
10197

98+
bool Regex::searchOneMatch(const std::string& s, std::vector<SMatchCapture>& captures) const {
99+
const char *subject = s.c_str();
100+
int ovector[OVECCOUNT];
101+
102+
int rc = pcre_exec(m_pc, m_pce, subject, s.size(), 0, 0, ovector, OVECCOUNT);
103+
104+
for (int i = 0; i < rc; i++) {
105+
size_t start = ovector[2*i];
106+
size_t end = ovector[2*i+1];
107+
size_t len = end - start;
108+
if (end > s.size()) {
109+
continue;
110+
}
111+
SMatchCapture capture(i, start, len);
112+
captures.push_back(capture);
113+
}
114+
115+
return (rc > 0);
116+
}
117+
102118
int Regex::search(const std::string& s, SMatch *match) const {
103119
int ovector[OVECCOUNT];
104120
int ret = pcre_exec(m_pc, m_pce, s.c_str(),

‎src/utils/regex.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <fstream>
2020
#include <string>
2121
#include <list>
22+
#include <vector>
2223

2324
#ifndef SRC_UTILS_REGEX_H_
2425
#define SRC_UTILS_REGEX_H_
@@ -47,6 +48,16 @@ class SMatch {
4748
size_t m_offset;
4849
};
4950

51+
struct SMatchCapture {
52+
SMatchCapture(size_t group, size_t offset, size_t length) :
53+
m_group(group),
54+
m_offset(offset),
55+
m_length(length) { }
56+
57+
size_t m_group; // E.g. 0 = full match; 6 = capture group 6
58+
size_t m_offset; // offset of match within the analyzed string
59+
size_t m_length;
60+
};
5061

5162
class Regex {
5263
public:
@@ -58,6 +69,7 @@ class Regex {
5869
Regex& operator=(const Regex&) = delete;
5970

6071
std::list<SMatch> searchAll(const std::string& s) const;
72+
bool searchOneMatch(const std::string& s, std::vector<SMatchCapture>& captures) const;
6173
int search(const std::string &s, SMatch *match) const;
6274
int search(const std::string &s) const;
6375

‎test/test-cases/regression/variable-TX.json

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,5 +80,143 @@
8080
"SecRule REQUEST_HEADERS \"@rx ([A-z]+)\" \"id:1,log,pass,capture,id:14\"",
8181
"SecRule TX:0 \"@rx ([A-z]+)\" \"id:15\""
8282
]
83+
},
84+
{
85+
"enabled":1,
86+
"version_min":300000,
87+
"title":"Testing Variables :: capture group match after unused group",
88+
"client":{
89+
"ip":"200.249.12.31",
90+
"port":123
91+
},
92+
"server":{
93+
"ip":"200.249.12.31",
94+
"port":80
95+
},
96+
"request":{
97+
"uri":"/?key=aadd",
98+
"method":"GET"
99+
},
100+
"response":{
101+
"headers":{
102+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
103+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
104+
"Content-Type":"text/html"
105+
},
106+
"body":[
107+
"no need."
108+
]
109+
},
110+
"expected":{
111+
"debug_log":"Added regex subexpression TX\\.3: dd[\\s\\S]*Target value: \"dd\" \\(Variable\\: TX\\:3[\\s\\S]*Rule returned 1"
112+
},
113+
"rules":[
114+
"SecRuleEngine On",
115+
"SecRule ARGS \"@rx (aa)(bb|cc)?(dd)\" \"id:1,log,pass,capture,id:16\"",
116+
"SecRule TX:3 \"@streq dd\" \"id:19,phase:2,log,pass\""
117+
]
118+
},
119+
{
120+
"enabled":1,
121+
"version_min":300000,
122+
"title":"Testing Variables :: empty capture group match followed by nonempty capture group",
123+
"client":{
124+
"ip":"200.249.12.31",
125+
"port":123
126+
},
127+
"server":{
128+
"ip":"200.249.12.31",
129+
"port":80
130+
},
131+
"request":{
132+
"uri":"/?key=aadd",
133+
"method":"GET"
134+
},
135+
"response":{
136+
"headers":{
137+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
138+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
139+
"Content-Type":"text/html"
140+
},
141+
"body":[
142+
"no need."
143+
]
144+
},
145+
"expected":{
146+
"debug_log":"Added regex subexpression TX\\.3: dd[\\s\\S]*Target value: \"dd\" \\(Variable\\: TX\\:3[\\s\\S]*Rule returned 1"
147+
},
148+
"rules":[
149+
"SecRuleEngine On",
150+
"SecRule ARGS \"@rx (aa)(bb|cc|)(dd)\" \"id:18,phase:1,log,pass,capture\"",
151+
"SecRule TX:3 \"@streq dd\" \"id:19,phase:2,log,pass\""
152+
]
153+
},
154+
{
155+
"enabled":1,
156+
"version_min":300000,
157+
"title":"Testing Variables :: repeating capture group -- alternates",
158+
"client":{
159+
"ip":"200.249.12.31",
160+
"port":123
161+
},
162+
"server":{
163+
"ip":"200.249.12.31",
164+
"port":80
165+
},
166+
"request":{
167+
"uri":"/?key=_abc123_",
168+
"method":"GET"
169+
},
170+
"response":{
171+
"headers":{
172+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
173+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
174+
"Content-Type":"text/html"
175+
},
176+
"body":[
177+
"no need."
178+
]
179+
},
180+
"expected":{
181+
"debug_log":"Added regex subexpression TX\\.2: abc[\\s\\S]*Added regex subexpression TX\\.3: 123"
182+
},
183+
"rules":[
184+
"SecRuleEngine On",
185+
"SecRule ARGS \"@rx _((?:(abc)|(123))+)_\" \"id:18,phase:1,log,pass,capture\""
186+
]
187+
},
188+
{
189+
"enabled":1,
190+
"version_min":300000,
191+
"title":"Testing Variables :: repeating capture group -- same (nested)",
192+
"client":{
193+
"ip":"200.249.12.31",
194+
"port":123
195+
},
196+
"server":{
197+
"ip":"200.249.12.31",
198+
"port":80
199+
},
200+
"request":{
201+
"uri":"/?key=a:5a:8a:9",
202+
"method":"GET"
203+
},
204+
"response":{
205+
"headers":{
206+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
207+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
208+
"Content-Type":"text/html"
209+
},
210+
"body":[
211+
"no need."
212+
]
213+
},
214+
"expected":{
215+
"debug_log":"Added regex subexpression TX\\.1: 5[\\s\\S]*Added regex subexpression TX\\.2: 8[\\s\\S]*Added regex subexpression TX\\.3: 9"
216+
},
217+
"rules":[
218+
"SecRuleEngine On",
219+
"SecRule ARGS \"@rx a:([0-9])(?:a:([0-9])(?:a:([0-9]))*)*\" \"id:18,phase:1,log,pass,capture\""
220+
]
83221
}
84222
]

0 commit comments

Comments
 (0)