github_mirrors/ModSecurity
3
0
Fork 1
mirror of https://github.com/owasp-modsecurity/ModSecurity.git synced 2025-08-15 23:55:03 +03:00
Code Issues Packages Projects Releases Wiki Activity

Makes Rule a shared pointer

Browse Source
This commit is contained in:
Felipe Zimmerle 2018-11-22 10:48:33 -03:00
parent f55ff85b36
commit 248cce8aff
No known key found for this signature in database
GPG Key ID: E6DFB08CE8B11277
11 changed files with 872 additions and 906 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
headers/modsecurity/rule.h
View File

@ -100,20 +100,6 @@ class Rule {
bool containsTag(const std::string& name, Transaction *t); bool containsTag(const std::string& name, Transaction *t);
bool containsMsg(const std::string& name, Transaction *t); bool containsMsg(const std::string& name, Transaction *t);
int refCountDecreaseAndCheck() {
m_referenceCount--;
if (m_referenceCount == 0) {
delete this;
return 1;
}
return 0;
}
void refCountIncrease() {
m_referenceCount++;
}
void executeTransformations( void executeTransformations(
actions::Action *a, actions::Action *a,
std::shared_ptr<std::string> newValue, std::shared_ptr<std::string> newValue,
@ -140,7 +126,7 @@ class Rule {
int m_phase; int m_phase;
modsecurity::Variables::Variables *m_variables; modsecurity::Variables::Variables *m_variables;
operators::Operator *m_op; operators::Operator *m_op;
Rule *m_chainedRuleChild; std::shared_ptr<Rule> m_chainedRuleChild;
Rule *m_chainedRuleParent; Rule *m_chainedRuleParent;
std::string m_fileName; std::string m_fileName;
std::string m_marker; std::string m_marker;
@ -152,7 +138,6 @@ class Rule {
std::vector<actions::Tag *> m_actionsTag; std::vector<actions::Tag *> m_actionsTag;
private: private:
bool m_unconditional; bool m_unconditional;
int m_referenceCount;
}; };

37
headers/modsecurity/rules.h
View File

@ -37,19 +37,19 @@
namespace modsecurity { namespace modsecurity {
class Rules : public std::vector<Rule *> { class Rules {
public: public:
void dump() { void dump() {
for (int j = 0; j < size(); j++) { for (int j = 0; j < m_rules.size(); j++) {
std::cout << " Rule ID: " << std::to_string(at(j)->m_ruleId); std::cout << " Rule ID: " << std::to_string(m_rules.at(j)->m_ruleId);
std::cout << "--" << at(j) << std::endl; std::cout << "--" << m_rules.at(j) << std::endl;
} }
} }
int append(Rules *from, const std::vector<int64_t> &ids, std::ostringstream *err) { int append(Rules *from, const std::vector<int64_t> &ids, std::ostringstream *err) {
size_t j = 0; size_t j = 0;
for (; j < from->size(); j++) { for (; j < from->size(); j++) {
Rule *rule = from->at(j); Rule *rule = from->at(j).get();
if (std::binary_search(ids.begin(), ids.end(), rule->m_ruleId)) { if (std::binary_search(ids.begin(), ids.end(), rule->m_ruleId)) {
if (err != NULL) { if (err != NULL) {
*err << "Rule id: " << std::to_string(rule->m_ruleId) \ *err << "Rule id: " << std::to_string(rule->m_ruleId) \
@ -57,11 +57,34 @@ class Rules : public std::vector<Rule *> {
} }
return -1; return -1;
} }
rule->refCountIncrease();
} }
insert(end(), from->begin(), from->end()); m_rules.insert(m_rules.end(), from->m_rules.begin(), from->m_rules.end());
return j; return j;
} }
bool insert(std::shared_ptr<Rule> rule) {
return insert(rule, nullptr, nullptr);
}
bool insert(std::shared_ptr<Rule> rule, const std::vector<int64_t> *ids, std::ostringstream *err) {
if (ids != nullptr && err != nullptr
&& std::binary_search(ids->begin(), ids->end(), rule->m_ruleId)) {
if (err != NULL) {
*err << "Rule id: " << std::to_string(rule->m_ruleId) \
<< " is duplicated" << std::endl;
}
return false;
}
m_rules.push_back(rule);
return true;
}
size_t size() { return m_rules.size(); }
std::shared_ptr<Rule> operator[](int index) { return m_rules[index]; }
std::shared_ptr<Rule> at(int index) { return m_rules[index]; }
std::vector<std::shared_ptr<Rule> > m_rules;
}; };

21
headers/modsecurity/rules_set_phases.h
View File

@ -31,26 +31,11 @@ class Driver;
/** @ingroup ModSecurity_CPP_API */ /** @ingroup ModSecurity_CPP_API */
class RulesSetPhases { class RulesSetPhases {
public: public:
bool insert(std::shared_ptr<Rule> rule) {
~RulesSetPhases() {
/** Cleanup the rules */
for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
Rules *rules = &m_rulesAtPhase[i];
while (rules->empty() == false) {
Rule *rule = rules->back();
rules->pop_back();
if (rule->refCountDecreaseAndCheck()) {
rule = NULL;
}
}
}
}
bool insert(Rule *rule) {
if (rule->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) { if (rule->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) {
return false; return false;
} }
m_rulesAtPhase[rule->m_phase].push_back(rule); m_rulesAtPhase[rule->m_phase].insert(std::move(rule));
return true; return true;
} }
@ -62,7 +47,7 @@ class RulesSetPhases {
for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
v.reserve(m_rulesAtPhase[i].size()); v.reserve(m_rulesAtPhase[i].size());
for (size_t z = 0; z < m_rulesAtPhase[i].size(); z++) { for (size_t z = 0; z < m_rulesAtPhase[i].size(); z++) {
Rule *rule_ckc = m_rulesAtPhase[i].at(z); Rule *rule_ckc = m_rulesAtPhase[i].at(z).get();
if (rule_ckc->m_secMarker == true) { if (rule_ckc->m_secMarker == true) {
continue; continue;
} }

53
src/parser/driver.cc
View File

@ -29,7 +29,7 @@ Driver::Driver()
: RulesSetProperties(), : RulesSetProperties(),
trace_scanning(false), trace_scanning(false),
trace_parsing(false), trace_parsing(false),
lastRule(NULL) { } m_lastRule(nullptr) { }
Driver::~Driver() { Driver::~Driver() {
@ -43,15 +43,15 @@ Driver::~Driver() {
int Driver::addSecMarker(std::string marker) { int Driver::addSecMarker(std::string marker) {
for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
Rule *rule = new Rule(marker); std::unique_ptr<Rule> rule(new Rule(marker));
rule->m_phase = i; rule->m_phase = i;
m_rulesSetPhases.insert(rule); m_rulesSetPhases.insert(std::move(rule));
} }
return 0; return 0;
} }
int Driver::addSecAction(Rule *rule) { int Driver::addSecAction(std::unique_ptr<Rule> rule) {
if (rule->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) { if (rule->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) {
m_parserError << "Unknown phase: " << std::to_string(rule->m_phase); m_parserError << "Unknown phase: " << std::to_string(rule->m_phase);
m_parserError << std::endl; m_parserError << std::endl;
@ -59,54 +59,38 @@ int Driver::addSecAction(Rule *rule) {
} }
m_rulesSetPhases.insert(rule); m_rulesSetPhases.insert(std::move(rule));
return true; return true;
} }
int Driver::addSecRuleScript(RuleScript *rule) { int Driver::addSecRuleScript(std::unique_ptr<RuleScript> rule) {
m_rulesSetPhases.insert(rule); m_rulesSetPhases.insert(std::move(rule));
return true; return true;
} }
int Driver::addSecRule(Rule *rule) { int Driver::addSecRule(std::unique_ptr<Rule> r) {
if (rule->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) { if (r->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) {
m_parserError << "Unknown phase: " << std::to_string(rule->m_phase); m_parserError << "Unknown phase: " << std::to_string(r->m_phase);
m_parserError << std::endl; m_parserError << std::endl;
return false; return false;
} }
std::shared_ptr<Rule> rule(std::move(r));
if (lastRule && lastRule->m_chained) { if (m_lastRule != nullptr && m_lastRule->m_chained) {
if (lastRule->m_chainedRuleChild == NULL) { rule->m_phase = m_lastRule->m_phase;
rule->m_phase = lastRule->m_phase;
if (rule->m_theDisruptiveAction) { if (rule->m_theDisruptiveAction) {
m_parserError << "Disruptive actions can only be specified by"; m_parserError << "Disruptive actions can only be specified by";
m_parserError << " chain starter rules."; m_parserError << " chain starter rules.";
return false; return false;
} }
lastRule->m_chainedRuleChild = rule; m_lastRule->m_chainedRuleChild = rule;
rule->m_chainedRuleParent = lastRule; rule->m_chainedRuleParent = m_lastRule.get();
return true; m_lastRule = rule;
} else {
Rule *a = lastRule->m_chainedRuleChild;
while (a->m_chained && a->m_chainedRuleChild != NULL) {
a = a->m_chainedRuleChild;
}
if (a->m_chained && a->m_chainedRuleChild == NULL) {
a->m_chainedRuleChild = rule;
rule->m_chainedRuleParent = a;
if (a->m_theDisruptiveAction) {
m_parserError << "Disruptive actions can only be ";
m_parserError << "specified by chain starter rules.";
return false;
}
return true; return true;
} }
}
}
/* /*
* Checking if the rule has an ID and also checking if this ID is not used * Checking if the rule has an ID and also checking if this ID is not used
@ -129,14 +113,15 @@ int Driver::addSecRule(Rule *rule) {
} }
} }
lastRule = rule; m_lastRule = rule;
m_rulesSetPhases.insert(rule); m_rulesSetPhases.insert(rule);
return true; return true;
} }
int Driver::parse(const std::string &f, const std::string &ref) { int Driver::parse(const std::string &f, const std::string &ref) {
lastRule = NULL; m_lastRule = nullptr;
loc.push_back(new yy::location()); loc.push_back(new yy::location());
if (ref.empty()) { if (ref.empty()) {
this->ref.push_back("<<reference missing or not informed>>"); this->ref.push_back("<<reference missing or not informed>>");

8
src/parser/driver.h
View File

@ -55,10 +55,10 @@ class Driver : public RulesSetProperties {
Driver(); Driver();
virtual ~Driver(); virtual ~Driver();
int addSecRule(Rule *rule); int addSecRule(std::unique_ptr<Rule> rule);
int addSecAction(Rule *rule); int addSecAction(std::unique_ptr<Rule> rule);
int addSecMarker(std::string marker); int addSecMarker(std::string marker);
int addSecRuleScript(RuleScript *rule); int addSecRuleScript(std::unique_ptr<RuleScript> rule);
bool scan_begin(); bool scan_begin();
void scan_end(); void scan_end();
@ -79,7 +79,7 @@ class Driver : public RulesSetProperties {
std::list<std::string> ref; std::list<std::string> ref;
std::string buffer; std::string buffer;
Rule *lastRule; std::shared_ptr<Rule> m_lastRule;
RulesSetPhases m_rulesSetPhases; RulesSetPhases m_rulesSetPhases;
}; };

1586
src/parser/seclang-parser.cc
View File

File diff suppressed because it is too large Load Diff

28
src/parser/seclang-parser.yy
View File

@ -1108,16 +1108,15 @@ expression:
} }
Operator *op = $3.release(); Operator *op = $3.release();
Rule *rule = new Rule( std::unique_ptr<Rule> rule(new Rule(
/* op */ op, /* op */ op,
/* variables */ v, /* variables */ v,
/* actions */ a, /* actions */ a,
/* file name */ driver.ref.back(), /* file name */ driver.ref.back(),
/* line number */ @1.end.line /* line number */ @1.end.line
); ));
if (driver.addSecRule(rule) == false) { if (driver.addSecRule(std::move(rule)) == false) {
delete rule;
YYERROR; YYERROR;
} }
} }
@ -1128,15 +1127,14 @@ expression:
v->push_back(i.release()); v->push_back(i.release());
} }
Rule *rule = new Rule( std::unique_ptr<Rule> rule(new Rule(
/* op */ $3.release(), /* op */ $3.release(),
/* variables */ v, /* variables */ v,
/* actions */ NULL, /* actions */ NULL,
/* file name */ driver.ref.back(), /* file name */ driver.ref.back(),
/* line number */ @1.end.line /* line number */ @1.end.line
); ));
if (driver.addSecRule(rule) == false) { if (driver.addSecRule(std::move(rule)) == false) {
delete rule;
YYERROR; YYERROR;
} }
} }
@ -1146,14 +1144,14 @@ expression:
for (auto &i : *$2.get()) { for (auto &i : *$2.get()) {
a->push_back(i.release()); a->push_back(i.release());
} }
Rule *rule = new Rule( std::unique_ptr<Rule> rule(new Rule(
/* op */ NULL, /* op */ NULL,
/* variables */ NULL, /* variables */ NULL,
/* actions */ a, /* actions */ a,
/* file name */ driver.ref.back(), /* file name */ driver.ref.back(),
/* line number */ @1.end.line /* line number */ @1.end.line
); ));
driver.addSecAction(rule); driver.addSecAction(std::move(rule));
} }
| DIRECTIVE_SECRULESCRIPT actions | DIRECTIVE_SECRULESCRIPT actions
{ {
@ -1162,20 +1160,18 @@ expression:
for (auto &i : *$2.get()) { for (auto &i : *$2.get()) {
a->push_back(i.release()); a->push_back(i.release());
} }
RuleScript *r = new RuleScript( std::unique_ptr<RuleScript> r(new RuleScript(
/* path to script */ $1, /* path to script */ $1,
/* actions */ a, /* actions */ a,
/* file name */ driver.ref.back(), /* file name */ driver.ref.back(),
/* line number */ @1.end.line /* line number */ @1.end.line
); ));
if (r->init(&err) == false) { if (r->init(&err) == false) {
driver.error(@0, "Failed to load script: " + err); driver.error(@0, "Failed to load script: " + err);
delete r;
YYERROR; YYERROR;
} }
if (driver.addSecRuleScript(r) == false) { if (driver.addSecRuleScript(std::move(r)) == false) {
delete r;
YYERROR; YYERROR;
} }
} }

6
src/rule.cc
View File

@ -71,7 +71,6 @@ Rule::Rule(std::string marker)
m_variables(NULL), m_variables(NULL),
m_ver(""), m_ver(""),
m_unconditional(false), m_unconditional(false),
m_referenceCount(1),
m_theDisruptiveAction(nullptr), m_theDisruptiveAction(nullptr),
m_containsStaticBlockAction(false), m_containsStaticBlockAction(false),
m_containsCaptureAction(false), m_containsCaptureAction(false),
@ -106,7 +105,6 @@ Rule::Rule(Operator *_op,
m_variables(_variables), m_variables(_variables),
m_ver(""), m_ver(""),
m_unconditional(false), m_unconditional(false),
m_referenceCount(1),
m_theDisruptiveAction(nullptr), m_theDisruptiveAction(nullptr),
m_containsStaticBlockAction(false), m_containsStaticBlockAction(false),
m_containsCaptureAction(false), m_containsCaptureAction(false),
@ -147,10 +145,6 @@ Rule::~Rule() {
if (m_variables != NULL) { if (m_variables != NULL) {
delete m_variables; delete m_variables;
} }
if (m_chainedRuleChild != NULL) {
delete m_chainedRuleChild;
}
} }

4
src/rules_set.cc
View File

@ -155,7 +155,7 @@ int RulesSet::evaluate(int phase, Transaction *t) {
return 0; return 0;
} }
std::vector<Rule *> *rules = m_rulesSetPhases[phase]; Rules *rules = m_rulesSetPhases[phase];
ms_dbg_a(t, 9, "This phase consists of " \ ms_dbg_a(t, 9, "This phase consists of " \
+ std::to_string(rules->size()) + " rule(s)."); + std::to_string(rules->size()) + " rule(s).");
@ -177,7 +177,7 @@ int RulesSet::evaluate(int phase, Transaction *t) {
} }
for (int i = 0; i < rules->size(); i++) { for (int i = 0; i < rules->size(); i++) {
Rule *rule = rules->at(i); Rule *rule = rules->at(i).get();
if (t->m_marker.empty() == false) { if (t->m_marker.empty() == false) {
ms_dbg_a(t, 9, "Skipped rule id '" + std::to_string(rule->m_ruleId) \ ms_dbg_a(t, 9, "Skipped rule id '" + std::to_string(rule->m_ruleId) \
+ "' due to a SecMarker: " + t->m_marker); + "' due to a SecMarker: " + t->m_marker);

6
test/optimization/optimization.cc
View File

@ -68,7 +68,7 @@ int main(int argc, char **argv) {
int nphases = modsecurity::Phases::NUMBER_OF_PHASES; int nphases = modsecurity::Phases::NUMBER_OF_PHASES;
for (int i = 0; i < nphases; i++) { for (int i = 0; i < nphases; i++) {
std::vector<Rule *> *rules = modsecRules->m_rulesSetPhases[i]; Rules *rules = modsecRules->m_rulesSetPhases[i];
if (rules->size() == 0) { if (rules->size() == 0) {
continue; continue;
} }
@ -79,7 +79,9 @@ int main(int argc, char **argv) {
std::unordered_map<std::string, int> operators; std::unordered_map<std::string, int> operators;
std::unordered_map<std::string, int> variables; std::unordered_map<std::string, int> variables;
std::unordered_map<std::string, int> op2var; std::unordered_map<std::string, int> op2var;
for (auto &z : *rules) {
for (int i = 0; i < rules->size(); i++) {
std::shared_ptr<Rule> z = rules->at(i);
std::string key; std::string key;
if (z == NULL) { if (z == NULL) {
continue; continue;

2
test/test-cases/regression/issue-1831.json
View File

@ -191,7 +191,7 @@
"rules": [ "rules": [
"SecRuleEngine On", "SecRuleEngine On",
"SecRule ARGS \"@rx .\" \"id:954100,phase:1,block,capture,t:none,t:lowercase,msg:'Disclosure of IIS install location',logdata:'Matched Data',tag:'application-multi',tag:'language-multi',tag:'platform-iis',tag:'platform-windows',tag:'attack-disclosure',ctl:auditLogParts=+E,rev:3,ver:'OWASP_CRS/3.0.0',severity:'ERROR',chain\"", "SecRule ARGS \"@rx .\" \"id:954100,phase:1,block,capture,t:none,t:lowercase,msg:'Disclosure of IIS install location',logdata:'Matched Data',tag:'application-multi',tag:'language-multi',tag:'platform-iis',tag:'platform-windows',tag:'attack-disclosure',ctl:auditLogParts=+E,rev:3,ver:'OWASP_CRS/3.0.0',severity:'ERROR',chain\"",
"SecRule &GLOBAL:alerted_970018_iisDefLoc \"@eq 0\" \"setvar:'global.alerted_970018_iisDefLoc',setvar:'tx.msg=%{rule.msg}',setvar:'tx.outbound_anomaly_score=+%{tx.error_anomaly_score}',setvar:'tx.anomaly_score=+%{tx.error_anomaly_score}'\"" "SecRule ARGS \"@eq 0\" \"setvar:'global.alerted_970018_iisDefLoc',setvar:'tx.msg=%{rule.msg}',setvar:'tx.outbound_anomaly_score=+%{tx.error_anomaly_score}',setvar:'tx.anomaly_score=+%{tx.error_anomaly_score}'\""
] ]
} }
] ]