Merge branch 'PHP-8.3' into PHP-8.4

* PHP-8.3:
  Harden proc_open() against cmd.exe hijacking

Author: cmb69

NEWS

@@ -20,6 +20,9 @@ PHP                                                                        NEWS
   . Fixed bug GH-17037 (UAF in user filter when adding existing filter name due
     to incorrect error handling). (nielsdos)
 
+- Windows:
+  . Hardened proc_open() against cmd.exe hijacking. (cmb)
+
 05 Dec 2024, PHP 8.4.2
 
 - BcMath:

ext/standard/Makefile.frag.w32

@@ -7,3 +7,7 @@ ext\standard\url_scanner_ex.c: ext\standard\url_scanner_ex.re
 	$(RE2C) $(RE2C_FLAGS) -b -o ext/standard/url_scanner_ex.c ext/standard/url_scanner_ex.re
 
 $(BUILD_DIR)\ext\standard\basic_functions.obj: $(PHP_SRC_DIR)\Zend\zend_language_parser.h
+
+$(PHP_SRC_DIR)\ext\standard\tests\helpers\bad_cmd.exe: $(PHP_SRC_DIR)\ext\standard\tests\helpers\bad_cmd.c
+	cd $(PHP_SRC_DIR)\ext\standard\tests\helpers
+	$(PHP_CL) /nologo bad_cmd.c

ext/standard/proc_open.c

@@ -698,22 +698,77 @@ static void init_process_info(PROCESS_INFORMATION *pi)
 	memset(&pi, 0, sizeof(pi));
 }
 
+/* on success, returns length of *comspec, which then needs to be efree'd by caller */
+static size_t find_comspec_nt(wchar_t **comspec)
+{
+	zend_string *path = NULL;
+	wchar_t *pathw = NULL;
+	wchar_t *bufp = NULL;
+	DWORD buflen = MAX_PATH, len = 0;
+
+	path = php_getenv("PATH", 4);
+	if (path == NULL) {
+		goto out;
+	}
+	pathw = php_win32_cp_any_to_w(ZSTR_VAL(path));
+	if (pathw == NULL) {
+		goto out;
+	}
+	bufp = emalloc(buflen * sizeof(wchar_t));
+	do {
+		/* the first call to SearchPathW() fails if the buffer is too small,
+		 * what is unlikely but possible; to avoid an explicit second call to
+		 * SeachPathW() and the error handling, we're looping */
+		len = SearchPathW(pathw, L"cmd.exe", NULL, buflen, bufp, NULL);
+		if (len == 0) {
+			goto out;
+		}
+		if (len < buflen) {
+			break;
+		}
+		buflen = len;
+		bufp = erealloc(bufp, buflen * sizeof(wchar_t));
+	} while (1);
+	*comspec = bufp;
+
+out:
+	if (path != NULL) {
+		zend_string_release(path);
+	}
+	if (pathw != NULL) {
+		free(pathw);
+	}
+	if (bufp != NULL && bufp != *comspec) {
+		efree(bufp);
+	}
+	return len;
+}
+
 static zend_result convert_command_to_use_shell(wchar_t **cmdw, size_t cmdw_len)
 {
-	size_t len = sizeof(COMSPEC_NT) + sizeof(" /s /c ") + cmdw_len + 3;
+	wchar_t *comspec;
+	size_t len = find_comspec_nt(&comspec);
+	if (len == 0) {
+		php_error_docref(NULL, E_WARNING, "Command conversion failed");
+		return FAILURE;
+	}
+	len += sizeof(" /s /c ") + cmdw_len + 3;
 	wchar_t *cmdw_shell = (wchar_t *)malloc(len * sizeof(wchar_t));
 
 	if (cmdw_shell == NULL) {
+		efree(comspec);
 		php_error_docref(NULL, E_WARNING, "Command conversion failed");
 		return FAILURE;
 	}
 
-	if (_snwprintf(cmdw_shell, len, L"%hs /s /c \"%s\"", COMSPEC_NT, *cmdw) == -1) {
+	if (_snwprintf(cmdw_shell, len, L"%s /s /c \"%s\"", comspec, *cmdw) == -1) {
+		efree(comspec);
 		free(cmdw_shell);
 		php_error_docref(NULL, E_WARNING, "Command conversion failed");
 		return FAILURE;
 	}
 
+	efree(comspec);
 	free(*cmdw);
 	*cmdw = cmdw_shell;
 

ext/standard/tests/general_functions/proc_open_cmd.phpt

@@ -0,0 +1,28 @@
+--TEST--
+Harden against cmd.exe hijacking
+--SKIPIF--
+<?php
+if (PHP_OS_FAMILY !== "Windows") die("skip only for Windows");
+?>
+--FILE--
+<?php
+copy(__DIR__ . "/../helpers/bad_cmd.exe", "cmd.exe");
+$spec = [["pipe", "r"], ["pipe", "w"], ["pipe", "w"]];
+var_dump($proc = proc_open("@echo hello", $spec, $pipes, null));
+$read = [$pipes[1], $pipes[2]];
+$write = $except = null;
+if (($num = stream_select($read, $write, $except, 1000)) === false) {
+    echo "stream_select() failed\n";
+} elseif ($num > 0) {
+    foreach ($read as $stream) {
+        fpassthru($stream);
+    }
+}
+?>
+--EXPECTF--
+resource(%d) of type (process)
+hello
+--CLEAN--
+<?php
+@unlink("cmd.exe");
+?>

ext/standard/tests/helpers/bad_cmd.c

@@ -0,0 +1,7 @@
+#include <stdio.h>
+
+int main()
+{
+	printf("pwnd!\n");
+	return 0;
+}

win32/build/Makefile

@@ -54,7 +54,7 @@ DEBUGGER_CMD=
 DEBUGGER_ARGS=
 !endif
 
-all: generated_files $(EXT_TARGETS) $(PECL_TARGETS) $(SAPI_TARGETS)
+all: generated_files $(EXT_TARGETS) $(PECL_TARGETS) $(SAPI_TARGETS) test_helpers
 
 build_dirs: $(BUILD_DIR) $(BUILD_DIRS_SUB) $(BUILD_DIR_DEV)
 
@@ -184,6 +184,7 @@ clean-pgo: clean-all
 	-del /f /q $(BUILD_DIR)\$(DIST_ZIP_PECL)
 	-del /f /q $(BUILD_DIR)\$(DIST_ZIP_TEST_PACK)
 
+test_helpers: $(PHP_SRC_DIR)\ext\standard\tests\helpers\bad_cmd.exe
 
 !if $(PHP_TEST_INI_PATH) == ""
 test: set-tmp-env