Allow for more robust parsing for multipart header folding. Reported by Sogeti/ESEC R&D (MODSEC-118). Added additional multipart regression tests.

CHANGES

@@ -1,6 +1,9 @@
-14 Jan 2010 - 2.5.12
+18 Jan 2010 - 2.5.12
 --------------------
 
+ * Allow for more robust parsing for multipart header folding.  Reported
+   by Sogeti/ESEC R&D.
+
  * Fixed failure to match internally set TX variables with regex
    (TX:/.../) syntax.
  

apache2/msc_multipart.c

@@ -279,12 +279,20 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) {
     } else {
         /* Header line. */
 
-        if ((msr->mpd->buf[0] == '\t') || (msr->mpd->buf[0] == ' ')) {
+        if (isspace(msr->mpd->buf[0])) {
             char *header_value, *new_value, *data;
 
             /* header folding, add data to the header we are building */
             msr->mpd->flag_header_folding = 1;
 
+            /* RFC-2557 states header folding is SP / HTAB, but PHP and
+             * perhaps others will take any whitespace.  So, we accept,
+             * but with a flag set.
+             */
+            if ((msr->mpd->buf[0] != '\t') && (msr->mpd->buf[0] != ' ')) {
+                msr->mpd->flag_invalid_header_folding = 1;
+            }
+
             if (msr->mpd->mpp->last_header_name == NULL) {
                 /* we are not building a header at this moment */
                 *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid part header (folding error).");
@@ -293,7 +301,15 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) {
 
             /* locate the beginning of data */
             data = msr->mpd->buf;
-            while((*data == '\t') || (*data == ' ')) data++;
+            while(isspace(*data)) {
+                /* Flag invalid header folding if an invalid RFC-2557 character is used anywhere
+                 * in the folding prefix.
+                 */
+                if ((*data != '\t') && (*data != ' ')) {
+                    msr->mpd->flag_invalid_header_folding = 1;
+                }
+                data++;
+            }
 
             new_value = apr_pstrdup(msr->mp, data);
             remove_lf_crlf_inplace(new_value);
@@ -879,6 +895,14 @@ int multipart_complete(modsec_rec *msr, char **error_msg) {
         if (msr->mpd->flag_missing_semicolon) {
             msr_log(msr, 4, "Multipart: Warning: missing semicolon in C-T header.");
         }
+
+        if (msr->mpd->flag_invalid_quoting) {
+            msr_log(msr, 4, "Multipart: Warning: invalid quoting used.");
+        }
+
+        if (msr->mpd->flag_invalid_header_folding) {
+            msr_log(msr, 4, "Multipart: Warning: invalid header folding used.");
+        }        
     }
 
     if ((msr->mpd->seen_data != 0) && (msr->mpd->is_complete == 0)) {

apache2/msc_multipart.h

@@ -118,6 +118,7 @@ struct multipart_data {
     int                      flag_boundary_whitespace;
     int                      flag_missing_semicolon;
     int                      flag_invalid_quoting;
+    int                      flag_invalid_header_folding;
 };
 
 

apache2/re_variables.c

@@ -1378,6 +1378,18 @@ static int var_multipart_invalid_quoting_generate(modsec_rec *msr, msre_var *var
     }
 }
 
+/* MULTIPART_INVALID_HEADER_FOLDING */
+
+static int var_multipart_invalid_header_folding_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_invalid_header_folding != 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,
@@ -1394,6 +1406,7 @@ static int var_multipart_strict_error_generate(modsec_rec *msr, msre_var *var, m
             ||(msr->mpd->flag_lf_line != 0)
             ||(msr->mpd->flag_missing_semicolon != 0)
             ||(msr->mpd->flag_invalid_quoting != 0)
+            ||(msr->mpd->flag_invalid_header_folding != 0)
         ) {
             return var_simple_generate(var, vartab, mptmp, "1");
         }
@@ -2478,6 +2491,17 @@ void msre_engine_register_default_variables(msre_engine *engine) {
         PHASE_REQUEST_BODY
     );
 
+    /* MULTIPART_INVALID_HEADER_FOLDING */
+    msre_engine_variable_register(engine,
+        "MULTIPART_INVALID_HEADER_FOLDING",
+        VAR_SIMPLE,
+        0, 0,
+        NULL,
+        var_multipart_invalid_header_folding_generate,
+        VAR_DONT_CACHE, /* flag */
+        PHASE_REQUEST_BODY
+    );
+
     /* MULTIPART_STRICT_ERROR */
     msre_engine_variable_register(engine,
         "MULTIPART_STRICT_ERROR",

doc/modsecurity2-apache-reference.xml

@@ -3173,7 +3173,8 @@ SecRule ARGS "@pm some key words" id:12345,deny,status:500</programlisting>
       <literal>MULTIPART_HEADER_FOLDING</literal>,
       <literal>MULTIPART_LF_LINE</literal>,
       <literal>MULTIPART_SEMICOLON_MISSING</literal>
-      <literal>MULTIPART_INVALID_QUOTING</literal>. Each of these variables
+      <literal>MULTIPART_INVALID_QUOTING</literal>
+      <literal>MULTIPART_INVALID_HEADER_FOLDING</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 either this variable
@@ -3196,7 +3197,8 @@ DA %{MULTIPART_DATA_AFTER}, \
 HF %{MULTIPART_HEADER_FOLDING}, \
 LF %{MULTIPART_LF_LINE}, \
 SM %{MULTIPART_SEMICOLON_MISSING}, \
-IQ %{MULTIPART_INVALID_QUOTING}'"</programlisting>
+IQ %{MULTIPART_INVALID_QUOTING}, \
+IQ %{MULTIPART_INVALID_HEADER_FOLDING}'"</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

@@ -57,7 +57,8 @@ DA %{MULTIPART_DATA_AFTER}, \
 HF %{MULTIPART_HEADER_FOLDING}, \
 LF %{MULTIPART_LF_LINE}, \
 SM %{MULTIPART_SEMICOLON_MISSING}, \
-IQ %{MULTIPART_INVALID_QUOTING}'"
+IQ %{MULTIPART_INVALID_QUOTING}, \
+IH %{MULTIPART_INVALID_HEADER_FOLDING}'"
 
 # Did we see anything that might be a boundary?
 SecRule MULTIPART_UNMATCHED_BOUNDARY "!@eq 0" \