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

Splits Rule class into: Rule, RuleBase, RuleMarker

Browse Source
This commit is contained in:
Felipe Zimmerle 2019-02-18 15:42:13 -03:00
parent fda03c0016
commit 43f8aee6b6
No known key found for this signature in database
GPG Key ID: E6DFB08CE8B11277
18 changed files with 948 additions and 851 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

96
headers/modsecurity/rule.h
View File

@ -26,6 +26,7 @@
#define HEADERS_MODSECURITY_RULE_H_
#include "modsecurity/transaction.h"
#include "modsecurity/modsecurity.h"
#include "modsecurity/variable_value.h"
@ -63,7 +64,80 @@ using Tags = std::vector<actions::Tag *>;
using SetVars = std::vector<actions::SetVar *>;
using MatchActions = std::vector<actions::Action *>;
class Rule {
class RuleBase {
public:
RuleBase(std::unique_ptr<std::string> fileName, int lineNumber)
: m_fileName(std::move(fileName)),
m_lineNumber(lineNumber),
m_phase(modsecurity::Phases::RequestHeadersPhase) {
}
virtual bool evaluate(Transaction *transaction,
std::shared_ptr<RuleMessage> rm) = 0;
std::shared_ptr<std::string> getFileName() const {
return m_fileName;
}
int getLineNumber() const {
return m_lineNumber;
}
int getPhase() const { return m_phase; }
void setPhase(int phase) { m_phase = phase; }
virtual std::string getReference() {
return *m_fileName + ":" + std::to_string(m_lineNumber);
}
virtual bool isMarker() { return false; }
private:
std::shared_ptr<std::string> m_fileName;
int m_lineNumber;
// FIXME: phase may not be neede to SecMarker.
int m_phase;
};
class RuleMarker : public RuleBase {
public:
RuleMarker(
const std::string &name,
std::unique_ptr<std::string> fileName,
int lineNumber)
: RuleBase(std::move(fileName), lineNumber),
m_name(std::make_shared<std::string>(name)) { }
virtual bool evaluate(Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override {
if (transaction->isInsideAMarker()) {
if (*transaction->getCurrentMarker() == *m_name) {
transaction->removeMarker();
// FIXME: Move this to .cc
// ms_dbg_a(transaction, 4, "Out of a SecMarker " + *m_name);
}
}
return true;
};
std::shared_ptr<std::string> getName() const {
return m_name;
}
bool isMarker() override { return true; }
private:
std::shared_ptr<std::string> m_name;
};
class Rule : public RuleBase {
public:
Rule(operators::Operator *op,
variables::Variables *variables,
@ -71,11 +145,13 @@ class Rule {
Transformations *transformations,
std::unique_ptr<std::string> fileName,
int lineNumber);
explicit Rule(const std::string &marker);
explicit Rule(const std::string &marker,
std::unique_ptr<std::string> fileName,
int lineNumber);
virtual ~Rule();
virtual bool evaluate(Transaction *transaction,
std::shared_ptr<RuleMessage> rm);
std::shared_ptr<RuleMessage> rm) override;
void organizeActions(std::vector<actions::Action *> *actions);
void cleanUpActions();
@ -119,8 +195,6 @@ class Rule {
inline bool isUnconditional() const { return m_operator == NULL; }
virtual bool isMarker() { return m_isSecMarker; }
inline bool isChained() const { return m_isChained == true; }
inline bool hasCaptureAction() const { return m_containsCaptureAction == true; }
inline void setChained(bool b) { m_isChained = b; }
@ -133,23 +207,23 @@ class Rule {
std::string msg(Transaction *t);
inline bool hasSeverity() const { return m_severity != NULL; }
int severity() const;
int getPhase() const { return m_phase; }
void setPhase(int phase) { m_phase = phase; }
std::string getOperatorName() const;
int64_t m_ruleId;
virtual std::string getReference() override {
return std::to_string(m_ruleId);
}
std::unique_ptr<Rule> m_chainedRuleChild;
Rule *m_chainedRuleParent;
std::shared_ptr<std::string> m_fileName;
std::string m_marker;
std::string m_rev;
std::string m_ver;
int m_accuracy;
int m_maturity;
int m_lineNumber;
private:
modsecurity::variables::Variables *m_variables;
@ -174,7 +248,7 @@ class Rule {
bool m_isSecMarker:1;
bool m_unconditional:1;
int m_phase;
};

4
headers/modsecurity/rule_message.h
View File

@ -55,9 +55,9 @@ class RuleMessage {
m_reference(""),
m_rev(rule->m_rev),
m_rule(rule),
m_ruleFile(rule->m_fileName),
m_ruleFile(rule->getFileName()),
m_ruleId(rule->m_ruleId),
m_ruleLine(rule->m_lineNumber),
m_ruleLine(rule->getLineNumber()),
m_saveMessage(true),
m_serverIpAddress(trans->m_serverIpAddress),
m_severity(0),

22
headers/modsecurity/rules.h
View File

@ -40,7 +40,7 @@ class Rules {
public:
void dump() const {
for (int j = 0; j < m_rules.size(); j++) {
std::cout << " Rule ID: " << std::to_string(m_rules.at(j)->m_ruleId);
std::cout << " Rule ID: " << m_rules.at(j)->getReference();
std::cout << "--" << m_rules.at(j) << std::endl;
}
}
@ -48,8 +48,8 @@ class Rules {
int append(Rules *from, const std::vector<int64_t> &ids, std::ostringstream *err) {
size_t j = 0;
for (; j < from->size(); j++) {
Rule *rule = from->at(j).get();
if (std::binary_search(ids.begin(), ids.end(), rule->m_ruleId)) {
Rule *rule = dynamic_cast<Rule *>(from->at(j).get());
if (rule && 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;
@ -61,28 +61,28 @@ class Rules {
return j;
}
bool insert(std::shared_ptr<Rule> rule) {
bool insert(std::shared_ptr<RuleBase> 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 && std::binary_search(ids->begin(), ids->end(), rule->m_ruleId)) {
bool insert(std::shared_ptr<RuleBase> rule, const std::vector<int64_t> *ids, std::ostringstream *err) {
Rule *r = dynamic_cast<Rule *>(rule.get());
if (ids != nullptr && std::binary_search(ids->begin(), ids->end(), r->m_ruleId)) {
if (err != nullptr) {
*err << "Rule id: " << std::to_string(rule->m_ruleId) \
*err << "Rule id: " << std::to_string(r->m_ruleId) \
<< " is duplicated" << std::endl;
}
return false;
}
m_rules.push_back(rule);
return true;
}
size_t size() const { return m_rules.size(); }
std::shared_ptr<Rule> operator[](int index) const { return m_rules[index]; }
std::shared_ptr<Rule> at(int index) const { return m_rules[index]; }
std::shared_ptr<RuleBase> operator[](int index) const { return m_rules[index]; }
std::shared_ptr<RuleBase> at(int index) const { return m_rules[index]; }
std::vector<std::shared_ptr<Rule> > m_rules;
std::vector<std::shared_ptr<RuleBase> > m_rules;
};

2
headers/modsecurity/rules_set_phases.h
View File

@ -40,7 +40,7 @@ class Rule;
class RulesSetPhases {
public:
bool insert(std::shared_ptr<Rule> rule);
bool insert(std::shared_ptr<RuleBase> rule);
int append(RulesSetPhases *from, std::ostringstream *err);
void dump() const;

39
headers/modsecurity/transaction.h
View File

@ -26,6 +26,7 @@
#include <utility>
#include <vector>
#include <memory>
#include <stack>
#endif
#include <stdlib.h>
@ -48,6 +49,7 @@ typedef struct Rules_t RulesSet;
#include "modsecurity/collection/collection.h"
#include "modsecurity/variable_origin.h"
#ifndef NO_LOGS
#define ms_dbg(b, c) \
do { \
@ -284,9 +286,38 @@ class TransactionAnchoredVariables {
int m_variableOffset;
};
class TransactionSecMarkerManagement {
public:
bool isInsideAMarker() const {
if (m_marker) {
return true;
}
return false;
}
std::shared_ptr<std::string> getCurrentMarker() const {
if (m_marker) {
return m_marker;
} else {
throw;
}
}
void removeMarker() {
m_marker.reset();
}
void addMarker(std::shared_ptr<std::string> name) {
m_marker = name;
}
private:
std::shared_ptr<std::string> m_marker;
};
/** @ingroup ModSecurity_CPP_API */
class Transaction : public TransactionAnchoredVariables {
class Transaction : public TransactionAnchoredVariables, public TransactionSecMarkerManagement {
public:
Transaction(ModSecurity *transaction, RulesSet *rules, void *logCbData);
Transaction(ModSecurity *transaction, RulesSet *rules, char *id,
@ -518,12 +549,6 @@ class Transaction : public TransactionAnchoredVariables {
*/
std::shared_ptr<std::string> m_id;
/**
* Holds the SecMarker name that this transaction should wait to perform
* rules evaluation again.
*/
std::string m_marker;
/**
* Holds the amount of rules that should be skipped. If bigger than 0 the
* current rule should be skipped and the number needs to be decreased.

4
src/actions/skip_after.cc
View File

@ -28,8 +28,8 @@ namespace actions {
bool SkipAfter::evaluate(Rule *rule, Transaction *transaction) {
ms_dbg_a(transaction, 5, "Setting skipAfter for: " + m_parser_payload);
transaction->m_marker = m_parser_payload;
ms_dbg_a(transaction, 5, "Setting skipAfter for: " + *m_skipName);
transaction->addMarker(m_skipName);
return true;
}

6
src/actions/skip_after.h
View File

@ -14,6 +14,7 @@
*/
#include <string>
#include <memory>
#include "modsecurity/actions/action.h"
@ -30,9 +31,12 @@ namespace actions {
class SkipAfter : public Action {
public:
explicit SkipAfter(const std::string &action)
: Action(action, RunTimeOnlyIfMatchKind) { }
: Action(action, RunTimeOnlyIfMatchKind),
m_skipName(std::make_shared<std::string>(m_parser_payload)) { }
bool evaluate(Rule *rule, Transaction *transaction) override;
private:
std::shared_ptr<std::string> m_skipName;
};

14
src/parser/driver.cc
View File

@ -41,9 +41,11 @@ Driver::~Driver() {
}
int Driver::addSecMarker(std::string marker) {
int Driver::addSecMarker(std::string marker, std::unique_ptr<std::string> fileName, int lineNumber) {
// FIXME: we might move this to the parser.
for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
std::unique_ptr<Rule> rule(new Rule(marker));
RuleMarker *r = new RuleMarker(marker, std::move(fileName), lineNumber);
std::unique_ptr<RuleMarker> rule(std::move(r));
rule->setPhase(i);
m_rulesSetPhases.insert(std::move(rule));
}
@ -58,7 +60,6 @@ int Driver::addSecAction(std::unique_ptr<Rule> rule) {
return false;
}
m_rulesSetPhases.insert(std::move(rule));
return true;
@ -99,15 +100,16 @@ int Driver::addSecRule(std::unique_ptr<Rule> r) {
*/
if (rule->m_ruleId == 0) {
m_parserError << "Rules must have an ID. File: ";
m_parserError << rule->m_fileName << " at line: ";
m_parserError << std::to_string(rule->m_lineNumber) << std::endl;
m_parserError << rule->getFileName() << " at line: ";
m_parserError << std::to_string(rule->getLineNumber()) << std::endl;
return false;
}
for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
Rules *rules = m_rulesSetPhases[i];
for (int j = 0; j < rules->size(); j++) {
if (rules->at(j)->m_ruleId == rule->m_ruleId) {
Rule *lr = dynamic_cast<Rule *>(rules->at(j).get());
if (lr && lr->m_ruleId == rule->m_ruleId) {
m_parserError << "Rule id: " << std::to_string(rule->m_ruleId) \
<< " is duplicated" << std::endl;
return false;

2
src/parser/driver.h
View File

@ -68,7 +68,7 @@ class Driver : public RulesSetProperties {
int addSecRule(std::unique_ptr<Rule> rule);
int addSecAction(std::unique_ptr<Rule> rule);
int addSecMarker(std::string marker);
int addSecMarker(std::string marker, std::unique_ptr<std::string> fileName, int lineNumber);
int addSecRuleScript(std::unique_ptr<RuleScript> rule);
bool scan_begin();

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

File diff suppressed because it is too large Load Diff

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

@ -1221,7 +1221,10 @@ expression:
}
| CONFIG_DIR_SEC_MARKER
{
driver.addSecMarker(modsecurity::utils::string::removeBracketsIfNeeded($1));
driver.addSecMarker(modsecurity::utils::string::removeBracketsIfNeeded($1),
/* file name */ std::unique_ptr<std::string>(new std::string(*@1.end.filename)),
/* line number */ @1.end.line
);
}
| CONFIG_DIR_RULE_ENG CONFIG_VALUE_OFF
{

32
src/rule.cc
View File

@ -51,17 +51,18 @@ using variables::Variable;
using actions::transformations::None;
using actions::transformations::Transformation;
Rule::Rule(const std::string &marker)
: m_ruleId(0),
Rule::Rule(const std::string &marker,
std::unique_ptr<std::string> fileName,
int lineNumber)
: RuleBase(std::move(fileName), lineNumber),
m_ruleId(0),
m_chainedRuleChild(nullptr),
m_chainedRuleParent(NULL),
/* m_fileName(""), */
m_marker(marker),
m_rev(""),
m_ver(""),
m_accuracy(0),
m_maturity(0),
m_lineNumber(0),
m_variables(NULL),
m_operator(NULL),
m_disruptiveAction(nullptr),
@ -77,8 +78,7 @@ Rule::Rule(const std::string &marker)
m_containsStaticBlockAction(false),
m_isChained(false),
m_isSecMarker(true),
m_unconditional(false),
m_phase(-1) { }
m_unconditional(false) { }
Rule::Rule(Operator *op,
variables::Variables *variables,
@ -86,16 +86,15 @@ Rule::Rule(Operator *op,
Transformations *transformations,
std::unique_ptr<std::string> fileName,
int lineNumber)
: m_ruleId(0),
: RuleBase(std::move(fileName), lineNumber),
m_ruleId(0),
m_chainedRuleChild(nullptr),
m_chainedRuleParent(NULL),
m_fileName(std::move(fileName)),
m_marker(""),
m_rev(""),
m_ver(""),
m_accuracy(0),
m_maturity(0),
m_lineNumber(lineNumber),
m_variables(variables),
m_operator(op),
m_disruptiveAction(nullptr),
@ -111,19 +110,10 @@ Rule::Rule(Operator *op,
m_containsStaticBlockAction(false),
m_isChained(false),
m_isSecMarker(false),
m_unconditional(false),
m_phase(-1) {
m_unconditional(false) {
organizeActions(actions);
/**
* If phase is not entered, we assume phase 2. For historical reasons.
*
*/
if (m_phase == -1) {
m_phase = modsecurity::Phases::RequestHeadersPhase;
}
delete actions;
}
@ -381,7 +371,7 @@ void Rule::executeTransformations(
// Notice that first we make sure that won't be a t:none
// on the target rule.
if (none == 0) {
for (auto &a : trans->m_rules->m_defaultActions[this->m_phase]) {
for (auto &a : trans->m_rules->m_defaultActions[getPhase()]) {
if (a->action_kind \
!= actions::Action::RunTimeBeforeMatchAttemptKind) {
continue;
@ -566,7 +556,7 @@ void Rule::executeActionsAfterFullMatch(Transaction *trans,
bool containsBlock, std::shared_ptr<RuleMessage> ruleMessage) {
bool disruptiveAlreadyExecuted = false;
for (auto &a : trans->m_rules->m_defaultActions[this->m_phase]) {
for (auto &a : trans->m_rules->m_defaultActions[getPhase()]) {
if (a.get()->action_kind != actions::Action::RunTimeOnlyIfMatchKind) {
continue;
}

50
src/rules_set.cc
View File

@ -133,42 +133,38 @@ int RulesSet::evaluate(int phase, Transaction *t) {
//}
for (int i = 0; i < rules->size(); i++) {
Rule *rule = rules->at(i).get();
if (t->m_marker.empty() == false) {
ms_dbg_a(t, 9, "Skipped rule id '" + std::to_string(rule->m_ruleId) \
+ "' due to a SecMarker: " + t->m_marker);
#ifndef NO_LOGS
m_secmarker_skipped++;
#endif
ms_dbg_a(t, 9, "Rule: " + rule->m_marker);
// FIXME: This is not meant to be here. At the end of this refactoring,
// the shared pointer won't be used.
std::shared_ptr<RuleBase> rule = rules->at(i);
if (t->isInsideAMarker() && !rule->isMarker()) {
ms_dbg_a(t, 9, "Skipped rule id '" + rule->getReference() \
+ "' due to a SecMarker: " + *t->getCurrentMarker());
if (rule->isMarker() && rule->m_marker == t->m_marker) {
ms_dbg_a(t, 4, "Out of a SecMarker after skip " \
+ std::to_string(m_secmarker_skipped) + " rules.");
t->m_marker.clear();
#ifndef NO_LOGS
m_secmarker_skipped = 0;
#endif
}
} else if (rule->isMarker()) {
rule->evaluate(t, NULL);
} else if (t->m_skip_next > 0) {
t->m_skip_next--;
ms_dbg_a(t, 9, "Skipped rule id '" + std::to_string(rule->m_ruleId) \
ms_dbg_a(t, 9, "Skipped rule id '" + rule->getReference() \
+ "' due to a `skip' action. Still " + \
std::to_string(t->m_skip_next) + " to be skipped.");
} else if (t->m_allowType
!= actions::disruptive::NoneAllowType) {
ms_dbg_a(t, 9, "Skipped rule id '" + std::to_string(rule->m_ruleId) \
ms_dbg_a(t, 9, "Skipped rule id '" + rule->getReference() \
+ "' as request trough the utilization of an `allow' action.");
} else if (m_exceptions.contains(rule->m_ruleId)) {
ms_dbg_a(t, 9, "Skipped rule id '" + std::to_string(rule->m_ruleId) \
+ "'. Removed by an SecRuleRemove directive.");
} else {
RuleBase *base = rule.get();
Rule *ruleWithOperator = dynamic_cast<Rule *>(base);
if (m_exceptions.contains(ruleWithOperator->m_ruleId)) {
ms_dbg_a(t, 9, "Skipped rule id '" + rule->getReference() \
+ "'. Removed by an SecRuleRemove directive.");
continue;
}
bool remove_rule = false;
if (m_exceptions.m_remove_rule_by_msg.empty() == false) {
for (auto &z : m_exceptions.m_remove_rule_by_msg) {
if (rule->containsMsg(z, t) == true) {
if (ruleWithOperator->containsMsg(z, t) == true) {
ms_dbg_a(t, 9, "Skipped rule id '" \
+ std::to_string(rule->m_ruleId) \
+ ruleWithOperator->getReference() \
+ "'. Removed by a SecRuleRemoveByMsg directive.");
remove_rule = true;
break;
@ -181,9 +177,9 @@ int RulesSet::evaluate(int phase, Transaction *t) {
if (m_exceptions.m_remove_rule_by_tag.empty() == false) {
for (auto &z : m_exceptions.m_remove_rule_by_tag) {
if (rule->containsTag(z, t) == true) {
if (ruleWithOperator->containsTag(z, t) == true) {
ms_dbg_a(t, 9, "Skipped rule id '" \
+ std::to_string(rule->m_ruleId) \
+ ruleWithOperator->getReference() \
+ "'. Removed by a SecRuleRemoveByTag directive.");
remove_rule = true;
break;
@ -196,9 +192,9 @@ int RulesSet::evaluate(int phase, Transaction *t) {
if (t->m_ruleRemoveByTag.empty() == false) {
for (auto &z : t->m_ruleRemoveByTag) {
if (rule->containsTag(z, t) == true) {
if (ruleWithOperator->containsTag(z, t) == true) {
ms_dbg_a(t, 9, "Skipped rule id '" \
+ std::to_string(rule->m_ruleId) \
+ ruleWithOperator->getReference() \
+ "'. Skipped due to a ruleRemoveByTag action.");
remove_rule = true;
break;

6
src/rules_set_phases.cc
View File

@ -29,7 +29,7 @@
namespace modsecurity {
bool RulesSetPhases::insert(std::shared_ptr<Rule> rule) {
bool RulesSetPhases::insert(std::shared_ptr<RuleBase> rule) {
if (rule->getPhase() >= modsecurity::Phases::NUMBER_OF_PHASES) {
return false;
}
@ -46,8 +46,8 @@ int RulesSetPhases::append(RulesSetPhases *from, std::ostringstream *err) {
for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
v.reserve(m_rulesAtPhase[i].size());
for (size_t z = 0; z < m_rulesAtPhase[i].size(); z++) {
Rule *rule_ckc = m_rulesAtPhase[i].at(z).get();
if (rule_ckc->isMarker() == true) {
Rule *rule_ckc = dynamic_cast<Rule *>(m_rulesAtPhase[i].at(z).get());
if (!rule_ckc) {
continue;
}
v.push_back(rule_ckc->m_ruleId);

2
src/transaction.cc
View File

@ -126,7 +126,6 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, void *logCbData)
m_requestBody(),
m_responseBody(),
/* m_id(), */
m_marker(""),
m_skip_next(0),
m_allowType(modsecurity::actions::disruptive::NoneAllowType),
m_uri_decoded(""),
@ -200,7 +199,6 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, char *id, void *logCb
m_requestBody(),
m_responseBody(),
m_id(std::unique_ptr<std::string>(new std::string(id))),
m_marker(""),
m_skip_next(0),
m_allowType(modsecurity::actions::disruptive::NoneAllowType),
m_uri_decoded(""),

2
test/cppcheck_suppressions.txt
View File

@ -35,7 +35,7 @@ invalidScanfArgType_int:src/rules_set_properties.cc:102
unmatchedSuppression:src/utils/geo_lookup.cc:82
useInitializationList:src/utils/shared_files.h:87
unmatchedSuppression:src/utils/msc_tree.cc
functionStatic:headers/modsecurity/transaction.h:373
functionStatic:headers/modsecurity/transaction.h:404
duplicateBranch:src/audit_log/audit_log.cc:223
unreadVariable:src/request_body_processor/multipart.cc:435
stlcstrParam:src/audit_log/writer/parallel.cc:145

6
test/optimization/optimization.cc
View File

@ -81,11 +81,12 @@ int main(int argc, char **argv) {
std::unordered_map<std::string, int> op2var;
for (int i = 0; i < rules->size(); i++) {
std::shared_ptr<Rule> z = rules->at(i);
std::string key;
auto z = rules->at(i);
//std::string key;
if (z == NULL) {
continue;
}
#if 0
if (z->isUnconditional() == false) {
std::string op = z->getOperatorName();
if (operators.count(op) > 0) {
@ -95,6 +96,7 @@ int main(int argc, char **argv) {
}
key = op;
}
#endif
#if 0
FIXME: This test may not be useful anymore. Disabling it for now.

2
test/test-cases/regression/secmarker.json
View File

@ -110,7 +110,7 @@
},
"expected": {
"audit_log": "",
"debug_log": "Out of a SecMarker after skip 6 rules.",
"debug_log": "Rule: 6",
"error_log": ""
},
"rules": [