Skip to content

Commit 846386a

Browse files
committed
Add proper error handling to @rx operator
1 parent 9278e00 commit 846386a

File tree

4 files changed

+102
-1
lines changed

4 files changed

+102
-1
lines changed

src/operators/rx.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ namespace operators {
2929

3030
bool Rx::init(const std::string &arg, std::string *error) {
3131
if (m_string->m_containsMacro == false) {
32+
std::string regex_error;
3233
m_re = new Regex(m_param);
34+
if (!m_re->ok(&regex_error)) {
35+
*error = "Failed to compile regular expression " + m_re->getPattern() + ": " + regex_error;
36+
return false;
37+
}
3338
}
3439

3540
return true;
@@ -47,6 +52,14 @@ bool Rx::evaluate(Transaction *transaction, Rule *rule,
4752
if (m_string->m_containsMacro) {
4853
std::string eparam(m_string->evaluate(transaction));
4954
re = new Regex(eparam);
55+
std::string regex_error;
56+
if (!re->ok(&regex_error)) {
57+
ms_dbg_a(transaction, 2,
58+
"Failed to compile regular expression with macro "
59+
+ re->getPattern() + ": " + regex_error);
60+
delete re;
61+
return false;
62+
}
5063
} else {
5164
re = m_re;
5265
}

src/regex/backend/pcre.cc

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,28 @@ Pcre::Pcre(const std::string& pattern_)
4343

4444
m_pc = pcre_compile(pattern.c_str(), PCRE_DOTALL|PCRE_MULTILINE,
4545
&errptr, &erroffset, NULL);
46+
if (m_pc == NULL) {
47+
m_error = "pcre_compile error at offset " + std::to_string(erroffset) + ": " + std::string(errptr);
48+
return;
49+
}
4650

4751
m_pce = pcre_study(m_pc, pcre_study_opt, &errptr);
52+
if (m_pce == NULL) {
53+
m_error = "pcre_study error: " + std::string(errptr);
54+
pcre_free(m_pc);
55+
m_pc = nullptr;
56+
return;
57+
}
4858

49-
pcre_fullinfo(m_pc, m_pce, PCRE_INFO_CAPTURECOUNT, &m_capture_count);
59+
int res = pcre_fullinfo(m_pc, m_pce, PCRE_INFO_CAPTURECOUNT, &m_capture_count);
60+
if (res != 0) {
61+
// N.B. There's no need to free m_pce here, because it's freed in
62+
// destructor anyway. However, m_pc must be freed and set to NULL
63+
// here so error condition is properly detected by ok() method.
64+
m_error = "pcre_fullinfo error: code " + std::to_string(res);
65+
pcre_free(m_pc);
66+
m_pc = nullptr;
67+
}
5068
}
5169

5270

src/regex/backend/pcre.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,17 @@ class Pcre {
4141
std::vector<RegexMatch> searchAll(const std::string& s, bool overlapping = false) const;
4242
bool search(const std::string &s, RegexMatch *m = nullptr, ssize_t max_groups = -1) const;
4343

44+
bool ok(std::string *error = nullptr) const {
45+
if (m_pc != NULL) {
46+
return true;
47+
}
48+
if (error != nullptr) {
49+
*error= m_error;
50+
}
51+
52+
return false;
53+
}
54+
4455
const std::string& getPattern() const {
4556
return pattern;
4657
};
@@ -51,6 +62,8 @@ class Pcre {
5162

5263
pcre *m_pc = NULL;
5364
pcre_extra *m_pce = NULL;
65+
66+
std::string m_error;
5467
};
5568

5669

test/test-cases/regression/operator-rx.json

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,62 @@
8585
"SecRuleEngine On",
8686
"SecRule REQUEST_HEADERS:Content-Length \"!^0$\" \"id:1,phase:2,pass,t:trim,block\""
8787
]
88+
},
89+
{
90+
"enabled":1,
91+
"version_min":300000,
92+
"version_max":0,
93+
"title":"Testing Operator :: @rx with invalid regular expression",
94+
"expected":{
95+
"parser_error":"Rules error.*Failed to compile regular expression \\(\\(value1\\):"
96+
},
97+
"rules":[
98+
"SecRuleEngine On",
99+
"SecRule ARGS \"@rx ((value1)\" \"id:1,phase:2,pass,t:trim\""
100+
]
101+
},
102+
{
103+
"enabled":1,
104+
"version_min":300000,
105+
"title":"Testing Operator :: @rx with invalid regular expression after macro expansion",
106+
"client":{
107+
"ip":"200.249.12.31",
108+
"port":123
109+
},
110+
"server":{
111+
"ip":"200.249.12.31",
112+
"port":80
113+
},
114+
"request":{
115+
"headers":{
116+
"Host":"localhost",
117+
"User-Agent":"curl/7.38.0",
118+
"Accept":"*/*",
119+
"Content-Length": "27",
120+
"Content-Type": "application/x-www-form-urlencoded"
121+
},
122+
"uri":"/",
123+
"method":"POST",
124+
"body": [
125+
"param1=value1&param2=value2"
126+
]
127+
},
128+
"response":{
129+
"headers":{
130+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
131+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
132+
"Content-Type":"text/html"
133+
},
134+
"body":[
135+
"no need."
136+
]
137+
},
138+
"expected":{
139+
"debug_log":"Failed to compile regular expression with macro \\(\\(\\):"
140+
},
141+
"rules":[
142+
"SecRuleEngine On",
143+
"SecRule ARGS \"@rx ((%{TX.DOESNT_NEED_TO_EXIST_IT_WILL_BE_AN_EMPTY_STRING})\" \"id:1,phase:2,pass,t:trim\""
144+
]
88145
}
89146
]

0 commit comments

Comments
 (0)