From 1dc4f3e134d4b5fe0776d46f1342c603fdff072f Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Mon, 9 Sep 2024 15:22:07 +0200 Subject: [PATCH 4/9] 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) (cherry picked from commit c9e67e9debe6ed0b313ebc6769a3ca0e417cd781) (cherry picked from commit cc487bc778a079891c67e927edf0fc1a46f0e5e3) --- 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 407cc3be14..1efd28298e 100644 --- a/main/rfc1867.c +++ b/main/rfc1867.c @@ -763,6 +763,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 TSRMLS_CC))) { 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 8747962af4a9a76d81b55b4455896da40cd523c6 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Mon, 23 Sep 2024 18:54:31 +0100 Subject: [PATCH 7/9] 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) (cherry picked from commit 2d5ff57eb7a36f9f0655c7073c4c702a903d9005) (cherry picked from commit d5931f3c995e5d1d92289f0acbbaef1678d911a6) --- 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 9/9] adapt GHSA-9pqp-7h25-4f32 test for 7.x (cherry picked from commit 29065f33f37f99ba33254cb23c941647bcd7372c) (cherry picked from commit 87ed9429a17e38daec4dcfd7a3c3db194197ccb3) (cherry picked from commit d97de82afe8696b6d76cc11bc7b6d6c2652d06d9) (cherry picked from commit 64a9dfdec2cb530428c9cbe90f98f346c5d23797) (cherry picked from commit fb9688a470070f0ab656e2f94efbee2988a30eaf) --- 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