Skip to content

Commit 183094a

Browse files
author
Felipe Zimmerle
committed
Fix RULE lookup in chained rules.
1 parent 51e9fb7 commit 183094a

File tree

4 files changed

+59
-24
lines changed

4 files changed

+59
-24
lines changed

headers/modsecurity/rule.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ class Rule {
140140
int m_phase;
141141
modsecurity::Variables::Variables *m_variables;
142142
operators::Operator *m_op;
143-
Rule *m_chainedRule;
143+
Rule *m_chainedRuleChild;
144+
Rule *m_chainedRuleParent;
144145
std::string m_fileName;
145146
std::string m_marker;
146147
std::string m_rev;

src/parser/driver.cc

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,22 +78,24 @@ int Driver::addSecRule(Rule *rule) {
7878
}
7979

8080
if (lastRule && lastRule->m_chained) {
81-
if (lastRule->m_chainedRule == NULL) {
81+
if (lastRule->m_chainedRuleChild == NULL) {
8282
rule->m_phase = lastRule->m_phase;
83-
lastRule->m_chainedRule = rule;
83+
lastRule->m_chainedRuleChild = rule;
84+
rule->m_chainedRuleParent = lastRule;
8485
if (rule->m_theDisruptiveAction) {
8586
m_parserError << "Disruptive actions can only be specified by";
8687
m_parserError << " chain starter rules.";
8788
return false;
8889
}
8990
return true;
9091
} else {
91-
Rule *a = lastRule->m_chainedRule;
92-
while (a->m_chained && a->m_chainedRule != NULL) {
93-
a = a->m_chainedRule;
92+
Rule *a = lastRule->m_chainedRuleChild;
93+
while (a->m_chained && a->m_chainedRuleChild != NULL) {
94+
a = a->m_chainedRuleChild;
9495
}
95-
if (a->m_chained && a->m_chainedRule == NULL) {
96-
a->m_chainedRule = rule;
96+
if (a->m_chained && a->m_chainedRuleChild == NULL) {
97+
a->m_chainedRuleChild = rule;
98+
rule->m_chainedRuleParent = a;
9799
if (a->m_theDisruptiveAction) {
98100
m_parserError << "Disruptive actions can only be ";
99101
m_parserError << "specified by chain starter rules.";

src/rule.cc

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ Rule::Rule(std::string marker)
5757
m_actionsSetVar(),
5858
m_actionsTag(),
5959
m_chained(false),
60-
m_chainedRule(NULL),
60+
m_chainedRuleChild(NULL),
6161
m_fileName(""),
6262
m_lineNumber(0),
6363
m_marker(marker),
@@ -91,7 +91,8 @@ Rule::Rule(Operator *_op,
9191
m_actionsSetVar(),
9292
m_actionsTag(),
9393
m_chained(false),
94-
m_chainedRule(NULL),
94+
m_chainedRuleChild(NULL),
95+
m_chainedRuleParent(NULL),
9596
m_fileName(fileName),
9697
m_lineNumber(lineNumber),
9798
m_marker(""),
@@ -146,8 +147,8 @@ Rule::~Rule() {
146147
delete m_variables;
147148
}
148149

149-
if (m_chainedRule != NULL) {
150-
delete m_chainedRule;
150+
if (m_chainedRuleChild != NULL) {
151+
delete m_chainedRuleChild;
151152
}
152153
}
153154

@@ -793,7 +794,7 @@ bool Rule::evaluate(Transaction *trans,
793794
goto end_exec;
794795
}
795796

796-
if (this->m_chainedRule == NULL) {
797+
if (this->m_chainedRuleChild == NULL) {
797798
#ifndef NO_LOGS
798799
trans->debug(4, "Rule is marked as chained but there " \
799800
"isn't a subsequent rule.");
@@ -804,7 +805,7 @@ bool Rule::evaluate(Transaction *trans,
804805
#ifndef NO_LOGS
805806
trans->debug(4, "Executing chained rule.");
806807
#endif
807-
recursiveGlobalRet = this->m_chainedRule->evaluate(trans, ruleMessage);
808+
recursiveGlobalRet = this->m_chainedRuleChild->evaluate(trans, ruleMessage);
808809

809810
if (recursiveGlobalRet == true) {
810811
goto end_exec;

src/variables/rule.h

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,17 @@ class Rule_DictElement : public VariableDictElement { \
4040
static void id(Transaction *t,
4141
Rule *rule,
4242
std::vector<const VariableValue *> *l) {
43-
if (!rule) {
43+
Rule *r = rule;
44+
45+
while (r && r->m_ruleId == 0) {
46+
r = r->m_chainedRuleParent;
47+
}
48+
49+
if (!r || r->m_ruleId == 0) {
4450
return;
4551
}
4652
std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
47-
std::string *a = new std::string(std::to_string(rule->m_ruleId));
53+
std::string *a = new std::string(std::to_string(r->m_ruleId));
4854
VariableValue *var = new VariableValue(
4955
std::make_shared<std::string>("RULE:id"),
5056
a
@@ -60,11 +66,18 @@ class Rule_DictElement : public VariableDictElement { \
6066
static void rev(Transaction *t,
6167
Rule *rule,
6268
std::vector<const VariableValue *> *l) {
63-
if (!rule) {
69+
Rule *r = rule;
70+
71+
while (r && r->m_rev.empty()) {
72+
r = r->m_chainedRuleParent;
73+
}
74+
75+
if (!r) {
6476
return;
6577
}
78+
6679
std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
67-
std::string *a = new std::string(rule->m_rev);
80+
std::string *a = new std::string(r->m_rev);
6881
VariableValue *var = new VariableValue(
6982
std::make_shared<std::string>("RULE:rev"),
7083
a
@@ -80,9 +93,15 @@ class Rule_DictElement : public VariableDictElement { \
8093
static void severity(Transaction *t,
8194
Rule *rule,
8295
std::vector<const VariableValue *> *l) {
83-
if (rule && rule->m_severity) {
96+
Rule *r = rule;
97+
98+
while (r && !r->m_severity) {
99+
r = r->m_chainedRuleParent;
100+
}
101+
102+
if (r && r->m_severity) {
84103
std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
85-
std::string *a = new std::string(std::to_string(rule->m_severity->m_severity));
104+
std::string *a = new std::string(std::to_string(r->m_severity->m_severity));
86105
VariableValue *var = new VariableValue(
87106
std::make_shared<std::string>("RULE:severity"),
88107
a
@@ -99,9 +118,15 @@ class Rule_DictElement : public VariableDictElement { \
99118
static void logData(Transaction *t,
100119
Rule *rule,
101120
std::vector<const VariableValue *> *l) {
102-
if (rule && rule->m_logData) {
121+
Rule *r = rule;
122+
123+
while (r && !r->m_logData) {
124+
r = r->m_chainedRuleParent;
125+
}
126+
127+
if (r && r->m_logData) {
103128
std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
104-
std::string *a = new std::string(rule->m_logData->data(t));
129+
std::string *a = new std::string(r->m_logData->data(t));
105130
VariableValue *var = new VariableValue(
106131
std::make_shared<std::string>("RULE:logdata"),
107132
a
@@ -117,9 +142,15 @@ class Rule_DictElement : public VariableDictElement { \
117142
static void msg(Transaction *t,
118143
Rule *rule,
119144
std::vector<const VariableValue *> *l) {
120-
if (rule && rule->m_msg) {
145+
Rule *r = rule;
146+
147+
while (r && !r->m_msg) {
148+
r = r->m_chainedRuleParent;
149+
}
150+
151+
if (r && r->m_msg) {
121152
std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
122-
std::string *a = new std::string(rule->m_msg->data(t));
153+
std::string *a = new std::string(r->m_msg->data(t));
123154
VariableValue *var = new VariableValue(
124155
std::make_shared<std::string>("RULE:msg"),
125156
a

0 commit comments

Comments
 (0)