diff --git a/CHANGES b/CHANGES index 25b5bd9c..b4b4da5c 100644 --- a/CHANGES +++ b/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 diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index 0758e235..74c76a58 100644 --- a/apache2/apache2_config.c +++ b/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, diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index 261151ba..8e1880ed 100644 --- a/apache2/modsecurity.h +++ b/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; diff --git a/apache2/msc_json.c b/apache2/msc_json.c index 737069b8..17f938b6 100644 --- a/apache2/msc_json.c +++ b/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); diff --git a/apache2/msc_parsers.c b/apache2/msc_parsers.c index 61344aa1..8bbf9726 100644 --- a/apache2/msc_parsers.c +++ b/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); + } } diff --git a/tests/regression/config/10-request-directives.t b/tests/regression/config/10-request-directives.t index 42094e11..21b83ff1 100644 --- a/tests/regression/config/10-request-directives.t +++ b/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", + ), +}, +