diff --git a/CHANGES b/CHANGES index 3effd8876..6b2ef91de 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ v3.x.y - YYYY-MMM-DD (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] diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc index e09ba320a..7e44fdaf6 100644 --- a/src/request_body_processor/multipart.cc +++ b/src/request_body_processor/multipart.cc @@ -227,12 +227,18 @@ int Multipart::boundary_characters_valid(const char *boundary) { } -void Multipart::validate_quotes(const char *data) { +void Multipart::validate_quotes(const char *data, char quote) { int i, len; 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++) { @@ -318,6 +324,7 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) { return -6; } + char quote = '\0'; if (name == "filename*") { /* filename*=charset'[optional-language]'filename */ /* Read beyond the charset and the optional language*/ @@ -357,7 +364,7 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) { * technically "'" is invalid and so flag_invalid_quoting is * set so the user can deal with it in the rules if they so wish. */ - char quote = *p; + quote = *p; // remember which quote character was used for the value if (quote == '\'') { m_flag_invalid_quoting = 1; @@ -408,7 +415,7 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) { /* evaluate part */ if (name == "name") { - validate_quotes(value.c_str()); + validate_quotes(value.c_str(), quote); m_transaction->m_variableMultipartName.set(value, value, offset + ((p - c_d_value) - value.size())); @@ -424,7 +431,7 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) { ms_dbg_a(m_transaction, 9, "Multipart: Content-Disposition name: " + value + "."); } else if (name == "filename") { - validate_quotes(value.c_str()); + validate_quotes(value.c_str(), quote); m_transaction->m_variableMultipartFileName.set(value, value, \ offset + ((p - c_d_value) - value.size())); diff --git a/src/request_body_processor/multipart.h b/src/request_body_processor/multipart.h index 66c857ac1..185656e76 100644 --- a/src/request_body_processor/multipart.h +++ b/src/request_body_processor/multipart.h @@ -162,7 +162,7 @@ class Multipart { int process_part_header(std::string *error, int offset); int process_part_data(std::string *error, size_t offset); - void validate_quotes(const char *data); + void validate_quotes(const char *data, char quote); size_t m_reqbody_no_files_length; std::list m_parts; diff --git a/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json b/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json index 9e53bb643..402e4541d 100644 --- a/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json +++ b/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json @@ -342,6 +342,126 @@ "SecRuleEngine On", "SecRule REQBODY_ERROR \"@contains 0\" \"id:1,phase:3,pass,t:trim\"" ] + }, + { + "enabled":1, + "version_min":300000, + "title":"Testing Variables :: MULTIPART_STRICT_ERROR - IQ ", + "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=--------------------------756b6d74fa1a8ee2", + "Expect":"100-continue" + }, + "uri":"/", + "method":"POST", + "body":[ + "----------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"name\"", + "", + "test", + "----------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=file'data; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is a very small test file..", + "----------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is another very small test file..", + "----------------------------756b6d74fa1a8ee2--" + ] + }, + "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":{ + "http_code": 403, + "debug_log":"Warning: invalid quoting used" + }, + "rules":[ + "SecRuleEngine On", + "SecRule MULTIPART_STRICT_ERROR \"!@eq 0\" \"id:1,phase:2,deny,status:403\"" + ] + }, + { + "enabled":1, + "version_min":300000, + "title":"Testing Variables :: MULTIPART_STRICT_ERROR - IQ ", + "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=--------------------------756b6d74fa1a8ee2", + "Expect":"100-continue" + }, + "uri":"/", + "method":"POST", + "body":[ + "----------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"name\"", + "", + "test", + "----------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"file'data\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is a very small test file..", + "----------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is another very small test file..", + "----------------------------756b6d74fa1a8ee2--" + ] + }, + "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":{ + "http_code": 200 + }, + "rules":[ + "SecRuleEngine On", + "SecRule MULTIPART_INVALID_QUOTING \"!@eq 0\" \"id:1,phase:2,deny,status:403\"", + "SecRule MULTIPART_STRICT_ERROR \"!@eq 0\" \"id:2,phase:2,pass\"" + ] } ]