Treating ARGS_NAMES as an array instead of scalar

Both value and key are the same.
owasp-modsecurity · Aug 22, 2017 · 1d3c4c6 · 1d3c4c6
1 parent 81879cd
commit 1d3c4c6
Show file tree

Hide file tree

Showing 10 changed files with 3,419 additions and 3,347 deletions.
diff --git a/headers/modsecurity/transaction.h b/headers/modsecurity/transaction.h
@@ -172,7 +172,6 @@ class TransactionAnchoredVariables {
         m_variableOffset(0)
         { }
 
-    AnchoredVariable m_variableArgsNames;
     AnchoredVariable m_variableArgGetNames;
     AnchoredVariable m_variableArgPostNames;
     AnchoredVariable m_variableRequestHeadersNames;
@@ -227,6 +226,7 @@ class TransactionAnchoredVariables {
     AnchoredVariable m_variableUrlEncodedError;
     AnchoredVariable m_variableUserID;
 
+    AnchoredSetVariable m_variableArgsNames;
     AnchoredSetVariable m_variableArgs;
     AnchoredSetVariable m_variableArgsGet;
     AnchoredSetVariable m_variableArgsPost;

diff --git a/src/macro_expansion.cc b/src/macro_expansion.cc
@@ -73,10 +73,7 @@ std::string MacroExpansion::expand(const std::string& input,
             collection = variable.find(":");
         }
         if (collection == std::string::npos) {
-            if (compareStrNoCase(variable, "ARGS_NAMES")) {
-                variableValue = transaction->m_variableArgsNames.resolveFirst();
-            }
-            else if (compareStrNoCase(variable, "ARGS_GET_NAMES")) {
+            if (compareStrNoCase(variable, "ARGS_GET_NAMES")) {
                 variableValue = transaction->m_variableArgGetNames.resolveFirst();
             }
             else if (compareStrNoCase(variable, "ARGS_POST_NAMES")) {
@@ -245,6 +242,9 @@ std::string MacroExpansion::expand(const std::string& input,
             if (compareStrNoCase(col, "ARGS")) {
                 variableValue = transaction->m_variableArgs.resolveFirst(var);
             }
+            if (compareStrNoCase(variable, "ARGS_NAMES")) {
+                variableValue = transaction->m_variableArgsNames.resolveFirst(var);
+            }
             else if (compareStrNoCase(col, "RULE")) {
                 variableValue = transaction->m_variableRule.resolveFirst(var);
             }

diff --git a/src/parser/seclang-parser.cc b/src/parser/seclang-parser.cc
diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy
@@ -1514,6 +1514,18 @@ var:
       {
         VARIABLE_CONTAINER($$, new Variables::Args_NoDictElement());
       }
+    | VARIABLE_ARGS_NAMES DICT_ELEMENT
+      {
+        VARIABLE_CONTAINER($$, new Variables::ArgsNames_DictElement($2));
+      }
+    | VARIABLE_ARGS_NAMES DICT_ELEMENT_REGEXP
+      {
+        VARIABLE_CONTAINER($$, new Variables::ArgsNames_DictElementRegexp($2));
+      }
+    | VARIABLE_ARGS_NAMES
+      {
+        VARIABLE_CONTAINER($$, new Variables::ArgsNames_NoDictElement());
+      }
     | VARIABLE_ARGS_POST DICT_ELEMENT
       {
         VARIABLE_CONTAINER($$, new Variables::ArgsPost_DictElement($2));
@@ -1794,13 +1806,6 @@ var:
       {
         VARIABLE_CONTAINER($$, new Variables::Session_NoDictElement());
       }
-
-
-
-    | VARIABLE_ARGS_NAMES
-      {
-        VARIABLE_CONTAINER($$, new Variables::ArgsNames());
-      }
     | VARIABLE_ARGS_GET_NAMES
       {
         VARIABLE_CONTAINER($$, new Variables::ArgsGetNames());

diff --git a/src/parser/seclang-scanner.cc b/src/parser/seclang-scanner.cc
diff --git a/src/parser/seclang-scanner.ll b/src/parser/seclang-scanner.ll
@@ -755,6 +755,8 @@ EQUALS_MINUS                            (?i:=\-)
 {VARIABLE_USER_ID}                          { return p::make_VARIABLE_USER_ID(*driver.loc.back()); }
 {VARIABLE_ARGS}                             { return p::make_VARIABLE_ARGS(*driver.loc.back()); }
 {VARIABLE_ARGS}[:]                          { BEGIN(EXPECTING_VAR_PARAMETER); return p::make_VARIABLE_ARGS(*driver.loc.back()); }
+{VARIABLE_ARGS_NAMES}                       { return p::make_VARIABLE_ARGS(*driver.loc.back()); }
+{VARIABLE_ARGS_NAMES}[:]                    { BEGIN(EXPECTING_VAR_PARAMETER); return p::make_VARIABLE_ARGS(*driver.loc.back()); }
 {VARIABLE_ARGS_GET}                         { return p::make_VARIABLE_ARGS_GET(*driver.loc.back()); }
 {VARIABLE_ARGS_GET}[:]                      { BEGIN(EXPECTING_VAR_PARAMETER); return p::make_VARIABLE_ARGS_GET(*driver.loc.back()); }
 {VARIABLE_ARGS_POST}                        { return p::make_VARIABLE_ARGS_POST(*driver.loc.back()); }

diff --git a/src/transaction.cc b/src/transaction.cc
@@ -309,7 +309,7 @@ bool Transaction::addArgument(const std::string& orig, const std::string& key,
         m_variableArgsPost.set(key, value, offset);
         m_variableArgPostNames.append(key, offset - key.size() - 1, true);
     }
-    m_variableArgsNames.append(key, offset - key.size() - 1, true);
+    m_variableArgsNames.set(key, key, offset - key.size() - 1);
 
     m_ARGScombinedSizeDouble = m_ARGScombinedSizeDouble + \
         key.length() + value.length();

diff --git a/src/variables/args_names.h b/src/variables/args_names.h
@@ -29,18 +29,51 @@ namespace modsecurity {
 class Transaction;
 namespace Variables {
 
-class ArgsNames : public Variable {
+class ArgsNames_DictElement : public Variable {
  public:
-    ArgsNames()
+    explicit ArgsNames_DictElement(std::string dictElement)
+        : Variable("ARGS_NAMES" + std::string(":") + std::string(dictElement)),
+        m_dictElement(dictElement) { }
+
+    void evaluate(Transaction *transaction,
+        Rule *rule,
+        std::vector<const collection::Variable *> *l) override {
+        transaction->m_variableArgsNames.resolve(m_dictElement, l);
+    }
+
+    std::string m_dictElement;
+};
+
+
+class ArgsNames_NoDictElement : public Variable {
+ public:
+    ArgsNames_NoDictElement()
         : Variable("ARGS_NAMES") { }
 
     void evaluate(Transaction *transaction,
         Rule *rule,
-        std::vector<const collection::Variable *> *l) {
-        transaction->m_variableArgsNames.evaluate(l);
+        std::vector<const collection::Variable *> *l) override {
+        transaction->m_variableArgsNames.resolve(l);
     }
 };
 
+
+class ArgsNames_DictElementRegexp : public Variable {
+ public:
+    explicit ArgsNames_DictElementRegexp(std::string dictElement)
+        : Variable("ARGS_NAMES:regex(" + dictElement + ")"),
+        m_r(dictElement) {
+    }
+
+    void evaluate(Transaction *transaction,
+        Rule *rule,
+        std::vector<const collection::Variable *> *l) override {
+        transaction->m_variableArgsNames.resolveRegularExpression(&m_r, l);
+    }
+
+    Utils::Regex m_r;
+};
+
 }  // namespace Variables
 }  // namespace modsecurity
 

diff --git a/test/test-cases/regression/offset-variable.json b/test/test-cases/regression/offset-variable.json
@@ -314,11 +314,11 @@
       ]
     },
     "expected":{
-      "error_log":"o0,17v17,6v31,6v45,6v149,6v163,6v177,6t:trim"
+      "error_log":"o0,3v17,6t:trimo0,3v149,6t:trimo0,3v31,6t:trimo0,3v163,6t:trimo0,3v45,6t:trimo0,3v177,6t:trim"
     },
     "rules":[
       "SecRequestBodyAccess On",
-      "SecRule ARGS_NAMES \"@rx param1 param2 par\" \"id:1,phase:2,pass,t:trim,msg:'ops'\""
+      "SecRule ARGS_NAMES \"@rx par\" \"id:1,phase:2,pass,t:trim,msg:'ops'\""
     ]
   },
     {

diff --git a/test/test-cases/regression/variable-ARGS_NAMES.json b/test/test-cases/regression/variable-ARGS_NAMES.json
@@ -31,7 +31,7 @@
       ]
     },
     "expected":{  
-      "debug_log":"Target value: \"key key\""
+      "debug_log":"Target value: \"key\""
     },
     "rules":[  
       "SecRuleEngine On",
@@ -70,7 +70,7 @@
       ]
     },
     "expected":{  
-      "debug_log":"Target value: \"key key\""
+      "debug_log":"Target value: \"key\""
     },
     "rules":[  
       "SecRuleEngine On",
@@ -114,7 +114,7 @@
       ]
     },
     "expected":{
-      "debug_log":"Target value: \"param1 param2\""
+      "debug_log":"Target value: \"param1\""
     },
     "rules":[
       "SecRuleEngine On",
@@ -158,7 +158,7 @@
       ]
     },
     "expected":{
-      "debug_log":"Target value: \"param1 param2\""
+      "debug_log":"Target value: \"param1\" "
     },
     "rules":[
       "SecRuleEngine On",