Using shared_ptr instead of unique_ptr on rules exceptions

owasp-modsecurity · Oct 23, 2018 · fa5f378 · fa5f378
1 parent e63344c
commit fa5f378
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 17 deletions.
diff --git a/CHANGES b/CHANGES
@@ -1,6 +1,8 @@
 v3.0.3 - YYYY-MMM-DD (to be released)
 -------------------------------------
 
+ - Using shared_ptr instead of unique_ptr on rules exceptions
+   [Issue #1697 - @zimmerle, @brianp9906, @victorhora, @LeSwiss, @defanator]
  - Changes debuglogs schema to avoid unecessary str allocation
    [0xb2840 - @zimmerle]
  - Fix the SecUnicodeMapFile and SecUnicodeCodePage

diff --git a/headers/modsecurity/rules_exceptions.h b/headers/modsecurity/rules_exceptions.h
@@ -73,15 +73,15 @@ class RulesExceptions {
         std::string *error);
 
     std::unordered_multimap<std::shared_ptr<std::string>,
-        std::unique_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::unique_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::unique_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::unique_ptr<actions::Action>> m_action_pre_update_target_by_id;
+        std::shared_ptr<actions::Action>> m_action_pre_update_target_by_id;
     std::unordered_multimap<double,
-        std::unique_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_tag;
 

diff --git a/src/rule.cc b/src/rule.cc
@@ -683,6 +683,9 @@ bool Rule::evaluate(Transaction *trans,
 
     for (auto &var : vars) {
         std::vector<const VariableValue *> e;
+        if (!var) {
+            continue;
+        }
         var->evaluate(trans, this, &e);
         for (const VariableValue *v : e) {
             const std::string &value = v->m_value;

diff --git a/src/rules_exceptions.cc b/src/rules_exceptions.cc
@@ -96,7 +96,8 @@ bool RulesExceptions::loadUpdateTargetByTag(const std::string &tag,
         m_variable_update_target_by_tag.emplace(
             std::pair<std::shared_ptr<std::string>,
                 std::unique_ptr<Variables::Variable>>(
-                    std::make_shared<std::string>(tag), std::move(i)));
+                    std::make_shared<std::string>(tag),
+                    std::move(i)));
     }
 
     return true;
@@ -110,7 +111,8 @@ bool RulesExceptions::loadUpdateTargetById(double id,
     for (auto &i : *var) {
         m_variable_update_target_by_id.emplace(
             std::pair<double,
-                std::unique_ptr<Variables::Variable>>(id , std::move(i)));
+                std::unique_ptr<Variables::Variable>>(id,
+                std::move(i)));
     }
 
     return true;
@@ -220,36 +222,36 @@ bool RulesExceptions::merge(RulesExceptions *from) {
     for (auto &p : from->m_variable_update_target_by_tag) {
         m_variable_update_target_by_tag.emplace(
             std::pair<std::shared_ptr<std::string>,
-            std::unique_ptr<Variables::Variable>>(p.first,
-                std::move(p.second)));
+            std::shared_ptr<Variables::Variable>>(p.first,
+                p.second));
     }
 
     for (auto &p : from->m_variable_update_target_by_msg) {
         m_variable_update_target_by_msg.emplace(
             std::pair<std::shared_ptr<std::string>,
-            std::unique_ptr<Variables::Variable>>(p.first,
-                std::move(p.second)));
+            std::shared_ptr<Variables::Variable>>(p.first,
+                p.second));
     }
 
     for (auto &p : from->m_variable_update_target_by_id) {
         m_variable_update_target_by_id.emplace(
             std::pair<double,
-                std::unique_ptr<Variables::Variable>>(p.first,
-                    std::move(p.second)));
+                std::shared_ptr<Variables::Variable>>(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<double,
-                std::unique_ptr<actions::Action>>(p.first,
-                    std::move(p.second)));
+                std::shared_ptr<actions::Action>>(p.first,
+                    p.second));
     }
 
     for (auto &p : from->m_action_pre_update_target_by_id) {
         m_action_pre_update_target_by_id.emplace(
             std::pair<double,
-                std::unique_ptr<actions::Action>>(p.first,
-                    std::move(p.second)));
+                std::shared_ptr<actions::Action>>(p.first,
+                    p.second));
     }
 
     for (auto &p : from->m_remove_rule_by_msg) {