From d7e9e0aa5b894d41181806d33a9186de6baf5cfa Mon Sep 17 00:00:00 2001 From: WGH Date: Fri, 24 Jul 2020 20:12:35 +0300 Subject: [PATCH] Make all "rule id" variables of type RuleId Previously, ModSecurity inconsistently used RuleId, int and double for rule id variables in different places. --- headers/modsecurity/rules_exceptions.h | 22 +++++----- src/actions/rule_id.cc | 10 ++--- src/actions/rule_id.h | 2 +- src/parser/seclang-parser.cc | 16 ++++---- src/parser/seclang-parser.yy | 16 ++++---- src/rules_exceptions.cc | 57 +++++++++++++------------- 6 files changed, 62 insertions(+), 61 deletions(-) diff --git a/headers/modsecurity/rules_exceptions.h b/headers/modsecurity/rules_exceptions.h index ec3a552d..8b24343e 100644 --- a/headers/modsecurity/rules_exceptions.h +++ b/headers/modsecurity/rules_exceptions.h @@ -28,6 +28,8 @@ #include #endif +#include "modsecurity/modsecurity.h" + #ifndef HEADERS_MODSECURITY_RULES_EXCEPTIONS_H_ #define HEADERS_MODSECURITY_RULES_EXCEPTIONS_H_ @@ -51,9 +53,9 @@ class RulesExceptions { ~RulesExceptions(); bool load(const std::string &data, std::string *error); - bool addRange(int a, int b); - bool addNumber(int a); - bool contains(int a); + bool addRange(RuleId a, RuleId b); + bool addNumber(RuleId a); + bool contains(RuleId a); bool merge(RulesExceptions *from); bool loadRemoveRuleByMsg(const std::string &msg, std::string *error); @@ -67,11 +69,11 @@ class RulesExceptions { std::unique_ptr > > v, std::string *error); - bool loadUpdateTargetById(double id, + bool loadUpdateTargetById(RuleId id, std::unique_ptr > > v, std::string *error); - bool loadUpdateActionById(double id, + bool loadUpdateActionById(RuleId id, std::unique_ptr > > actions, std::string *error); @@ -79,18 +81,18 @@ class RulesExceptions { std::shared_ptr> m_variable_update_target_by_tag; std::unordered_multimap, std::shared_ptr> m_variable_update_target_by_msg; - std::unordered_multimap> m_variable_update_target_by_id; - std::unordered_multimap> m_action_transformation_update_target_by_id; - std::unordered_multimap> m_action_pos_update_target_by_id; std::list m_remove_rule_by_msg; std::list m_remove_rule_by_tag; private: - std::list > m_ranges; - std::list m_numbers; + std::list > m_ranges; + std::list m_numbers; }; } // namespace modsecurity diff --git a/src/actions/rule_id.cc b/src/actions/rule_id.cc index 7cce70e4..c5d5f9ae 100644 --- a/src/actions/rule_id.cc +++ b/src/actions/rule_id.cc @@ -26,18 +26,16 @@ namespace actions { bool RuleId::init(std::string *error) { std::string a = m_parserPayload; - try { - m_ruleId = std::stod(a); - } catch (...) { + std::istringstream iss(a); + iss >> m_ruleId; + if (iss.fail()) { m_ruleId = 0; error->assign("The input \"" + a + "\" does not " \ "seems to be a valid rule id."); return false; } - std::ostringstream oss; - oss << std::setprecision(40) << m_ruleId; - if (a != oss.str() || m_ruleId < 0) { + if (a != std::to_string(m_ruleId) || m_ruleId < 0) { error->assign("The input \"" + a + "\" does not seems " \ "to be a valid rule id."); return false; diff --git a/src/actions/rule_id.h b/src/actions/rule_id.h index 8813c7c5..de7265a3 100644 --- a/src/actions/rule_id.h +++ b/src/actions/rule_id.h @@ -41,7 +41,7 @@ class RuleId : public ActionTypeRuleMetaData { } private: - double m_ruleId; + modsecurity::RuleId m_ruleId; }; diff --git a/src/parser/seclang-parser.cc b/src/parser/seclang-parser.cc index bdc59656..ef17cbb9 100644 --- a/src/parser/seclang-parser.cc +++ b/src/parser/seclang-parser.cc @@ -2880,10 +2880,10 @@ namespace yy { #line 1480 "seclang-parser.yy" { std::string error; - double ruleId; - try { - ruleId = std::stod(yystack_[1].value.as < std::string > ()); - } catch (...) { + std::istringstream iss(yystack_[1].value.as < std::string > ()); + RuleId ruleId; + iss >> ruleId; + if (iss.fail()) { std::stringstream ss; ss << "SecRuleUpdateTargetById: failed to load:"; ss << "The input \"" + yystack_[1].value.as < std::string > () + "\" does not "; @@ -2910,10 +2910,10 @@ namespace yy { #line 1506 "seclang-parser.yy" { std::string error; - double ruleId; - try { - ruleId = std::stod(yystack_[1].value.as < std::string > ()); - } catch (...) { + std::istringstream iss(yystack_[1].value.as < std::string > ()); + RuleId ruleId; + iss >> ruleId; + if (iss.fail()) { std::stringstream ss; ss << "SecRuleUpdateActionById: failed to load:"; ss << "The input \"" + yystack_[1].value.as < std::string > () + "\" does not "; diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy index dbbcf45e..72883984 100644 --- a/src/parser/seclang-parser.yy +++ b/src/parser/seclang-parser.yy @@ -1479,10 +1479,10 @@ expression: | CONFIG_SEC_RULE_UPDATE_TARGET_BY_ID variables_pre_process { std::string error; - double ruleId; - try { - ruleId = std::stod($1); - } catch (...) { + std::istringstream iss($1); + RuleId ruleId; + iss >> ruleId; + if (iss.fail()) { std::stringstream ss; ss << "SecRuleUpdateTargetById: failed to load:"; ss << "The input \"" + $1 + "\" does not "; @@ -1505,10 +1505,10 @@ expression: | CONFIG_SEC_RULE_UPDATE_ACTION_BY_ID actions { std::string error; - double ruleId; - try { - ruleId = std::stod($1); - } catch (...) { + std::istringstream iss($1); + RuleId ruleId; + iss >> ruleId; + if (iss.fail()) { std::stringstream ss; ss << "SecRuleUpdateActionById: failed to load:"; ss << "The input \"" + $1 + "\" does not "; diff --git a/src/rules_exceptions.cc b/src/rules_exceptions.cc index 7167aa7f..641a3501 100644 --- a/src/rules_exceptions.cc +++ b/src/rules_exceptions.cc @@ -33,7 +33,7 @@ RulesExceptions::~RulesExceptions() { } -bool RulesExceptions::loadUpdateActionById(double id, +bool RulesExceptions::loadUpdateActionById(RuleId id, std::unique_ptr > > actions, std::string *error) { @@ -48,14 +48,15 @@ bool RulesExceptions::loadUpdateActionById(double id, if (dynamic_cast(a.get())) { actions::transformations::Transformation *t = dynamic_cast(a.release()); m_action_transformation_update_target_by_id.emplace( - std::pair>(id, std::shared_ptr(t)) ); continue; + } m_action_pos_update_target_by_id.emplace( - std::pair>(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 > > var, std::string *error) { for (auto &i : *var) { m_variable_update_target_by_id.emplace( - std::pair>(id, std::move(i))); } @@ -139,19 +140,18 @@ bool RulesExceptions::load(const std::string &a, std::string *error) { if (dash != std::string::npos) { std::string n1s = std::string(b, 0, dash); std::string n2s = std::string(b, dash + 1, b.size() - (dash + 1)); - int n1n = 0; - int n2n = 0; - try { - n1n = std::stoi(n1s); - added = true; - } catch (...) { + std::istringstream n1ss(n1s), n2ss(n2s); + RuleId n1n = 0; + RuleId n2n = 0; + + n1ss >> n1n; + if (n1ss.fail()) { error->assign("Not a number: " + n1s); return false; } - try { - n2n = std::stoi(n2s); - added = true; - } catch (...) { + + n2ss >> n2n; + if (n2ss.fail()) { error->assign("Not a number: " + n2s); return false; } @@ -163,14 +163,15 @@ bool RulesExceptions::load(const std::string &a, std::string *error) { addRange(n1n, n2n); added = true; } else { - try { - int num = std::stoi(b); - addNumber(num); - added = true; - } catch (...) { + std::istringstream iss(b); + RuleId num; + iss >> num; + if (iss.fail()) { error->assign("Not a number or range: " + b); 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); 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)); return true; } -bool RulesExceptions::contains(int a) { - for (int z : m_numbers) { +bool RulesExceptions::contains(RuleId a) { + for (RuleId z : m_numbers) { if (a == z) { return true; } @@ -213,7 +214,7 @@ bool RulesExceptions::contains(int a) { bool RulesExceptions::merge(RulesExceptions *from) { - for (int a : from->m_numbers) { + for (RuleId a : from->m_numbers) { bool ret = addNumber(a); if (ret == false) { return ret; @@ -242,21 +243,21 @@ bool RulesExceptions::merge(RulesExceptions *from) { for (auto &p : from->m_variable_update_target_by_id) { m_variable_update_target_by_id.emplace( - std::pair>(p.first, p.second)); } for (auto &p : from->m_action_pos_update_target_by_id) { m_action_pos_update_target_by_id.emplace( - std::pair>(p.first, p.second)); } for (auto &p : from->m_action_transformation_update_target_by_id) { m_action_transformation_update_target_by_id.emplace( - std::pair>(p.first, p.second)); }