Skip to content

Support configurable limit on number of arguments processed #2844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
@@ -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
Expand Down
30 changes: 30 additions & 0 deletions apache2/apache2_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion apache2/modsecurity.h
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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;
Expand Down
11 changes: 10 additions & 1 deletion apache2/msc_json.c
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
Expand Down
19 changes: 17 additions & 2 deletions apache2/msc_parsers.c
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
}
}

47 changes: 47 additions & 0 deletions tests/regression/config/10-request-directives.t
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
},