Added SecUploadFileLimit (MODSEC-116).

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.
 

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,

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;

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));
                 }
             }

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;
 };
 
 

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",

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--
+            ),
+        ),
+    ),
+},
+

doc/modsecurity2-apache-reference.xml

@@ -2530,6 +2530,42 @@ SecRuleUpdateActionById 12345 "t:compressWhitespace,deny,status:403,msg:'A new m
       <literal>SecUploadKeepFiles</literal>.</para>
     </section>
 
+    <section>
+      <title><literal>SecUploadFileLimit</literal></title>
+
+      <para><emphasis>Description:</emphasis> Configures the maximum number of
+      file uploads processed in a multipart POST.</para>
+
+      <para><emphasis>Syntax:</emphasis> <literal
+      moreinfo="none">SecUploadFileLimit number</literal></para>
+
+      <para><emphasis>Example Usage:</emphasis> <literal
+      moreinfo="none">SecUploadFileLimit 10</literal></para>
+
+      <para><emphasis>Processing Phase:</emphasis> N/A</para>
+
+      <para><emphasis>Scope:</emphasis> Any</para>
+
+      <para><emphasis>Version:</emphasis> 2.5.12</para>
+
+      <para><emphasis>Dependencies/Notes:</emphasis> 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 <literal
+      moreinfo="none">MULTIPART_FILE_LIMIT_EXCEEDED</literal> and <literal
+      moreinfo="none">MULTIPART_STRICT_ERROR</literal> flags will be set. To
+      prevent bypassing any file checks, you must check for one of these
+      flags.</para>
+
+      <note>
+        <para>If the limit is exceeded, the part name and file name will still
+        be recorded in <literal moreinfo="none">FILES_NAME</literal> and
+        <literal moreinfo="none">FILES</literal>, the file size will be
+        recorded in <literal moreinfo="none">FILES_SIZES</literal>, but there
+        will be no record in <literal moreinfo="none">FILES_TMPNAMES</literal>
+        as a temporary file was not created.</para>
+      </note>
+    </section>
+
     <section>
       <title><literal>SecUploadFileMode</literal></title>
 
@@ -3174,7 +3210,8 @@ SecRule ARGS "@pm some key words" id:12345,deny,status:500</programlisting>
       <literal>MULTIPART_LF_LINE</literal>,
       <literal>MULTIPART_SEMICOLON_MISSING</literal>
       <literal>MULTIPART_INVALID_QUOTING</literal>
-      <literal>MULTIPART_INVALID_HEADER_FOLDING</literal>. Each of these
+      <literal>MULTIPART_INVALID_HEADER_FOLDING</literal>
+      <literal>MULTIPART_FILE_LIMIT_EXCEEDED</literal>. Each of these
       variables covers one unusual (although sometimes legal) aspect of the
       request body in <literal>multipart/form-data format</literal>. Your
       policies should <emphasis>always</emphasis> 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}'"</programlisting>
+IQ %{MULTIPART_INVALID_HEADER_FOLDING}, \
+FE %{MULTIPART_FILE_LIMIT_EXCEEDED}'"</programlisting>
 
       <para>The <literal>multipart/form-data</literal> parser was upgraded in
       ModSecurity v2.1.3 to actively look for signs of evasion. Many variables

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" \