diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc index 8cdbd12ff..d14108a26 100644 --- a/src/request_body_processor/multipart.cc +++ b/src/request_body_processor/multipart.cc @@ -1346,11 +1346,21 @@ int Multipart::count_boundary_params(const std::string& str_header_value) { int count = 0; const auto lower = utils::string::tolower(str_header_value); - const char *s = lower.c_str(); + const char *start = lower.c_str(); + const char *s = start; while ((s = strstr(s, "boundary")) != NULL) { + /* Only count "boundary" when it is a parameter name: at the start + * or after a ';' / whitespace, and followed by '='. A boundary value + * such as "----geckoformboundary..." followed by another parameter + * ("charset=UTF-8") must not count (#3610). */ + bool param_start = (s == start) || s[-1] == ';' || isspace(s[-1]); s += 8; - - if (strchr(s, '=') != NULL) { + const char *p = s; + /* Same whitespace as Multipart::init accepts before the '='. */ + while (isspace(*p)) { + p++; + } + if (param_start && *p == '=') { count++; } } diff --git a/test/test-cases/regression/request-body-parser-multipart-boundary-count.json b/test/test-cases/regression/request-body-parser-multipart-boundary-count.json new file mode 100644 index 000000000..b42b01f32 --- /dev/null +++ b/test/test-cases/regression/request-body-parser-multipart-boundary-count.json @@ -0,0 +1,235 @@ +[ + { + "enabled": 1, + "version_min": 300000, + "title": "multipart parser (boundary parameter count) (1) - word boundary inside the value followed by another parameter (#3610)", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "Content-Type": "multipart/form-data;boundary=----geckoformboundary4a7acee3709293e183ef2932a147a331;charset=UTF-8", + "Content-Length": "169" + }, + "uri": "/", + "method": "POST", + "body": [ + "------geckoformboundary4a7acee3709293e183ef2932a147a331\r\n", + "Content-Disposition: form-data; name=\"namea\"\r\n", + "\r\n", + "111\r\n", + "------geckoformboundary4a7acee3709293e183ef2932a147a331--\r\n" + ] + }, + "response": { + "headers": { + "Content-Type": "text/html" + }, + "body": [ + "no need." + ] + }, + "expected": { + "http_code": 403 + }, + "rules": [ + "SecRuleEngine On", + "SecRequestBodyAccess On", + "SecRule REQBODY_PROCESSOR_ERROR \"@eq 1\" \"phase:2,deny,status:400,id:500097,msg:'%{REQBODY_ERROR_MSG}'\"", + "SecRule ARGS:namea \"@streq 111\" \"phase:2,deny,status:403,id:500098\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "multipart parser (boundary parameter count) (2) - value starting with the word boundary followed by another parameter", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "Content-Type": "multipart/form-data; boundary=boundary0000; charset=UTF-8", + "Content-Length": "87" + }, + "uri": "/", + "method": "POST", + "body": [ + "--boundary0000\r\n", + "Content-Disposition: form-data; name=\"namea\"\r\n", + "\r\n", + "111\r\n", + "--boundary0000--\r\n" + ] + }, + "response": { + "headers": { + "Content-Type": "text/html" + }, + "body": [ + "no need." + ] + }, + "expected": { + "http_code": 403 + }, + "rules": [ + "SecRuleEngine On", + "SecRequestBodyAccess On", + "SecRule REQBODY_PROCESSOR_ERROR \"@eq 1\" \"phase:2,deny,status:400,id:500097,msg:'%{REQBODY_ERROR_MSG}'\"", + "SecRule ARGS:namea \"@streq 111\" \"phase:2,deny,status:403,id:500098\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "multipart parser (boundary parameter count) (3) - two boundary parameters are still rejected", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "Content-Type": "multipart/form-data; boundary=0000; boundary=1111", + "Content-Length": "71" + }, + "uri": "/", + "method": "POST", + "body": [ + "--0000\r\n", + "Content-Disposition: form-data; name=\"namea\"\r\n", + "\r\n", + "111\r\n", + "--0000--\r\n" + ] + }, + "response": { + "headers": { + "Content-Type": "text/html" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Multiple boundary parameters in C-T", + "http_code": 400 + }, + "rules": [ + "SecRuleEngine On", + "SecRequestBodyAccess On", + "SecRule REQBODY_PROCESSOR_ERROR \"@eq 1\" \"phase:2,deny,status:400,id:500097,msg:'%{REQBODY_ERROR_MSG}'\"", + "SecRule ARGS:namea \"@streq 111\" \"phase:2,deny,status:403,id:500098\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "multipart parser (boundary parameter count) (4) - two boundary parameters with whitespace before the equals are still rejected", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "Content-Type": "multipart/form-data; boundary=0000; boundary =1111", + "Content-Length": "71" + }, + "uri": "/", + "method": "POST", + "body": [ + "--0000\r\n", + "Content-Disposition: form-data; name=\"namea\"\r\n", + "\r\n", + "111\r\n", + "--0000--\r\n" + ] + }, + "response": { + "headers": { + "Content-Type": "text/html" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Multiple boundary parameters in C-T", + "http_code": 400 + }, + "rules": [ + "SecRuleEngine On", + "SecRequestBodyAccess On", + "SecRule REQBODY_PROCESSOR_ERROR \"@eq 1\" \"phase:2,deny,status:400,id:500097,msg:'%{REQBODY_ERROR_MSG}'\"", + "SecRule ARGS:namea \"@streq 111\" \"phase:2,deny,status:403,id:500098\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "multipart parser (boundary parameter count) (5) - two boundary parameters with a vertical tab before the equals are still rejected", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "Content-Type": "multipart/form-data; boundary=0000; boundary\u000b=1111", + "Content-Length": "71" + }, + "uri": "/", + "method": "POST", + "body": [ + "--0000\r\n", + "Content-Disposition: form-data; name=\"namea\"\r\n", + "\r\n", + "111\r\n", + "--0000--\r\n" + ] + }, + "response": { + "headers": { + "Content-Type": "text/html" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Multiple boundary parameters in C-T", + "http_code": 400 + }, + "rules": [ + "SecRuleEngine On", + "SecRequestBodyAccess On", + "SecRule REQBODY_PROCESSOR_ERROR \"@eq 1\" \"phase:2,deny,status:400,id:500097,msg:'%{REQBODY_ERROR_MSG}'\"", + "SecRule ARGS:namea \"@streq 111\" \"phase:2,deny,status:403,id:500098\"" + ] + } +] \ No newline at end of file diff --git a/test/test-suite.in b/test/test-suite.in index da1c44df1..cade81580 100644 --- a/test/test-suite.in +++ b/test/test-suite.in @@ -100,6 +100,7 @@ TESTS+=test/test-cases/regression/operator-verifysvnr.json TESTS+=test/test-cases/regression/request-body-parser-json.json TESTS+=test/test-cases/regression/request-body-parser-multipart-crlf.json TESTS+=test/test-cases/regression/request-body-parser-multipart.json +TESTS+=test/test-cases/regression/request-body-parser-multipart-boundary-count.json TESTS+=test/test-cases/regression/request-body-parser-xml.json TESTS+=test/test-cases/regression/request-body-parser-xml-validade-dtd.json TESTS+=test/test-cases/regression/rule-920120.json