From d65a1e6f9171e01ca81adc8d9dd1daf3125efc36 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Mon, 9 Sep 2024 15:22:07 +0200 Subject: [PATCH 1/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. --- 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 2ddad7950fd..83d141d38b1 100644 --- a/main/rfc1867.c +++ b/main/rfc1867.c @@ -751,6 +751,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 00000000000..adf72a361a2 --- /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) { +} + From 4b9cd27ff5c0177dcb160caeae1ea79e761ada58 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 14 Jun 2024 19:49:22 +0200 Subject: [PATCH 2/8] Fix GHSA-p99j-rfp4-xqvq It's no use trying to work around whatever the operating system and Apache do because we'll be fighting that until eternity. Change the skip_getopt condition such that when we're running in CGI or FastCGI mode we always skip the argument parsing. This is a BC break, but this seems to be the only way to get rid of this class of issues. --- sapi/cgi/cgi_main.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/sapi/cgi/cgi_main.c b/sapi/cgi/cgi_main.c index 2c1fa9332df..fba0563aa18 100644 --- a/sapi/cgi/cgi_main.c +++ b/sapi/cgi/cgi_main.c @@ -1748,7 +1748,6 @@ int main(int argc, char *argv[]) int status = 0; #endif char *query_string; - char *decoded_query_string; int skip_getopt = 0; #if defined(SIGPIPE) && defined(SIG_IGN) @@ -1803,10 +1802,15 @@ int main(int argc, char *argv[]) * the executable. Ideally we skip argument parsing when we're in cgi or fastcgi mode, * but that breaks PHP scripts on Linux with a hashbang: `#!/php-cgi -d option=value`. * Therefore, this code only prevents passing arguments if the query string starts with a '-'. - * Similarly, scripts spawned in subprocesses on Windows may have the same issue. */ + * Similarly, scripts spawned in subprocesses on Windows may have the same issue. + * However, Windows has lots of conversion rules and command line parsing rules that + * are too difficult and dangerous to reliably emulate. */ if((query_string = getenv("QUERY_STRING")) != NULL && strchr(query_string, '=') == NULL) { +#ifdef PHP_WIN32 + skip_getopt = cgi || fastcgi; +#else unsigned char *p; - decoded_query_string = strdup(query_string); + char *decoded_query_string = strdup(query_string); php_url_decode(decoded_query_string, strlen(decoded_query_string)); for (p = (unsigned char *)decoded_query_string; *p && *p <= ' '; p++) { /* skip all leading spaces */ @@ -1815,22 +1819,8 @@ int main(int argc, char *argv[]) skip_getopt = 1; } - /* On Windows we have to take into account the "best fit" mapping behaviour. */ -#ifdef PHP_WIN32 - if (*p >= 0x80) { - wchar_t wide_buf[1]; - wide_buf[0] = *p; - char char_buf[4]; - size_t wide_buf_len = sizeof(wide_buf) / sizeof(wide_buf[0]); - size_t char_buf_len = sizeof(char_buf) / sizeof(char_buf[0]); - if (WideCharToMultiByte(CP_ACP, 0, wide_buf, wide_buf_len, char_buf, char_buf_len, NULL, NULL) == 0 - || char_buf[0] == '-') { - skip_getopt = 1; - } - } -#endif - free(decoded_query_string); +#endif } while (!skip_getopt && (c = php_getopt(argc, argv, OPTIONS, &php_optarg, &php_optind, 0, 2)) != -1) { From c1c14c8a0f1067baac1b22474df19266afe21a4d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 18 Jun 2024 21:28:26 +0200 Subject: [PATCH 3/8] Fix GHSA-94p6-54jq-9mwp Apache only generates REDIRECT_STATUS, so explicitly check for that if the server name is Apache, don't allow other variable names. Furthermore, redirect.so and Netscape no longer exist, so remove those entries as we can't check their server name anymore. We now also check for the configuration override *first* such that it always take precedence. This would allow for a mitigation path if something like this happens in the future. --- sapi/cgi/cgi_main.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/sapi/cgi/cgi_main.c b/sapi/cgi/cgi_main.c index fba0563aa18..c7bb7ccabf9 100644 --- a/sapi/cgi/cgi_main.c +++ b/sapi/cgi/cgi_main.c @@ -1910,18 +1910,17 @@ int main(int argc, char *argv[]) /* check force_cgi after startup, so we have proper output */ if (cgi && CGIG(force_redirect)) { - /* Apache will generate REDIRECT_STATUS, - * Netscape and redirect.so will generate HTTP_REDIRECT_STATUS. - * redirect.so and installation instructions available from - * http://www.koehntopp.de/php. - * -- kk@netuse.de - */ - if (!getenv("REDIRECT_STATUS") && - !getenv ("HTTP_REDIRECT_STATUS") && - /* this is to allow a different env var to be configured - * in case some server does something different than above */ - (!CGIG(redirect_status_env) || !getenv(CGIG(redirect_status_env))) - ) { + /* This is to allow a different environment variable to be configured + * in case the we cannot auto-detect which environment variable to use. + * Checking this first to allow user overrides in case the environment + * variable can be set by an untrusted party. */ + const char *redirect_status_env = CGIG(redirect_status_env); + if (!redirect_status_env) { + /* Apache will generate REDIRECT_STATUS. */ + redirect_status_env = "REDIRECT_STATUS"; + } + + if (!getenv(redirect_status_env)) { zend_try { SG(sapi_headers).http_response_code = 400; PUTS("Security Alert! The PHP CGI cannot be accessed directly.\n\n\ From 4580b8b3e1495df45a894ab928b23067159eedb8 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 12 Sep 2024 13:11:11 +0100 Subject: [PATCH 4/8] Fix GHSA-865w-9rf3-2wh5: FPM: Logs from childrens may be altered --- sapi/fpm/fpm/fpm_stdio.c | 2 +- .../log-bwp-msg-flush-split-sep-pos-end.phpt | 47 +++++++++++++++++++ ...log-bwp-msg-flush-split-sep-pos-start.phpt | 47 +++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 sapi/fpm/tests/log-bwp-msg-flush-split-sep-pos-end.phpt create mode 100644 sapi/fpm/tests/log-bwp-msg-flush-split-sep-pos-start.phpt diff --git a/sapi/fpm/fpm/fpm_stdio.c b/sapi/fpm/fpm/fpm_stdio.c index 8f71e8cbfcd..dec540d17ac 100644 --- a/sapi/fpm/fpm/fpm_stdio.c +++ b/sapi/fpm/fpm/fpm_stdio.c @@ -229,7 +229,7 @@ stdio_read: if ((sizeof(FPM_STDIO_CMD_FLUSH) - cmd_pos) <= in_buf && !memcmp(buf, &FPM_STDIO_CMD_FLUSH[cmd_pos], sizeof(FPM_STDIO_CMD_FLUSH) - cmd_pos)) { zlog_stream_finish(log_stream); - start = cmd_pos; + start = sizeof(FPM_STDIO_CMD_FLUSH) - cmd_pos; } else { zlog_stream_str(log_stream, &FPM_STDIO_CMD_FLUSH[0], cmd_pos); } diff --git a/sapi/fpm/tests/log-bwp-msg-flush-split-sep-pos-end.phpt b/sapi/fpm/tests/log-bwp-msg-flush-split-sep-pos-end.phpt new file mode 100644 index 00000000000..52826320080 --- /dev/null +++ b/sapi/fpm/tests/log-bwp-msg-flush-split-sep-pos-end.phpt @@ -0,0 +1,47 @@ +--TEST-- +FPM: Buffered worker output plain log with msg with flush split position towards separator end +--SKIPIF-- + +--FILE-- +start(); +$tester->expectLogStartNotices(); +$tester->request()->expectEmptyBody(); +$tester->expectLogLine(str_repeat('a', 1013) . "Quarkslab", decorated: false); +$tester->expectLogLine("Quarkslab", decorated: false); +$tester->terminate(); +$tester->expectLogTerminatingNotices(); +$tester->close(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- + diff --git a/sapi/fpm/tests/log-bwp-msg-flush-split-sep-pos-start.phpt b/sapi/fpm/tests/log-bwp-msg-flush-split-sep-pos-start.phpt new file mode 100644 index 00000000000..34905938553 --- /dev/null +++ b/sapi/fpm/tests/log-bwp-msg-flush-split-sep-pos-start.phpt @@ -0,0 +1,47 @@ +--TEST-- +FPM: Buffered worker output plain log with msg with flush split position towards separator start +--SKIPIF-- + +--FILE-- +start(); +$tester->expectLogStartNotices(); +$tester->request()->expectEmptyBody(); +$tester->expectLogLine(str_repeat('a', 1009) . "Quarkslab", decorated: false); +$tester->expectLogLine("Quarkslab", decorated: false); +$tester->terminate(); +$tester->expectLogTerminatingNotices(); +$tester->close(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- + From 8d87bc3e266d4848f74dd4b2b7ee5fe4d666bab7 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Mon, 23 Sep 2024 12:09:57 +0100 Subject: [PATCH 5/8] Update NEWS with security fixes info --- NEWS | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 3bc2c282ec0..1cd4bdd779d 100644 --- a/NEWS +++ b/NEWS @@ -1,8 +1,21 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| -?? ??? ????, PHP 8.1.30 +26 Sep 2024, PHP 8.1.30 +- CGI: + . Fixed bug GHSA-p99j-rfp4-xqvq (Bypass of CVE-2024-4577, Parameter Injection + Vulnerability). (CVE-2024-8926) (nielsdos) + . Fixed bug GHSA-94p6-54jq-9mwp (cgi.force_redirect configuration is + byppassible due to the environment variable collision). (CVE-2024-8927) + (nielsdos) +- FPM: + . Fixed bug GHSA-865w-9rf3-2wh5 (Logs from childrens may be altered). + (CVE-2024-9026) (Jakub Zelenka) + +- SAPI: + . Fixed bug GHSA-9pqp-7h25-4f32 (Erroneous parsing of multipart form data). + (CVE-2024-8925) (Arnaud) 06 Jun 2024, PHP 8.1.29 From 4bcc7d5778e70858b4c54c07d21f40ffb09979e2 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 --- 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 af819163705..29bcb6557d5 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: Mon, 23 Sep 2024 20:50:51 +0100 Subject: [PATCH 7/8] [skip ci] Fix typo in NEWS Co-authored-by: Niels Dossche <7771979+nielsdos@users.noreply.github.com> --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 1cd4bdd779d..e3e7a5613be 100644 --- a/NEWS +++ b/NEWS @@ -6,7 +6,7 @@ PHP NEWS . Fixed bug GHSA-p99j-rfp4-xqvq (Bypass of CVE-2024-4577, Parameter Injection Vulnerability). (CVE-2024-8926) (nielsdos) . Fixed bug GHSA-94p6-54jq-9mwp (cgi.force_redirect configuration is - byppassible due to the environment variable collision). (CVE-2024-8927) + bypassable due to the environment variable collision). (CVE-2024-8927) (nielsdos) - FPM: From fcbcf2f2810e3e72abd3b36a6401e3e9268d52ce Mon Sep 17 00:00:00 2001 From: Ben Ramsey Date: Thu, 26 Sep 2024 12:52:41 -0500 Subject: [PATCH 8/8] PHP-8.1 is now for PHP 8.1.31-dev --- NEWS | 4 ++++ Zend/zend.h | 2 +- configure.ac | 2 +- main/php_version.h | 6 +++--- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index e3e7a5613be..b9130ae18db 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| +?? ??? ????, PHP 8.1.31 + + + 26 Sep 2024, PHP 8.1.30 - CGI: diff --git a/Zend/zend.h b/Zend/zend.h index 56ce93f685d..dcf69979f95 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -20,7 +20,7 @@ #ifndef ZEND_H #define ZEND_H -#define ZEND_VERSION "4.1.30-dev" +#define ZEND_VERSION "4.1.31-dev" #define ZEND_ENGINE_3 diff --git a/configure.ac b/configure.ac index 2c21f30ff26..f6902707abb 100644 --- a/configure.ac +++ b/configure.ac @@ -17,7 +17,7 @@ dnl Basic autoconf initialization, generation of config.nice. dnl ---------------------------------------------------------------------------- AC_PREREQ([2.68]) -AC_INIT([PHP],[8.1.30-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) +AC_INIT([PHP],[8.1.31-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) AC_CONFIG_SRCDIR([main/php_version.h]) AC_CONFIG_AUX_DIR([build]) AC_PRESERVE_HELP_ORDER diff --git a/main/php_version.h b/main/php_version.h index 3327cdc332e..bff67225834 100644 --- a/main/php_version.h +++ b/main/php_version.h @@ -2,7 +2,7 @@ /* edit configure.ac to change version number */ #define PHP_MAJOR_VERSION 8 #define PHP_MINOR_VERSION 1 -#define PHP_RELEASE_VERSION 30 +#define PHP_RELEASE_VERSION 31 #define PHP_EXTRA_VERSION "-dev" -#define PHP_VERSION "8.1.30-dev" -#define PHP_VERSION_ID 80130 +#define PHP_VERSION "8.1.31-dev" +#define PHP_VERSION_ID 80131