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.

Author: stevendore

headers/modsecurity/rules_properties.h

@@ -431,34 +431,36 @@ class RulesProperties {
         std::vector<modsecurity::Rule *> *to,
         std::ostringstream *err) {
         int amount_of_rules = 0;
+        // TODO: std::vector could be replaced with something more efficient.
+        std::vector<int64_t> v;
         for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
             std::vector<modsecurity::Rule *> *rules_to = to+i;
-            std::vector<modsecurity::Rule *> *rules_from = from+i;
-            // FIXME: std::vector could be replaced with something more efficient.
-            std::vector<int64_t> 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<modsecurity::Rule *> *rules_from = from+i;
+            std::vector<modsecurity::Rule *> *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;