Merge pull request #2660 from martinhsv/v2/master

Multipart names may include single quote if double-quote enclosed

Author: martinhsv

Diff for: CHANGES

@@ -1,6 +1,8 @@
 DD mmm YYYY - 2.9.x (to be released)
 -------------------
 
+ * Multipart names/filenames may include single quote if double-quote enclosed
+   [Issue #2352 @martinhsv]
  * Add SecRequestBodyJsonDepthLimit to modsecurity.conf-recommended
    [Issue #2647 @theMiddleBlue, @airween, @877509395 ,@martinhsv]
  * IIS: Update dependencies for Windows build as of v2.9.5

Diff for: apache2/msc_multipart.c

@@ -20,7 +20,7 @@
 #include "msc_util.h"
 #include "msc_parsers.h"
 
-void validate_quotes(modsec_rec *msr, char *data)  {
+void validate_quotes(modsec_rec *msr, char *data, char quote)  {
     int i, len;
 
     if(msr == NULL)
@@ -32,6 +32,12 @@ void validate_quotes(modsec_rec *msr, char *data)  {
     if(data == NULL)
         return;
 
+    // if the value was enclosed in double quotes, then we don't care about
+    // a single quote character within the name.
+    if (quote == '"') {
+        return;
+    }
+
     len = strlen(data);
 
     for(i = 0; i < len; i++)   {
@@ -123,9 +129,10 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value)
          * set so the user can deal with it in the rules if they so wish.
          */
 
+        char quote = '\0';
         if ((*p == '"') || (*p == '\'')) {
             /* quoted */
-            char quote = *p;
+            quote = *p; // remember which quote character was used for the value
 
             if (quote == '\'') {
                 msr->mpd->flag_invalid_quoting = 1;
@@ -182,7 +189,7 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value)
 
         if (strcmp(name, "name") == 0) {
 
-            validate_quotes(msr, value);
+            validate_quotes(msr, value, quote);
 
             msr->multipart_name = apr_pstrdup(msr->mp, value);
 
@@ -201,7 +208,7 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value)
         else
         if (strcmp(name, "filename") == 0) {
 
-            validate_quotes(msr, value);
+            validate_quotes(msr, value, quote);
 
             msr->multipart_filename = apr_pstrdup(msr->mp, value);
 

Diff for: tests/regression/misc/00-multipart-parser.t

@@ -736,6 +736,45 @@
     ),
 },
 
+# Single quote within double quotes is ok
+{
+    type => "misc",
+    comment => "multipart parser (C-D uses single quote within double quotes)",
+    conf => qq(
+        SecRuleEngine On
+        SecDebugLog $ENV{DEBUG_LOG}
+        SecDebugLogLevel 9
+        SecRequestBodyAccess On
+        SecRule MULTIPART_INVALID_QUOTING "!\@eq 0" "phase:2,deny,id:500169"
+    ),
+    match_log => {
+        debug => [ qr/Adding request argument \(BODY\): name "a'b/s, 1 ],
+        -debug => [ qr/Adding request argument \(BODY\): name "b/s, 1 ],
+    },
+    match_response => {
+        status => qr/^200$/,
+    },
+    request => new HTTP::Request(
+        POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
+        [
+            "Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646",
+        ],
+        normalize_raw_request_data(
+            q(
+                -----------------------------69343412719991675451336310646
+                Content-Disposition: form-data; name="a'b"
+                
+                1
+                -----------------------------69343412719991675451336310646
+                Content-Disposition: form-data; name="aaa"; filename="d'ummy"
+                
+                2
+                -----------------------------69343412719991675451336310646--
+            ),
+        ),
+    ),
+},
+
 # Invalid boundary separators
 {
     type => "misc",