Make all "rule id" variables of type RuleId

Previously, ModSecurity inconsistently used RuleId, int and double for
rule id variables in different places.
This commit is contained in:
WGH 2020-07-24 20:12:35 +03:00 committed by Felipe Zimmerle
parent bf98e3424f
commit 592c8f0b19
No known key found for this signature in database
GPG Key ID: E6DFB08CE8B11277
6 changed files with 62 additions and 61 deletions

View File

@ -28,6 +28,8 @@
#include <memory> #include <memory>
#endif #endif
#include "modsecurity/modsecurity.h"
#ifndef HEADERS_MODSECURITY_RULES_EXCEPTIONS_H_ #ifndef HEADERS_MODSECURITY_RULES_EXCEPTIONS_H_
#define HEADERS_MODSECURITY_RULES_EXCEPTIONS_H_ #define HEADERS_MODSECURITY_RULES_EXCEPTIONS_H_
@ -51,9 +53,9 @@ class RulesExceptions {
~RulesExceptions(); ~RulesExceptions();
bool load(const std::string &data, std::string *error); bool load(const std::string &data, std::string *error);
bool addRange(int a, int b); bool addRange(RuleId a, RuleId b);
bool addNumber(int a); bool addNumber(RuleId a);
bool contains(int a); bool contains(RuleId a);
bool merge(RulesExceptions *from); bool merge(RulesExceptions *from);
bool loadRemoveRuleByMsg(const std::string &msg, std::string *error); bool loadRemoveRuleByMsg(const std::string &msg, std::string *error);
@ -67,11 +69,11 @@ class RulesExceptions {
std::unique_ptr<std::vector<std::unique_ptr<variables::Variable> > > v, std::unique_ptr<std::vector<std::unique_ptr<variables::Variable> > > v,
std::string *error); std::string *error);
bool loadUpdateTargetById(double id, bool loadUpdateTargetById(RuleId id,
std::unique_ptr<std::vector<std::unique_ptr<variables::Variable> > > v, std::unique_ptr<std::vector<std::unique_ptr<variables::Variable> > > v,
std::string *error); std::string *error);
bool loadUpdateActionById(double id, bool loadUpdateActionById(RuleId id,
std::unique_ptr<std::vector<std::unique_ptr<actions::Action> > > actions, std::unique_ptr<std::vector<std::unique_ptr<actions::Action> > > actions,
std::string *error); std::string *error);
@ -79,18 +81,18 @@ class RulesExceptions {
std::shared_ptr<variables::Variable>> m_variable_update_target_by_tag; std::shared_ptr<variables::Variable>> m_variable_update_target_by_tag;
std::unordered_multimap<std::shared_ptr<std::string>, std::unordered_multimap<std::shared_ptr<std::string>,
std::shared_ptr<variables::Variable>> m_variable_update_target_by_msg; std::shared_ptr<variables::Variable>> m_variable_update_target_by_msg;
std::unordered_multimap<double, std::unordered_multimap<RuleId,
std::shared_ptr<variables::Variable>> m_variable_update_target_by_id; std::shared_ptr<variables::Variable>> m_variable_update_target_by_id;
std::unordered_multimap<double, std::unordered_multimap<RuleId,
std::shared_ptr<actions::transformations::Transformation>> m_action_transformation_update_target_by_id; std::shared_ptr<actions::transformations::Transformation>> m_action_transformation_update_target_by_id;
std::unordered_multimap<double, std::unordered_multimap<RuleId,
std::shared_ptr<actions::Action>> m_action_pos_update_target_by_id; std::shared_ptr<actions::Action>> m_action_pos_update_target_by_id;
std::list<std::string> m_remove_rule_by_msg; std::list<std::string> m_remove_rule_by_msg;
std::list<std::string> m_remove_rule_by_tag; std::list<std::string> m_remove_rule_by_tag;
private: private:
std::list<std::pair<int, int> > m_ranges; std::list<std::pair<RuleId, RuleId> > m_ranges;
std::list<int> m_numbers; std::list<RuleId> m_numbers;
}; };
} // namespace modsecurity } // namespace modsecurity

View File

@ -26,18 +26,16 @@ namespace actions {
bool RuleId::init(std::string *error) { bool RuleId::init(std::string *error) {
std::string a = m_parserPayload; std::string a = m_parserPayload;
try { std::istringstream iss(a);
m_ruleId = std::stod(a); iss >> m_ruleId;
} catch (...) { if (iss.fail()) {
m_ruleId = 0; m_ruleId = 0;
error->assign("The input \"" + a + "\" does not " \ error->assign("The input \"" + a + "\" does not " \
"seems to be a valid rule id."); "seems to be a valid rule id.");
return false; return false;
} }
std::ostringstream oss; if (a != std::to_string(m_ruleId) || m_ruleId < 0) {
oss << std::setprecision(40) << m_ruleId;
if (a != oss.str() || m_ruleId < 0) {
error->assign("The input \"" + a + "\" does not seems " \ error->assign("The input \"" + a + "\" does not seems " \
"to be a valid rule id."); "to be a valid rule id.");
return false; return false;

View File

@ -41,7 +41,7 @@ class RuleId : public ActionTypeRuleMetaData {
} }
private: private:
double m_ruleId; modsecurity::RuleId m_ruleId;
}; };

View File

@ -2880,10 +2880,10 @@ namespace yy {
#line 1480 "seclang-parser.yy" #line 1480 "seclang-parser.yy"
{ {
std::string error; std::string error;
double ruleId; std::istringstream iss(yystack_[1].value.as < std::string > ());
try { RuleId ruleId;
ruleId = std::stod(yystack_[1].value.as < std::string > ()); iss >> ruleId;
} catch (...) { if (iss.fail()) {
std::stringstream ss; std::stringstream ss;
ss << "SecRuleUpdateTargetById: failed to load:"; ss << "SecRuleUpdateTargetById: failed to load:";
ss << "The input \"" + yystack_[1].value.as < std::string > () + "\" does not "; ss << "The input \"" + yystack_[1].value.as < std::string > () + "\" does not ";
@ -2910,10 +2910,10 @@ namespace yy {
#line 1506 "seclang-parser.yy" #line 1506 "seclang-parser.yy"
{ {
std::string error; std::string error;
double ruleId; std::istringstream iss(yystack_[1].value.as < std::string > ());
try { RuleId ruleId;
ruleId = std::stod(yystack_[1].value.as < std::string > ()); iss >> ruleId;
} catch (...) { if (iss.fail()) {
std::stringstream ss; std::stringstream ss;
ss << "SecRuleUpdateActionById: failed to load:"; ss << "SecRuleUpdateActionById: failed to load:";
ss << "The input \"" + yystack_[1].value.as < std::string > () + "\" does not "; ss << "The input \"" + yystack_[1].value.as < std::string > () + "\" does not ";

View File

@ -1479,10 +1479,10 @@ expression:
| CONFIG_SEC_RULE_UPDATE_TARGET_BY_ID variables_pre_process | CONFIG_SEC_RULE_UPDATE_TARGET_BY_ID variables_pre_process
{ {
std::string error; std::string error;
double ruleId; std::istringstream iss($1);
try { RuleId ruleId;
ruleId = std::stod($1); iss >> ruleId;
} catch (...) { if (iss.fail()) {
std::stringstream ss; std::stringstream ss;
ss << "SecRuleUpdateTargetById: failed to load:"; ss << "SecRuleUpdateTargetById: failed to load:";
ss << "The input \"" + $1 + "\" does not "; ss << "The input \"" + $1 + "\" does not ";
@ -1505,10 +1505,10 @@ expression:
| CONFIG_SEC_RULE_UPDATE_ACTION_BY_ID actions | CONFIG_SEC_RULE_UPDATE_ACTION_BY_ID actions
{ {
std::string error; std::string error;
double ruleId; std::istringstream iss($1);
try { RuleId ruleId;
ruleId = std::stod($1); iss >> ruleId;
} catch (...) { if (iss.fail()) {
std::stringstream ss; std::stringstream ss;
ss << "SecRuleUpdateActionById: failed to load:"; ss << "SecRuleUpdateActionById: failed to load:";
ss << "The input \"" + $1 + "\" does not "; ss << "The input \"" + $1 + "\" does not ";

View File

@ -33,7 +33,7 @@ RulesExceptions::~RulesExceptions() {
} }
bool RulesExceptions::loadUpdateActionById(double id, bool RulesExceptions::loadUpdateActionById(RuleId id,
std::unique_ptr<std::vector<std::unique_ptr<actions::Action> > > actions, std::unique_ptr<std::vector<std::unique_ptr<actions::Action> > > actions,
std::string *error) { std::string *error) {
@ -48,14 +48,15 @@ bool RulesExceptions::loadUpdateActionById(double id,
if (dynamic_cast<actions::transformations::Transformation *>(a.get())) { if (dynamic_cast<actions::transformations::Transformation *>(a.get())) {
actions::transformations::Transformation *t = dynamic_cast<actions::transformations::Transformation *>(a.release()); actions::transformations::Transformation *t = dynamic_cast<actions::transformations::Transformation *>(a.release());
m_action_transformation_update_target_by_id.emplace( m_action_transformation_update_target_by_id.emplace(
std::pair<double, std::pair<RuleId,
std::shared_ptr<actions::transformations::Transformation>>(id, std::shared_ptr<actions::transformations::Transformation>(t)) std::shared_ptr<actions::transformations::Transformation>>(id, std::shared_ptr<actions::transformations::Transformation>(t))
); );
continue; continue;
} }
m_action_pos_update_target_by_id.emplace( m_action_pos_update_target_by_id.emplace(
std::pair<double, std::pair<RuleId,
std::unique_ptr<actions::Action>>(id , std::move(a)) std::unique_ptr<actions::Action>>(id , std::move(a))
); );
} }
@ -111,13 +112,13 @@ bool RulesExceptions::loadUpdateTargetByTag(const std::string &tag,
} }
bool RulesExceptions::loadUpdateTargetById(double id, bool RulesExceptions::loadUpdateTargetById(RuleId id,
std::unique_ptr<std::vector<std::unique_ptr<variables::Variable> > > var, std::unique_ptr<std::vector<std::unique_ptr<variables::Variable> > > var,
std::string *error) { std::string *error) {
for (auto &i : *var) { for (auto &i : *var) {
m_variable_update_target_by_id.emplace( m_variable_update_target_by_id.emplace(
std::pair<double, std::pair<RuleId,
std::unique_ptr<variables::Variable>>(id, std::unique_ptr<variables::Variable>>(id,
std::move(i))); std::move(i)));
} }
@ -139,19 +140,18 @@ bool RulesExceptions::load(const std::string &a, std::string *error) {
if (dash != std::string::npos) { if (dash != std::string::npos) {
std::string n1s = std::string(b, 0, dash); std::string n1s = std::string(b, 0, dash);
std::string n2s = std::string(b, dash + 1, b.size() - (dash + 1)); std::string n2s = std::string(b, dash + 1, b.size() - (dash + 1));
int n1n = 0; std::istringstream n1ss(n1s), n2ss(n2s);
int n2n = 0; RuleId n1n = 0;
try { RuleId n2n = 0;
n1n = std::stoi(n1s);
added = true; n1ss >> n1n;
} catch (...) { if (n1ss.fail()) {
error->assign("Not a number: " + n1s); error->assign("Not a number: " + n1s);
return false; return false;
} }
try {
n2n = std::stoi(n2s); n2ss >> n2n;
added = true; if (n2ss.fail()) {
} catch (...) {
error->assign("Not a number: " + n2s); error->assign("Not a number: " + n2s);
return false; return false;
} }
@ -163,14 +163,15 @@ bool RulesExceptions::load(const std::string &a, std::string *error) {
addRange(n1n, n2n); addRange(n1n, n2n);
added = true; added = true;
} else { } else {
try { std::istringstream iss(b);
int num = std::stoi(b); RuleId num;
addNumber(num); iss >> num;
added = true; if (iss.fail()) {
} catch (...) {
error->assign("Not a number or range: " + b); error->assign("Not a number or range: " + b);
return false; return false;
} }
addNumber(num);
added = true;
} }
} }
@ -183,20 +184,20 @@ bool RulesExceptions::load(const std::string &a, std::string *error) {
} }
bool RulesExceptions::addNumber(int a) { bool RulesExceptions::addNumber(RuleId a) {
m_numbers.push_back(a); m_numbers.push_back(a);
return true; return true;
} }
bool RulesExceptions::addRange(int a, int b) { bool RulesExceptions::addRange(RuleId a, RuleId b) {
m_ranges.push_back(std::make_pair(a, b)); m_ranges.push_back(std::make_pair(a, b));
return true; return true;
} }
bool RulesExceptions::contains(int a) { bool RulesExceptions::contains(RuleId a) {
for (int z : m_numbers) { for (RuleId z : m_numbers) {
if (a == z) { if (a == z) {
return true; return true;
} }
@ -213,7 +214,7 @@ bool RulesExceptions::contains(int a) {
bool RulesExceptions::merge(RulesExceptions *from) { bool RulesExceptions::merge(RulesExceptions *from) {
for (int a : from->m_numbers) { for (RuleId a : from->m_numbers) {
bool ret = addNumber(a); bool ret = addNumber(a);
if (ret == false) { if (ret == false) {
return ret; return ret;
@ -242,21 +243,21 @@ bool RulesExceptions::merge(RulesExceptions *from) {
for (auto &p : from->m_variable_update_target_by_id) { for (auto &p : from->m_variable_update_target_by_id) {
m_variable_update_target_by_id.emplace( m_variable_update_target_by_id.emplace(
std::pair<double, std::pair<RuleId,
std::shared_ptr<variables::Variable>>(p.first, std::shared_ptr<variables::Variable>>(p.first,
p.second)); p.second));
} }
for (auto &p : from->m_action_pos_update_target_by_id) { for (auto &p : from->m_action_pos_update_target_by_id) {
m_action_pos_update_target_by_id.emplace( m_action_pos_update_target_by_id.emplace(
std::pair<double, std::pair<RuleId,
std::shared_ptr<actions::Action>>(p.first, std::shared_ptr<actions::Action>>(p.first,
p.second)); p.second));
} }
for (auto &p : from->m_action_transformation_update_target_by_id) { for (auto &p : from->m_action_transformation_update_target_by_id) {
m_action_transformation_update_target_by_id.emplace( m_action_transformation_update_target_by_id.emplace(
std::pair<double, std::pair<RuleId,
std::shared_ptr<actions::transformations::Transformation>>(p.first, std::shared_ptr<actions::transformations::Transformation>>(p.first,
p.second)); p.second));
} }