Support configurable limit on number of arguments processed

CHANGES

@@ -1,6 +1,8 @@
 v3.0.4 - YYYY-MMM-DD (to be released)
 -------------------------------------
 
+ - Support configurable limit on number of arguments processed
+   [@jleproust, @martinhsv]
  - Fix: audit log data omitted when nolog,auditlog
    [@martinhsv]
  - Fix: ModSecurity 3.x inspectFile operator does not pass

headers/modsecurity/rules_properties.h

@@ -382,6 +382,7 @@ class RulesProperties {
                             from->m_tmpSaveUploadedFiles,
                             PropertyNotSetConfigBoolean);
 
+        to->m_argumentsLimit.merge(&from->m_argumentsLimit);
         to->m_requestBodyLimit.merge(&from->m_requestBodyLimit);
         to->m_responseBodyLimit.merge(&from->m_responseBodyLimit);
 
@@ -529,6 +530,7 @@ class RulesProperties {
     ConfigBoolean m_secXMLExternalEntity;
     ConfigBoolean m_tmpSaveUploadedFiles;
     ConfigBoolean m_uploadKeepFiles;
+    ConfigDouble m_argumentsLimit;
     ConfigDouble m_requestBodyLimit;
     ConfigDouble m_requestBodyNoFilesLimit;
     ConfigDouble m_responseBodyLimit;

src/parser/seclang-parser.yy

@@ -601,6 +601,7 @@ using namespace modsecurity::operators;
   CONFIG_SEC_CONN_R_STATE_LIMIT                "CONFIG_SEC_CONN_R_STATE_LIMIT"
   CONFIG_SEC_CONN_W_STATE_LIMIT                "CONFIG_SEC_CONN_W_STATE_LIMIT"
   CONFIG_SEC_SENSOR_ID                         "CONFIG_SEC_SENSOR_ID"
+  CONFIG_DIR_ARGS_LIMIT                        "CONFIG_DIR_ARGS_LIMIT"
   CONFIG_DIR_REQ_BODY                          "CONFIG_DIR_REQ_BODY"
   CONFIG_DIR_REQ_BODY_IN_MEMORY_LIMIT          "CONFIG_DIR_REQ_BODY_IN_MEMORY_LIMIT"
   CONFIG_DIR_REQ_BODY_LIMIT                    "CONFIG_DIR_REQ_BODY_LIMIT"
@@ -1553,6 +1554,11 @@ expression:
         YYERROR;
 #endif  // WITH_GEOIP
       }
+    | CONFIG_DIR_ARGS_LIMIT
+      {
+        driver.m_argumentsLimit.m_set = true;
+        driver.m_argumentsLimit.m_value = atoi($1.c_str());
+      }
     /* Body limits */
     | CONFIG_DIR_REQ_BODY_LIMIT
       {

src/parser/seclang-scanner.ll

@@ -359,6 +359,7 @@ CONFIG_SEC_STREAM_IN_BODY_INSPECTION    (?i:SecStreamInBodyInspection)
 CONFIG_SEC_STREAM_OUT_BODY_INSPECTION   (?i:SecStreamOutBodyInspection)
 CONFIG_DIR_PCRE_MATCH_LIMIT             (?i:SecPcreMatchLimit)
 CONFIG_DIR_PCRE_MATCH_LIMIT_RECURSION   (?i:SecPcreMatchLimitRecursion)
+CONFIG_DIR_ARGS_LIMIT                   (?i:SecArgumentsLimit)
 CONFIG_DIR_REQ_BODY                     (?i:SecRequestBodyAccess)
 CONFIG_DIR_REQ_BODY_IN_MEMORY_LIMIT     (?i:SecRequestBodyInMemoryLimit)
 CONFIG_DIR_REQ_BODY_LIMIT               (?i:SecRequestBodyLimit)
@@ -764,6 +765,7 @@ EQUALS_MINUS                            (?i:=\-)
 {CONFIG_DIR_GEO_DB}[ \t]+{FREE_TEXT_NEW_LINE}                              { return p::make_CONFIG_DIR_GEO_DB(strchr(yytext, ' ') + 1, *driver.loc.back()); }
 {CONFIG_DIR_PCRE_MATCH_LIMIT_RECURSION}[ \t]+{CONFIG_VALUE_NUMBER}         { return p::make_CONFIG_DIR_PCRE_MATCH_LIMIT_RECURSION(strchr(yytext, ' ') + 1, *driver.loc.back()); }
 {CONFIG_DIR_PCRE_MATCH_LIMIT}[ \t]+{CONFIG_VALUE_NUMBER}                   { return p::make_CONFIG_DIR_PCRE_MATCH_LIMIT(strchr(yytext, ' ') + 1, *driver.loc.back()); }
+{CONFIG_DIR_ARGS_LIMIT}[ \t]+{CONFIG_VALUE_NUMBER}                         { return p::make_CONFIG_DIR_ARGS_LIMIT(strchr(yytext, ' ') + 1, *driver.loc.back()); }
 {CONFIG_DIR_REQ_BODY_IN_MEMORY_LIMIT}[ \t]+{CONFIG_VALUE_NUMBER}           { return p::make_CONFIG_DIR_REQ_BODY_IN_MEMORY_LIMIT(strchr(yytext, ' ') + 1, *driver.loc.back()); }
 {CONFIG_DIR_REQ_BODY_LIMIT_ACTION}                                      { return p::make_CONFIG_DIR_REQ_BODY_LIMIT_ACTION(yytext, *driver.loc.back()); }
 {CONFIG_DIR_REQ_BODY_LIMIT}[ \t]+{CONFIG_VALUE_NUMBER}                     { return p::make_CONFIG_DIR_REQ_BODY_LIMIT(strchr(yytext, ' ') + 1, *driver.loc.back()); }

src/request_body_processor/json.cc

@@ -142,7 +142,10 @@ int JSON::addArgument(const std::string& value) {
     }
 
 
-    m_transaction->addArgument("JSON", path + data, value, 0);
+    if (!m_transaction->addArgument("JSON", path + data, value, 0)) {
+        // cancel parsing by returning false
+        return 0;
+    }
 
     return 1;
 }

src/transaction.cc

@@ -334,6 +334,12 @@ bool Transaction::addArgument(const std::string& orig, const std::string& key,
     ms_dbg(4, "Adding request argument (" + orig + "): name \"" + \
                 key + "\", value \"" + value + "\"");
 
+    if (m_rules->m_argumentsLimit.m_set
+            && m_variableArgs.size() >= m_rules->m_argumentsLimit.m_value) {
+        ms_dbg(4, "Skipping request argument, over limit (" + std::to_string(m_rules->m_argumentsLimit.m_value) + ")")
+        return false;
+    }
+
     size_t k_offset = offset;
     offset = offset + key.size() + 1;
     m_variableArgs.set(key, value, offset);

test/test-cases/regression/secargumentslimit.json

@@ -0,0 +1,86 @@
+[
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing SecArgumentLimit not over limit (1/1)",
+    "client":{
+      "ip":"200.249.12.31",
+      "port":123
+    },
+    "server":{
+      "ip":"200.249.12.31",
+      "port":80
+    },
+    "request":{
+      "headers":{
+        "Host":"localhost",
+        "User-Agent":"curl/7.38.0",
+        "Content-Type":"application/json"
+      },
+      "uri":"/?foo=bar",
+      "method":"POST",
+      "body": [
+        "{",
+        "       \"k1\":\"v1\",",
+        "       \"k2\":\"v2\",",
+        "       \"k3\":\"v3\",",
+        "       \"k4\":\"v4\",",
+        "       \"k5\":\"v5\"",
+        "}"
+      ]
+    },
+    "expected":{
+      "debug_log": " Running action deny",
+      "http_code":403
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecArgumentsLimit 6",
+      "SecRule REQUEST_HEADERS:Content-Type \"application/json\" \"id:'200001',phase:1,t:none,pass,nolog,ctl:requestBodyProcessor=JSON\"",
+      "SecRule REQBODY_ERROR \"!@eq 0\" \"id:'200002', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}'\"",
+      "SecRule ARGS:/k5/ \"@rx v5\" \"id:'1234',phase:2,deny,status:403,t:none,log,auditlog\""
+    ]
+  },
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing SecArgumentLimit over limit (2/2)",
+    "client":{
+      "ip":"200.249.12.31",
+      "port":123
+    },
+    "server":{
+      "ip":"200.249.12.31",
+      "port":80
+    },
+    "request":{
+      "headers":{
+        "Host":"localhost",
+        "User-Agent":"curl/7.38.0",
+        "Content-Type":"application/json"
+      },
+      "uri":"/?foo=bar",
+      "method":"POST",
+      "body": [
+        "{",
+        "       \"k1\":\"v1\",",
+        "       \"k2\":\"v2\",",
+        "       \"k3\":\"v3\",",
+        "       \"k4\":\"v4\",",
+        "       \"k5\":\"v5\"",
+        "}"
+      ]
+    },
+    "expected":{
+      "debug_log": "Skipping request argument, over limit",
+      "http_code":400
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecArgumentsLimit 5",
+      "SecRule REQUEST_HEADERS:Content-Type \"application/json\" \"id:'200001',phase:1,t:none,pass,nolog,ctl:requestBodyProcessor=JSON\"",
+      "SecRule REQBODY_ERROR \"!@eq 0\" \"id:'200002', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}'\"",
+      "SecRule ARGS:/k5/ \"@rx v5\" \"id:'1234',phase:2,deny,status:403,t:none,log,auditlog\""
+    ]
+  }
+]