diff --git a/CHANGES b/CHANGES
index c79a4351..c30a3534 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
-25 Jan 2010 - 2.5.12
+28 Jan 2010 - 2.5.12
--------------------
+ * Added SecUploadFileLimit to limit the number of uploaded file parts that
+ will be processed in a multipart POST. The default is 100.
+
* Fixed path normalization to better handle backreferences that extend
above root directories. Reported by Sogeti/ESEC R&D.
diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c
index 7ef3996b..83cb996e 100644
--- a/apache2/apache2_config.c
+++ b/apache2/apache2_config.c
@@ -89,6 +89,7 @@ void *create_directory_config(apr_pool_t *mp, char *path)
dcfg->upload_keep_files = NOT_SET;
dcfg->upload_validates_files = NOT_SET;
dcfg->upload_filemode = NOT_SET;
+ dcfg->upload_file_limit = NOT_SET;
/* These are only used during the configuration process. */
dcfg->tmp_chain_starter = NULL;
@@ -432,6 +433,8 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child)
? parent->upload_validates_files : child->upload_validates_files);
merged->upload_filemode = (child->upload_filemode == NOT_SET
? parent->upload_filemode : child->upload_filemode);
+ merged->upload_file_limit = (child->upload_file_limit == NOT_SET
+ ? parent->upload_file_limit : child->upload_file_limit);
/* Misc */
merged->data_dir = (child->data_dir == NOT_SET_P
@@ -541,6 +544,7 @@ void init_directory_config(directory_config *dcfg)
if (dcfg->upload_keep_files == NOT_SET) dcfg->upload_keep_files = KEEP_FILES_OFF;
if (dcfg->upload_validates_files == NOT_SET) dcfg->upload_validates_files = 0;
if (dcfg->upload_filemode == NOT_SET) dcfg->upload_filemode = mode2fileperms(0600);
+ if (dcfg->upload_file_limit == NOT_SET) dcfg->upload_file_limit = 100;
/* Misc */
if (dcfg->data_dir == NOT_SET_P) dcfg->data_dir = NULL;
@@ -1622,6 +1626,23 @@ static const char *cmd_upload_dir(cmd_parms *cmd, void *_dcfg, const char *p1)
return NULL;
}
+static const char *cmd_upload_file_limit(cmd_parms *cmd, void *_dcfg,
+ const char *p1)
+{
+ directory_config *dcfg = (directory_config *)_dcfg;
+
+ if (dcfg == NULL) return NULL;
+
+ if (strcasecmp(p1, "default") == 0) {
+ dcfg->upload_file_limit = NOT_SET;
+ }
+ else {
+ dcfg->upload_file_limit = atoi(p1);
+ }
+
+ return NULL;
+}
+
static const char *cmd_upload_filemode(cmd_parms *cmd, void *_dcfg,
const char *p1)
{
@@ -2333,6 +2354,14 @@ const command_rec module_directives[] = {
"path to the file upload area"
),
+ AP_INIT_TAKE1 (
+ "SecUploadFileLimit",
+ cmd_upload_file_limit,
+ NULL,
+ CMD_SCOPE_ANY,
+ "limit the number of uploaded files processed"
+ ),
+
AP_INIT_TAKE1 (
"SecUploadFileMode",
cmd_upload_filemode,
diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h
index 096461e8..8aa0b5f5 100644
--- a/apache2/modsecurity.h
+++ b/apache2/modsecurity.h
@@ -331,6 +331,7 @@ struct modsec_rec {
/* upload */
int upload_extract_files;
int upload_remove_files;
+ int upload_files_count;
/* other */
apr_table_t *collections_original;
@@ -437,6 +438,7 @@ struct directory_config {
int upload_keep_files;
int upload_validates_files;
int upload_filemode; /* int only so NOT_SET works */
+ int upload_file_limit;
/* Used only in the configuration phase. */
msre_rule *tmp_chain_starter;
diff --git a/apache2/msc_multipart.c b/apache2/msc_multipart.c
index 89b152dd..c520ca84 100644
--- a/apache2/msc_multipart.c
+++ b/apache2/msc_multipart.c
@@ -413,16 +413,32 @@ static int multipart_process_part_data(modsec_rec *msr, char **error_msg) {
/* add data to the part we are building */
if (msr->mpd->mpp->type == MULTIPART_FILE) {
+ int extract = msr->upload_extract_files;
/* remember where we started */
if (msr->mpd->mpp->length == 0) {
msr->mpd->mpp->offset = msr->mpd->buf_offset;
}
+ /* check if the file limit has been reached */
+ if (extract && (msr->mpd->nfiles >= msr->txcfg->upload_file_limit)) {
+ if (msr->mpd->flag_file_limit_exceeded == 0) {
+ *error_msg = apr_psprintf(msr->mp,
+ "Multipart: Upload file limit exceeded "
+ "SecUploadFileLimit %d.",
+ msr->txcfg->upload_file_limit);
+ msr_log(msr, 3, "%s", *error_msg);
+
+ msr->mpd->flag_file_limit_exceeded = 1;
+ }
+
+ extract = 0;
+ }
+
/* only store individual files on disk if we are going
* to keep them or if we need to have them approved later
*/
- if (msr->upload_extract_files) {
+ if (extract) {
/* first create a temporary file if we don't have it already */
if (msr->mpd->mpp->tmp_file_fd == 0) {
/* construct temporary file name */
@@ -437,8 +453,12 @@ static int multipart_process_part_data(modsec_rec *msr, char **error_msg) {
return -1;
}
+ /* keep track of the files count */
+ msr->mpd->nfiles++;
+
if (msr->txcfg->debuglog_level >= 4) {
- msr_log(msr, 4, "Multipart: Created temporary file: %s",
+ msr_log(msr, 4, "Multipart: Created temporary file %d: %s",
+ msr->mpd->nfiles,
log_escape_nq(msr->mp, msr->mpd->mpp->tmp_file_name));
}
}
diff --git a/apache2/msc_multipart.h b/apache2/msc_multipart.h
index 1ad9d2f9..5fff0fce 100644
--- a/apache2/msc_multipart.h
+++ b/apache2/msc_multipart.h
@@ -68,6 +68,9 @@ struct multipart_data {
/* this array keeps parts */
apr_array_header_t *parts;
+ /* Number of parts that are files */
+ int nfiles;
+
/* mime boundary used to detect when
* parts end and begin
*/
@@ -119,6 +122,7 @@ struct multipart_data {
int flag_missing_semicolon;
int flag_invalid_quoting;
int flag_invalid_header_folding;
+ int flag_file_limit_exceeded;
};
diff --git a/apache2/re_variables.c b/apache2/re_variables.c
index 2868a376..ccc56d28 100644
--- a/apache2/re_variables.c
+++ b/apache2/re_variables.c
@@ -1390,6 +1390,18 @@ static int var_multipart_invalid_header_folding_generate(modsec_rec *msr, msre_v
}
}
+/* MULTIPART_FILE_LIMIT_EXCEEDED */
+
+static int var_multipart_file_limit_exceeded_generate(modsec_rec *msr, msre_var *var, msre_rule *rule,
+ apr_table_t *vartab, apr_pool_t *mptmp)
+{
+ if ((msr->mpd != NULL)&&(msr->mpd->flag_file_limit_exceeded != 0)) {
+ return var_simple_generate(var, vartab, mptmp, "1");
+ } else {
+ return var_simple_generate(var, vartab, mptmp, "0");
+ }
+}
+
/* MULTIPART_STRICT_ERROR */
static int var_multipart_strict_error_generate(modsec_rec *msr, msre_var *var, msre_rule *rule,
@@ -1407,6 +1419,7 @@ static int var_multipart_strict_error_generate(modsec_rec *msr, msre_var *var, m
||(msr->mpd->flag_missing_semicolon != 0)
||(msr->mpd->flag_invalid_quoting != 0)
||(msr->mpd->flag_invalid_header_folding != 0)
+ ||(msr->mpd->flag_file_limit_exceeded != 0)
) {
return var_simple_generate(var, vartab, mptmp, "1");
}
@@ -2502,6 +2515,17 @@ void msre_engine_register_default_variables(msre_engine *engine) {
PHASE_REQUEST_BODY
);
+ /* MULTIPART_FILE_LIMIT_EXCEEDED */
+ msre_engine_variable_register(engine,
+ "MULTIPART_FILE_LIMIT_EXCEEDED",
+ VAR_SIMPLE,
+ 0, 0,
+ NULL,
+ var_multipart_file_limit_exceeded_generate,
+ VAR_DONT_CACHE, /* flag */
+ PHASE_REQUEST_BODY
+ );
+
/* MULTIPART_STRICT_ERROR */
msre_engine_variable_register(engine,
"MULTIPART_STRICT_ERROR",
diff --git a/apache2/t/regression/misc/00-multipart-parser.t b/apache2/t/regression/misc/00-multipart-parser.t
index da1a3eee..818dd23a 100644
--- a/apache2/t/regression/misc/00-multipart-parser.t
+++ b/apache2/t/regression/misc/00-multipart-parser.t
@@ -1737,3 +1737,80 @@
),
),
},
+
+# File limits
+{
+ type => "misc",
+ comment => "multipart parser (normal)",
+ conf => qq(
+ SecRuleEngine On
+ SecDebugLog $ENV{DEBUG_LOG}
+ SecDebugLogLevel 9
+ SecRequestBodyAccess On
+ SecTmpDir "$ENV{TEMP_DIR}"
+ SecUploadDir "$ENV{UPLOAD_DIR}"
+ SecUploadKeepFiles On
+ SecUploadFileLimit 2
+
+ # These should be set
+ SecRule MULTIPART_STRICT_ERROR "!\@eq 1" "phase:2,deny"
+ SecRule MULTIPART_FILE_LIMIT_EXCEEDED "!\@eq 1" "phase:2,deny"
+
+ # This should not be set
+ SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny"
+
+ # Theses should still be accurate
+ SecRule &FILES "!\@eq 3" "phase:2,deny"
+ SecRule &FILES_NAMES "!\@eq 3" "phase:2,deny"
+ SecRule &FILES_SIZES "!\@eq 3" "phase:2,deny"
+ SecRule FILES_SIZES:/^image/ "\@eq 0" "phase:2,deny"
+
+ # This should be the SecUploadFileLimit
+ SecRule &FILES_TMPNAMES "!\@eq 2" "phase:2,deny"
+ ),
+ match_log => {
+ debug => [ qr/Multipart: Upload file limit exceeded.*name: test.*variable: This is test data/s, 1 ],
+ },
+ match_response => {
+ status => qr/^200$/,
+ },
+ request => new HTTP::Request(
+ POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
+ [
+ "Content-Type" => q(multipart/form-data; boundary=0000),
+ ],
+ normalize_raw_request_data(
+ q(
+ --0000
+ Content-Disposition: form-data; name="name"
+
+ Brian Rectanus
+ --0000
+ Content-Disposition: form-data; name="email"
+
+ brian.rectanus@breach.com
+ --0000
+ Content-Disposition: form-data; name="image1"; filename="image1.jpg"
+ Content-Type: image/jpeg
+
+ BINARYDATA1
+ --0000
+ Content-Disposition: form-data; name="image2"; filename="image2.jpg"
+ Content-Type: image/jpeg
+
+ BINARYDATA2
+ --0000
+ Content-Disposition: form-data; name="image3"; filename="image3.jpg"
+ Content-Type: image/jpeg
+
+ BINARYDATA3
+ --0000
+ Content-Disposition: form-data; name="test"
+
+ This is test data.
+ --0000--
+ ),
+ ),
+ ),
+},
+
diff --git a/doc/modsecurity2-apache-reference.xml b/doc/modsecurity2-apache-reference.xml
index 902e30a4..835bd8b8 100644
--- a/doc/modsecurity2-apache-reference.xml
+++ b/doc/modsecurity2-apache-reference.xml
@@ -2530,6 +2530,42 @@ SecRuleUpdateActionById 12345 "t:compressWhitespace,deny,status:403,msg:'A new m
SecUploadKeepFiles.
+
+ SecUploadFileLimit
+
+ Description: Configures the maximum number of
+ file uploads processed in a multipart POST.
+
+ Syntax: SecUploadFileLimit number
+
+ Example Usage: SecUploadFileLimit 10
+
+ Processing Phase: N/A
+
+ Scope: Any
+
+ Version: 2.5.12
+
+ Dependencies/Notes: The default is set to 100
+ files, but you are encouraged to reduce this value. Any file over the
+ limit will not be extracted and the MULTIPART_FILE_LIMIT_EXCEEDED and MULTIPART_STRICT_ERROR flags will be set. To
+ prevent bypassing any file checks, you must check for one of these
+ flags.
+
+
+ If the limit is exceeded, the part name and file name will still
+ be recorded in FILES_NAME and
+ FILES, the file size will be
+ recorded in FILES_SIZES, but there
+ will be no record in FILES_TMPNAMES
+ as a temporary file was not created.
+
+
+
SecUploadFileMode
@@ -3174,7 +3210,8 @@ SecRule ARGS "@pm some key words" id:12345,deny,status:500
MULTIPART_LF_LINE,
MULTIPART_SEMICOLON_MISSING
MULTIPART_INVALID_QUOTING
- MULTIPART_INVALID_HEADER_FOLDING. Each of these
+ MULTIPART_INVALID_HEADER_FOLDING
+ MULTIPART_FILE_LIMIT_EXCEEDED. Each of these
variables covers one unusual (although sometimes legal) aspect of the
request body in multipart/form-data format. Your
policies should always contain a rule to check
@@ -3198,7 +3235,8 @@ HF %{MULTIPART_HEADER_FOLDING}, \
LF %{MULTIPART_LF_LINE}, \
SM %{MULTIPART_SEMICOLON_MISSING}, \
IQ %{MULTIPART_INVALID_QUOTING}, \
-IQ %{MULTIPART_INVALID_HEADER_FOLDING}'"
+IQ %{MULTIPART_INVALID_HEADER_FOLDING}, \
+FE %{MULTIPART_FILE_LIMIT_EXCEEDED}'"
The multipart/form-data parser was upgraded in
ModSecurity v2.1.3 to actively look for signs of evasion. Many variables
diff --git a/modsecurity.conf-minimal b/modsecurity.conf-minimal
index 584cac19..7afce654 100644
--- a/modsecurity.conf-minimal
+++ b/modsecurity.conf-minimal
@@ -12,6 +12,7 @@ SecPcreMatchLimitRecursion 1000
# TODO Choose a folder private to Apache.
# SecUploadDir /opt/apache-frontend/tmp/
SecUploadKeepFiles Off
+SecUploadFileLimit 10
# Debug log
SecDebugLog logs/modsec_debug.log
@@ -58,7 +59,8 @@ HF %{MULTIPART_HEADER_FOLDING}, \
LF %{MULTIPART_LF_LINE}, \
SM %{MULTIPART_SEMICOLON_MISSING}, \
IQ %{MULTIPART_INVALID_QUOTING}, \
-IH %{MULTIPART_INVALID_HEADER_FOLDING}'"
+IH %{MULTIPART_INVALID_HEADER_FOLDING}, \
+IH %{MULTIPART_FILE_LIMIT_EXCEEDED}'"
# Did we see anything that might be a boundary?
SecRule MULTIPART_UNMATCHED_BOUNDARY "!@eq 0" \