From 889186dd58f6329cacdb2adf8392718281aba92d Mon Sep 17 00:00:00 2001 From: Steven Date: Sat, 15 Sep 2018 16:32:20 -0400 Subject: [PATCH] Fix RulesProperties::appendRules() RulesProperties::appendRules() was not checking for duplicate IDs as well as throwing an error if there were secMarkers in more than one file (when calling any combination of rules->load(), rules->loadFromUri() or rules->loadRemote() more than once). To fix the secMarker issue, the if statement on rules_properties.h:441 just needed to be negated. This function also doesn't accurately check for duplicate IDs. the check can be circumvented by putting the rule in a different phase. To fix this the ruleId list (v) had to be populated completely before checking against the other list. --- headers/modsecurity/rules_properties.h | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/headers/modsecurity/rules_properties.h b/headers/modsecurity/rules_properties.h index c19d2837d7..492609e6c8 100644 --- a/headers/modsecurity/rules_properties.h +++ b/headers/modsecurity/rules_properties.h @@ -430,34 +430,36 @@ class RulesProperties { std::vector *to, std::ostringstream *err) { int amount_of_rules = 0; + // TODO: std::vector could be replaced with something more efficient. + std::vector v; for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { std::vector *rules_to = to+i; - std::vector *rules_from = from+i; - // TODO: std::vector could be replaced with something more efficient. - std::vector v; v.reserve(rules_to->size()); for (size_t z = 0; z < rules_to->size(); z++) { Rule *rule_ckc = rules_to->at(z); - if (rule_ckc->m_secMarker == false) { + if (rule_ckc->m_secMarker == true) { continue; } v.push_back(rule_ckc->m_ruleId); } - std::sort (v.begin(), v.end()); + } + std::sort (v.begin(), v.end()); + for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { + std::vector *rules_from = from+i; + std::vector *rules_to = to+i; for (size_t j = 0; j < rules_from->size(); j++) { Rule *rule = rules_from->at(j); if (std::binary_search (v.begin(), v.end(), rule->m_ruleId)) { if (err != NULL) { - *err << "Rule id: " \ - << std::to_string(rule->m_ruleId) \ + *err << "Rule id: " << std::to_string(rule->m_ruleId) \ << " is duplicated" << std::endl; } return -1; } amount_of_rules++; - rules_to->push_back(rule); rule->refCountIncrease(); + rules_to->push_back(rule); } } return amount_of_rules;