From c9e67e9debe6ed0b313ebc6769a3ca0e417cd781 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Mon, 9 Sep 2024 15:22:07 +0200 Subject: [PATCH 3/8] Fix GHSA-9pqp-7h25-4f32 multipart/form-data boundaries larger than the read buffer result in erroneous parsing, which violates data integrity. Limit boundary size, as allowed by RFC 1521: Encapsulation boundaries [...] must be no longer than 70 characters, not counting the two leading hyphens. We correctly parse payloads with boundaries of length up to FILLUNIT-strlen("\r\n--") bytes, so allow this for BC. (cherry picked from commit 19b49258d0c5a61398d395d8afde1123e8d161e0) (cherry picked from commit 2b0daf421c162376892832588eccdfa9a286ed09) (cherry picked from commit a24ac172f52e75101913f3946cfa5515f723c99f) (cherry picked from commit 08f0adf0700f8bbaa4fd75b7a694bbd9ae45300d) (cherry picked from commit 5731a40507feea683591addf3599d210cd7a1fd9) --- main/rfc1867.c | 7 ++ tests/basic/GHSA-9pqp-7h25-4f32.inc | 3 + tests/basic/GHSA-9pqp-7h25-4f32.phpt | 100 +++++++++++++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 tests/basic/GHSA-9pqp-7h25-4f32.inc create mode 100644 tests/basic/GHSA-9pqp-7h25-4f32.phpt diff --git a/main/rfc1867.c b/main/rfc1867.c index 1eb81827f5..0f8799ca64 100644 --- a/main/rfc1867.c +++ b/main/rfc1867.c @@ -769,6 +769,13 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* {{{ */ boundary_len = boundary_end-boundary; } + /* Boundaries larger than FILLUNIT-strlen("\r\n--") characters lead to + * erroneous parsing */ + if (boundary_len > FILLUNIT-strlen("\r\n--")) { + sapi_module.sapi_error(E_WARNING, "Boundary too large in multipart/form-data POST data"); + return; + } + /* Initialize the buffer */ if (!(mbuff = multipart_buffer_new(boundary, boundary_len))) { sapi_module.sapi_error(E_WARNING, "Unable to initialize the input buffer"); diff --git a/tests/basic/GHSA-9pqp-7h25-4f32.inc b/tests/basic/GHSA-9pqp-7h25-4f32.inc new file mode 100644 index 0000000000..adf72a361a --- /dev/null +++ b/tests/basic/GHSA-9pqp-7h25-4f32.inc @@ -0,0 +1,3 @@ + +--FILE-- + '1', + 'CONTENT_TYPE' => "multipart/form-data; boundary=$boundary", + 'CONTENT_LENGTH' => strlen($body), + 'REQUEST_METHOD' => 'POST', + 'SCRIPT_FILENAME' => __DIR__ . '/GHSA-9pqp-7h25-4f32.inc', + ]); + + $spec = [ + 0 => ['pipe', 'r'], + 1 => STDOUT, + 2 => STDOUT, + ]; + + $pipes = []; + + print "Starting...\n"; + + $handle = proc_open($cmd, $spec, $pipes, getcwd(), $env); + + fwrite($pipes[0], $body); + + $status = proc_close($handle); + + print "\n"; +} + +for ($offset = -1; $offset <= 1; $offset++) { + test(FILLUNIT - strlen("\r\n--") + $offset); +} + +?> +--EXPECTF-- +Boundary len: 5115 +Starting... +X-Powered-By: %s +Content-type: text/html; charset=UTF-8 + +Hello world +array(1) { + ["koko"]=> + string(5124) "BBB +--AAA%sCCC" +} + +Boundary len: 5116 +Starting... +X-Powered-By: %s +Content-type: text/html; charset=UTF-8 + +Hello world +array(1) { + ["koko"]=> + string(5125) "BBB +--AAA%sCCC" +} + +Boundary len: 5117 +Starting... +X-Powered-By: %s +Content-type: text/html; charset=UTF-8 + +
+Warning: Boundary too large in multipart/form-data POST data in Unknown on line 0
+Hello world +array(0) { +} + -- 2.46.1 From 2d5ff57eb7a36f9f0655c7073c4c702a903d9005 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Mon, 23 Sep 2024 18:54:31 +0100 Subject: [PATCH 6/8] Skip GHSA-9pqp-7h25-4f32 test on Windows (cherry picked from commit c70e25630832fa10d421328eed2b8e1a36af7a64) (cherry picked from commit c75683864f6e4188439e8ca2adbb05824918be12) (cherry picked from commit 2fd1b83817d20523e72bef3ad524cd5797f51acf) (cherry picked from commit 79eace3a64544088738d2fd341407cc32fe3ecaf) (cherry picked from commit 0c9258e4914695ca21b3d0cd3b1746bfc926f02e) --- tests/basic/GHSA-9pqp-7h25-4f32.phpt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/basic/GHSA-9pqp-7h25-4f32.phpt b/tests/basic/GHSA-9pqp-7h25-4f32.phpt index af81916370..29bcb6557d 100644 --- a/tests/basic/GHSA-9pqp-7h25-4f32.phpt +++ b/tests/basic/GHSA-9pqp-7h25-4f32.phpt @@ -5,6 +5,9 @@ GHSA-9pqp-7h25-4f32 if (!getenv('TEST_PHP_CGI_EXECUTABLE')) { die("skip php-cgi not available"); } +if (substr(PHP_OS, 0, 3) == 'WIN') { + die("skip not for Windows in CI - probably resource issue"); +} ?> --FILE-- Date: Thu, 26 Sep 2024 15:49:03 +0200 Subject: [PATCH 8/8] adapt GHSA-9pqp-7h25-4f32 test for 7.x (cherry picked from commit 29065f33f37f99ba33254cb23c941647bcd7372c) (cherry picked from commit 87ed9429a17e38daec4dcfd7a3c3db194197ccb3) (cherry picked from commit d97de82afe8696b6d76cc11bc7b6d6c2652d06d9) --- tests/basic/GHSA-9pqp-7h25-4f32.phpt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/basic/GHSA-9pqp-7h25-4f32.phpt b/tests/basic/GHSA-9pqp-7h25-4f32.phpt index 29bcb6557d..b913edc1c4 100644 --- a/tests/basic/GHSA-9pqp-7h25-4f32.phpt +++ b/tests/basic/GHSA-9pqp-7h25-4f32.phpt @@ -21,8 +21,10 @@ function test($boundaryLen) { getenv('TEST_PHP_CGI_EXECUTABLE'), '-C', '-n', + '-dlog_errors=1', __DIR__ . '/GHSA-9pqp-7h25-4f32.inc', ]; + $cmd = implode(' ', $cmd); $boundary = str_repeat('A', $boundaryLen); $body = "" @@ -92,11 +94,10 @@ array(1) { Boundary len: 5117 Starting... +PHP Warning: Boundary too large in multipart/form-data POST data in Unknown on line 0 X-Powered-By: %s Content-type: text/html; charset=UTF-8 -
-Warning: Boundary too large in multipart/form-data POST data in Unknown on line 0
Hello world array(0) { } -- 2.46.1