Merge pull request #2844 from martinhsv/v2/master

Support configurable limit on number of arguments processed

Author: martinhsv

CHANGES

@@ -1,6 +1,8 @@
 DD mmm YYYY - 2.9.x (to be released)
 -------------------
 
+ * Support configurable limit on number of arguments processed
+   [Issue #2844 - @jleproust, @martinhsv]
  * Silence compiler warning about discarded const
    [Issue #2843 - @Steve8291, @martinhsv]
  * Support for JIT option for PCRE2

apache2/apache2_config.c

@@ -54,6 +54,7 @@ void *create_directory_config(apr_pool_t *mp, char *path)
     dcfg->reqbody_limit = NOT_SET;
     dcfg->reqbody_no_files_limit = NOT_SET;
     dcfg->reqbody_json_depth_limit = NOT_SET;
+    dcfg->arguments_limit = NOT_SET;
     dcfg->resbody_access = NOT_SET;
 
     dcfg->debuglog_name = NOT_SET_P;
@@ -338,6 +339,8 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child)
         ? parent->reqbody_no_files_limit : child->reqbody_no_files_limit);
     merged->reqbody_json_depth_limit = (child->reqbody_json_depth_limit == NOT_SET
         ? parent->reqbody_json_depth_limit : child->reqbody_json_depth_limit);
+    merged->arguments_limit = (child->arguments_limit == NOT_SET
+        ? parent->arguments_limit : child->arguments_limit);
     merged->resbody_access = (child->resbody_access == NOT_SET
         ? parent->resbody_access : child->resbody_access);
 
@@ -655,6 +658,7 @@ void init_directory_config(directory_config *dcfg)
     if (dcfg->reqbody_limit == NOT_SET) dcfg->reqbody_limit = REQUEST_BODY_DEFAULT_LIMIT;
     if (dcfg->reqbody_no_files_limit == NOT_SET) dcfg->reqbody_no_files_limit = REQUEST_BODY_NO_FILES_DEFAULT_LIMIT;
     if (dcfg->reqbody_json_depth_limit == NOT_SET) dcfg->reqbody_json_depth_limit = REQUEST_BODY_JSON_DEPTH_DEFAULT_LIMIT;
+    if (dcfg->arguments_limit == NOT_SET) dcfg->arguments_limit = ARGUMENTS_LIMIT;
     if (dcfg->resbody_access == NOT_SET) dcfg->resbody_access = 0;
     if (dcfg->of_limit == NOT_SET) dcfg->of_limit = RESPONSE_BODY_DEFAULT_LIMIT;
     if (dcfg->if_limit_action == NOT_SET) dcfg->if_limit_action = REQUEST_BODY_LIMIT_ACTION_REJECT;
@@ -1955,6 +1959,24 @@ static const char *cmd_request_body_json_depth_limit(cmd_parms *cmd, void *_dcfg
     return NULL;
 }
 
+static const char *cmd_arguments_limit(cmd_parms *cmd, void *_dcfg,
+                                                     const char *p1)
+{
+    directory_config *dcfg = (directory_config *)_dcfg;
+    long int limit;
+
+    if (dcfg == NULL) return NULL;
+
+    limit = strtol(p1, NULL, 10);
+    if ((limit == LONG_MAX)||(limit == LONG_MIN)||(limit <= 0)) {
+        return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecArgumentsLimit: %s", p1);
+    }
+
+    dcfg->arguments_limit = limit;
+
+    return NULL;
+}
+
 static const char *cmd_request_body_access(cmd_parms *cmd, void *_dcfg,
                                            const char *p1)
 {
@@ -3596,6 +3618,14 @@ const command_rec module_directives[] = {
         "maximum request body JSON parsing depth ModSecurity will accept."
     ),
 
+    AP_INIT_TAKE1 (
+        "SecArgumentsLimit",
+        cmd_arguments_limit,
+        NULL,
+        CMD_SCOPE_ANY,
+        "maximum number of ARGS that ModSecurity will accept."
+    ),
+
     AP_INIT_TAKE1 (
         "SecRequestEncoding",
         cmd_request_encoding,

apache2/modsecurity.h

@@ -1,6 +1,6 @@
 /*
 * ModSecurity for Apache 2.x, http://www.modsecurity.org/
-* Copyright (c) 2004-2013 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+* Copyright (c) 2004-2022 Trustwave Holdings, Inc. (http://www.trustwave.com/)
 *
 * You may not use this file except in compliance with
 * the License.  You may obtain a copy of the License at
@@ -96,6 +96,7 @@ typedef struct msc_parm msc_parm;
 #define REQUEST_BODY_DEFAULT_LIMIT              134217728
 #define REQUEST_BODY_NO_FILES_DEFAULT_LIMIT     1048576
 #define REQUEST_BODY_JSON_DEPTH_DEFAULT_LIMIT   10000
+#define ARGUMENTS_LIMIT                         1000
 #define RESPONSE_BODY_DEFAULT_LIMIT             524288
 #define RESPONSE_BODY_HARD_LIMIT                1073741824L
 
@@ -500,6 +501,7 @@ struct directory_config {
     long int             reqbody_limit;
     long int             reqbody_no_files_limit;
     long int             reqbody_json_depth_limit;
+    long int             arguments_limit;
     int                  resbody_access;
 
     long int             of_limit;

apache2/msc_json.c

@@ -1,6 +1,6 @@
 /*
  * ModSecurity for Apache 2.x, http://www.modsecurity.org/
- * Copyright (c) 2004-2011 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ * Copyright (c) 2004-2022 Trustwave Holdings, Inc. (http://www.trustwave.com/)
  *
  * You may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
@@ -57,6 +57,15 @@ int json_add_argument(modsec_rec *msr, const char *value, unsigned length)
         msr_log(msr, 9, "Adding JSON argument '%s' with value '%s'",
             arg->name, arg->value);
     }
+    if (apr_table_elts(msr->arguments)->nelts >= msr->txcfg->arguments_limit) {
+        if (msr->txcfg->debuglog_level >= 4) {
+            msr_log(msr, 4, "Skipping request argument, over limit (%s): name \"%s\", value \"%s\"",
+                    arg->origin, log_escape_ex(msr->mp, arg->name, arg->name_len),
+                    log_escape_ex(msr->mp, arg->value, arg->value_len));
+        }
+        msr->msc_reqbody_error = 1;
+        return 0;
+    }
 
     apr_table_addn(msr->arguments,
         log_escape_nq_ex(msr->mp, arg->name, arg->name_len), (void *) arg);

apache2/msc_parsers.c

@@ -1,6 +1,6 @@
 /*
 * ModSecurity for Apache 2.x, http://www.modsecurity.org/
-* Copyright (c) 2004-2013 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+* Copyright (c) 2004-2022 Trustwave Holdings, Inc. (http://www.trustwave.com/)
 *
 * You may not use this file except in compliance with
 * the License.  You may obtain a copy of the License at
@@ -346,6 +346,21 @@ void add_argument(modsec_rec *msr, apr_table_t *arguments, msc_arg *arg)
                 log_escape_ex(msr->mp, arg->value, arg->value_len));
     }
 
-    apr_table_addn(arguments, log_escape_nq_ex(msr->mp, arg->name, arg->name_len), (void *)arg);
+    if (apr_table_elts(arguments)->nelts >= msr->txcfg->arguments_limit) {
+        if (msr->txcfg->debuglog_level >= 4) {
+            msr_log(msr, 4, "Skipping request argument, over limit (%s): name \"%s\", value \"%s\"",
+                    arg->origin, log_escape_ex(msr->mp, arg->name, arg->name_len),
+                    log_escape_ex(msr->mp, arg->value, arg->value_len));
+        }
+        if (msr->msc_reqbody_error != 1) {
+            char *error_msg = apr_psprintf(msr->mp, "SecArgumentsLimit exceeded");
+            msr->msc_reqbody_error = 1;
+            if (error_msg != NULL) {
+                msr->msc_reqbody_error_msg = error_msg;
+            }
+        }
+    } else {
+        apr_table_addn(arguments, log_escape_nq_ex(msr->mp, arg->name, arg->name_len), (void *)arg);
+    }
 }
 

tests/regression/config/10-request-directives.t

@@ -703,3 +703,50 @@
 	),
 },
 
+# SecArgumentsLimit
+{
+	type => "config",
+	comment => "SecArgumentsLimit (pos)",
+	conf => qq(
+		SecRuleEngine On
+		SecRequestBodyAccess On
+		SecArgumentsLimit 5
+		SecRule REQBODY_ERROR "!\@eq 0" "id:'500232',phase:2,log,deny,status:403,msg:'Failed to parse request body'"
+	),
+	match_log => {
+		error => [ qr/Access denied with code 403 /, 1 ],
+	},
+	match_response => {
+		status => qr/^403$/,
+	},
+	request => new HTTP::Request(
+		POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
+		[
+			"Content-Type" => "application/x-www-form-urlencoded",
+		],
+		"a=1&b=2&c=3&d=4&e=5&f=6",
+	),
+},
+{
+	type => "config",
+	comment => "SecArgumentsLimit (neg)",
+	conf => qq(
+		SecRuleEngine On
+		SecRequestBodyAccess On
+		SecArgumentsLimit 5
+		SecRule REQBODY_ERROR "!\@eq 0" "id:'500233',phase:2,log,deny,status:403,msg:'Failed to parse request body'"
+	),
+	match_log => {
+	},
+	match_response => {
+		status => qr/^200$/,
+	},
+	request => new HTTP::Request(
+		POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
+		[
+			"Content-Type" => "application/x-www-form-urlencoded",
+		],
+		"a=1&b=2&c=3&d=4&e=5",
+	),
+},
+