From 9767d671ecf15521ec5edc86f950787f79d5315f Mon Sep 17 00:00:00 2001 From: Felipe Zipitria Date: Mon, 12 Oct 2026 22:15:49 -0300 Subject: [PATCH 1/3] fix: only count real boundary parameters in the multipart C-T header Multipart::count_boundary_params counted every occurrence of the word "boundary" that had an '=' anywhere after it. A Content-Type such as multipart/form-data;boundary=----geckoformboundary4a7a...;charset=UTF-8 (Firefox boundary followed by the charset parameter that Spring adds) therefore counted two boundaries, "boundary=" and the "boundary" inside the value combined with the '=' of "charset=UTF-8", and the request was rejected with "Multiple boundary parameters in C-T". An occurrence is now only counted when it is a parameter name: at the start of the string or after ';' or whitespace, and followed by an optional run of blanks and '='. Genuine duplicates are still rejected. Fixes #3610 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0181jBv5JdNQviwDzfnkf1Gy --- src/request_body_processor/multipart.cc | 16 +- ...-body-parser-multipart-boundary-count.json | 188 ++++++++++++++++++ test/test-suite.in | 1 + 3 files changed, 202 insertions(+), 3 deletions(-) create mode 100644 test/test-cases/regression/request-body-parser-multipart-boundary-count.json diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc index 8cdbd12ff..6d7ca7bbc 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] == ';' || s[-1] == ' ' + || s[-1] == '\t'; s += 8; - - if (strchr(s, '=') != NULL) { + const char *p = s; + while (*p == ' ' || *p == '\t') { + 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..b894c4c44 --- /dev/null +++ b/test/test-cases/regression/request-body-parser-multipart-boundary-count.json @@ -0,0 +1,188 @@ +[ + { + "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\"" + ] + } +] \ 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 From 6dde7958eb97dffd526bfba796a05f2f7bd2b9e7 Mon Sep 17 00:00:00 2001 From: Felipe Zipitria Date: Mon, 12 Oct 2026 23:08:17 -0300 Subject: [PATCH 2/3] fix: count boundary parameters with the same whitespace rules as the parser Multipart::init accepts any isspace() character between the parameter name and the '=', so the counter has to as well; otherwise "boundary=0000; boundary=1111" counted as one parameter. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0181jBv5JdNQviwDzfnkf1Gy --- src/request_body_processor/multipart.cc | 6 +-- ...-body-parser-multipart-boundary-count.json | 47 +++++++++++++++++++ test/test-cases/secrules-language-tests | 2 +- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc index 6d7ca7bbc..d14108a26 100644 --- a/src/request_body_processor/multipart.cc +++ b/src/request_body_processor/multipart.cc @@ -1353,11 +1353,11 @@ int Multipart::count_boundary_params(const std::string& str_header_value) { * 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] == ';' || s[-1] == ' ' - || s[-1] == '\t'; + bool param_start = (s == start) || s[-1] == ';' || isspace(s[-1]); s += 8; const char *p = s; - while (*p == ' ' || *p == '\t') { + /* Same whitespace as Multipart::init accepts before the '='. */ + while (isspace(*p)) { p++; } if (param_start && *p == '=') { 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 index b894c4c44..b42b01f32 100644 --- a/test/test-cases/regression/request-body-parser-multipart-boundary-count.json +++ b/test/test-cases/regression/request-body-parser-multipart-boundary-count.json @@ -184,5 +184,52 @@ "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-cases/secrules-language-tests b/test/test-cases/secrules-language-tests index f73c73027..a3d4405e5 160000 --- a/test/test-cases/secrules-language-tests +++ b/test/test-cases/secrules-language-tests @@ -1 +1 @@ -Subproject commit f73c73027ef49ccf99c7911744929a71d15ff419 +Subproject commit a3d4405e5a2c90488c387e589c5534974575e35b From 3283c38a2f187b3244230e5e3bbfa2351b0d6e00 Mon Sep 17 00:00:00 2001 From: Felipe Zipitria Date: Wed, 14 Oct 2026 21:53:57 -0300 Subject: [PATCH 3/3] test: restore the secrules-language-tests submodule pointer The previous commit accidentally bumped the shared test-suite submodule to a local checkout's revision, which made nine removeComments cases fail in CI. Point it back at the revision v3/master uses. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Tz1sULBvqAwnRreKh5eC9h --- test/test-cases/secrules-language-tests | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-cases/secrules-language-tests b/test/test-cases/secrules-language-tests index a3d4405e5..f73c73027 160000 --- a/test/test-cases/secrules-language-tests +++ b/test/test-cases/secrules-language-tests @@ -1 +1 @@ -Subproject commit a3d4405e5a2c90488c387e589c5534974575e35b +Subproject commit f73c73027ef49ccf99c7911744929a71d15ff419