From b638e523afc45f42695ef04a3071609e13260579 Mon Sep 17 00:00:00 2001 From: Victor Hora Date: Wed, 14 Nov 2018 15:25:27 -0500 Subject: [PATCH] Make the boundary check less strict as per RFC2046 --- CHANGES | 4 +- src/request_body_processor/multipart.cc | 52 +++++++------------ .../variable-MULTIPART_STRICT_ERROR.json | 49 +++++++++++++++++ 3 files changed, 71 insertions(+), 34 deletions(-) diff --git a/CHANGES b/CHANGES index 1f2e882f..cd1a7999 100644 --- a/CHANGES +++ b/CHANGES @@ -1,7 +1,9 @@ v3.0.4 - YYYY-MMM-DD (to be released) ------------------------------------- - * Fix buffer size for utf8toUnicode transformation + - Make the boundary check less strict as per RFC2046 + [Issue #1943 - @victorhora, @allanbomsft] + - Fix buffer size for utf8toUnicode transformation [Issue #1208 - @katef, @victorhora] diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc index fdadc42a..f302720a 100644 --- a/src/request_body_processor/multipart.cc +++ b/src/request_body_processor/multipart.cc @@ -158,41 +158,27 @@ int Multipart::boundary_characters_valid(const char *boundary) { } while ((c = *p) != '\0') { - /* Control characters and space not allowed. */ - if (c < 32) { + // Check against allowed list defined in RFC2046 page 22 + if (!( + ('0' <= c && c <= '9') + || ('A' <= c && c <= 'Z') + || ('a' <= c && c <= 'z') + || (c == ' ' && *(p + 1) != '\0') // space allowed, but not as last character + || c == '\'' + || c == '(' + || c == ')' + || c == '+' + || c == '_' + || c == ',' + || c == '-' + || c == '.' + || c == '/' + || c == ':' + || c == '=' + || c == '?' + )) { return 0; } - - /* Non-ASCII characters not allowed. */ - if (c > 126) { - return 0; - } - - switch (c) { - /* Special characters not allowed. */ - case '(' : - case ')' : - case '<' : - case '>' : - case '@' : - case ',' : - case ';' : - case ':' : - case '\\' : - case '"' : - case '/' : - case '[' : - case ']' : - case '?' : - case '=' : - return 0; - break; - - default : - /* Do nothing. */ - break; - } - p++; } diff --git a/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json b/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json index be15ac12..9e53bb64 100644 --- a/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json +++ b/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json @@ -293,6 +293,55 @@ "SecRuleEngine On", "SecRule MULTIPART_STRICT_ERROR \"@contains 0\" \"id:1,phase:3,pass,t:trim\"" ] + }, + { + "enabled":1, + "version_min":300000, + "title":"Testing Variables :: MULTIPART_STRICT_ERROR - RFC2046", + "client":{ + "ip":"200.249.12.31", + "port":123 + }, + "server":{ + "ip":"200.249.12.31", + "port":80 + }, + "request":{ + "headers":{ + "Host":"localhost", + "User-Agent":"curl/7.38.0", + "Accept":"*/*", + "Content-Length":"330", + "Content-Type":"multipart/form-data; boundary=0123456789AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz '()+_,-./:=?", + "Expect":"100-continue" + }, + "uri":"/", + "method":"POST", + "body":[ + "--0123456789AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz '()+_,-./:=?", + "Content-Disposition: form-data; name=\"name\"", + "", + "1", + "--0123456789AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz '()+_,-./:=?--" + ] + }, + "response":{ + "headers":{ + "Date":"Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type":"text/html" + }, + "body":[ + "no need." + ] + }, + "expected":{ + "debug_log":"Target value: \"0\" \\(Variable: REQBODY_ERROR\\)" + }, + "rules":[ + "SecRuleEngine On", + "SecRule REQBODY_ERROR \"@contains 0\" \"id:1,phase:3,pass,t:trim\"" + ] } ]