diff --git a/Makefile.am b/Makefile.am index fd0884092c..2ff2f6ad48 100644 --- a/Makefile.am +++ b/Makefile.am @@ -128,6 +128,7 @@ TESTS+=test/test-cases/regression/variable-TIME_MIN.json TESTS+=test/test-cases/regression/action-setuid.json TESTS+=test/test-cases/regression/action-setrsc.json TESTS+=test/test-cases/regression/issue-1152.json +TESTS+=test/test-cases/regression/issue-1528.json TESTS+=test/test-cases/regression/config-calling_phases_by_name.json TESTS+=test/test-cases/regression/variable-USERID.json TESTS+=test/test-cases/regression/request-body-parser-multipart.json diff --git a/src/operators/rx.cc b/src/operators/rx.cc index 9ed07cb975..746d60e2b7 100644 --- a/src/operators/rx.cc +++ b/src/operators/rx.cc @@ -33,12 +33,21 @@ bool Rx::evaluate(Transaction *transaction, Rule *rule, const std::string& input, std::shared_ptr ruleMessage) { SMatch match; std::list matches; + Regex * re = m_re; if (m_param.empty()) { return true; } - matches = m_re->searchAll(input); + if (m_macro_expansion_possible) { + std::string eparam = MacroExpansion::expand(m_param, transaction); + + if (eparam != m_param) { + re = new Regex(eparam); + } + } + + matches = re->searchAll(input); if (rule && rule->getActionsByName("capture").size() > 0 && transaction) { int i = 0; matches.reverse(); @@ -56,6 +65,10 @@ bool Rx::evaluate(Transaction *transaction, Rule *rule, logOffset(ruleMessage, i.m_offset, i.m_length); } + if (re != m_re) { + delete re; + } + if (matches.size() > 0) { return true; } @@ -64,5 +77,17 @@ bool Rx::evaluate(Transaction *transaction, Rule *rule, } +void Rx::checkIfMacroExpansionPossible() { + size_t pos = m_param.find("%{"); + + if (pos == std::string::npos) { + m_macro_expansion_possible = false; + return; + } + + m_macro_expansion_possible = (m_param.find('}', pos) != std::string::npos); +} + + } // namespace operators } // namespace modsecurity diff --git a/src/operators/rx.h b/src/operators/rx.h index 713aed2252..c2fd0c6473 100644 --- a/src/operators/rx.h +++ b/src/operators/rx.h @@ -38,14 +38,20 @@ class Rx : public Operator { Rx(std::string op, std::string param, bool negation) : Operator(op, param, negation) { m_re = new Regex(param); + + checkIfMacroExpansionPossible(); } Rx(std::string name, std::string param) : Operator(name, param) { m_re = new Regex(param); + + checkIfMacroExpansionPossible(); } explicit Rx(std::string param) : Operator("Rx", param) { m_re = new Regex(param); + + checkIfMacroExpansionPossible(); } ~Rx() { @@ -65,6 +71,9 @@ class Rx : public Operator { private: Regex *m_re; + bool m_macro_expansion_possible; + + void checkIfMacroExpansionPossible(); }; diff --git a/test/test-cases/regression/issue-1528.json b/test/test-cases/regression/issue-1528.json new file mode 100644 index 0000000000..1899fbc501 --- /dev/null +++ b/test/test-cases/regression/issue-1528.json @@ -0,0 +1,38 @@ +[ +{ + "enabled": 1, + "version_min": 209000, + "version_max": -1, + "title": "Macro expansion inside regex does not work", + "url": "https:\/\/github.com\/SpiderLabs\/ModSecurity\/issues\/1528", + "gihub_issue": 1528, + "client": { + "ip": "200.249.12.31", + "port": 2313 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "uri":"/?param=attack", + "headers": "", + "body": "", + "method": "GET", + "http_version": 1.1 + }, + "response": { + "headers": "", + "body": "" + }, + "expected": { + "debug_log": "Rule returned 1", + "error_log": "Matched \"Operator `Rx' with parameter `\\^%{tx\\.bad_value}\\$' against variable `ARGS:param' \\(Value: `attack' " + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:1, nolog, setvar:tx.bad_value=attack\"", + "SecRule ARGS:param \"@rx ^%{tx.bad_value}$\" id:2" + ] +} +]