Merge branch 'PHP-8.3' into PHP-8.4

2024-11-23 01:44:06 +08:00 · 2024-09-26 14:24:13 -05:00 · 2024-09-26 14:24:13 -05:00 · d854a54b5f
commit d854a54b5f
parent a37a3d3d81 6d99ccc268
8 changed files with 242 additions and 31 deletions
--- a/15
+++ b/15
@ -2,6 +2,13 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.4.0RC2

+- 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
+    bypassable due to the environment variable collision). (CVE-2024-8927)
+    (nielsdos)
+
 - Core:
  . Fixed bug GH-16040 (Use-after-free of object released in hook). (ilutov)
  . Fixed bug GH-16054 (Segmentation fault when resizing hash table iterator
@ -12,6 +19,10 @@ PHP                                                                        NEWS
  . Fixed bug GH-16039 (Segmentation fault (access null pointer) in
    ext/dom/parentnode/tree.c). (nielsdos)

+- FPM:
+  . Fixed bug GHSA-865w-9rf3-2wh5 (Logs from childrens may be altered).
+    (CVE-2024-9026) (Jakub Zelenka)
+
 - LDAP:
  . Fixed bug GH-16032 (Various NULL pointer dereferencements in
    ldap_modify_batch()). (Girgias)
@ -20,6 +31,10 @@ PHP                                                                        NEWS
  . Fixed bug GH-16009 (Segmentation fault with frameless functions and
    undefined CVs). (nielsdos)

+- SAPI:
+  . Fixed bug GHSA-9pqp-7h25-4f32 (Erroneous parsing of multipart form data).
+    (CVE-2024-8925) (Arnaud)
+
 26 Sep 2024, PHP 8.4.0RC1

 - BcMath:
--- a/main/rfc1867.c
+++ b/main/rfc1867.c
@ -742,6 +742,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 */
 	mbuff = multipart_buffer_new(boundary, boundary_len);

--- a/sapi/cgi/cgi_main.c
+++ b/sapi/cgi/cgi_main.c
@ -1749,7 +1749,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)
@ -1804,10 +1803,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 */
@ -1816,22 +1820,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
 	}

 	php_ini_builder_init(&ini_builder);
@ -1898,18 +1888,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("<b>Security Alert!</b> The PHP CGI cannot be accessed directly.\n\n\
--- a/sapi/fpm/fpm/fpm_stdio.c
+++ b/sapi/fpm/fpm/fpm_stdio.c
@ -228,7 +228,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);
 			}
--- 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
@ -0,0 +1,47 @@
+--TEST--
+FPM: Buffered worker output plain log with msg with flush split position towards separator end
+--SKIPIF--
+<?php include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 1
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+catch_workers_output = yes
+decorate_workers_output = no
+EOT;
+
+$code = <<<EOT
+<?php
+file_put_contents('php://stderr', str_repeat('a', 1013) . "Quarkslab\0fscf\0Quarkslab");
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->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--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
--- 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
@ -0,0 +1,47 @@
+--TEST--
+FPM: Buffered worker output plain log with msg with flush split position towards separator start
+--SKIPIF--
+<?php include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 1
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+catch_workers_output = yes
+decorate_workers_output = no
+EOT;
+
+$code = <<<EOT
+<?php
+file_put_contents('php://stderr', str_repeat('a', 1009) . "Quarkslab\0fscf\0Quarkslab");
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->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--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
--- a/tests/basic/GHSA-9pqp-7h25-4f32.inc
+++ b/tests/basic/GHSA-9pqp-7h25-4f32.inc
@ -0,0 +1,3 @@
+<?php
+print "Hello world\n";
+var_dump($_POST);
--- a/tests/basic/GHSA-9pqp-7h25-4f32.phpt
+++ b/tests/basic/GHSA-9pqp-7h25-4f32.phpt
@ -0,0 +1,103 @@
+--TEST--
+GHSA-9pqp-7h25-4f32
+--SKIPIF--
+<?php
+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--
+<?php
+
+const FILLUNIT = 5 * 1024;
+
+function test($boundaryLen) {
+    printf("Boundary len: %d\n", $boundaryLen);
+
+    $cmd = [
+        getenv('TEST_PHP_CGI_EXECUTABLE'),
+        '-C',
+        '-n',
+        __DIR__ . '/GHSA-9pqp-7h25-4f32.inc',
+    ];
+
+    $boundary = str_repeat('A', $boundaryLen);
+    $body = ""
+        . "--$boundary\r\n"
+        . "Content-Disposition: form-data; name=\"koko\"\r\n"
+        . "\r\n"
+        . "BBB\r\n--" . substr($boundary, 0, -1) . "CCC\r\n"
+        . "--$boundary--\r\n"
+        ;
+
+    $env = array_merge($_ENV, [
+        'REDIRECT_STATUS' => '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
+
+<br />
+<b>Warning</b>:  Boundary too large in multipart/form-data POST data in <b>Unknown</b> on line <b>0</b><br />
+Hello world
+array(0) {
+}
+