Skip to content

Commit e6a82ad

Browse files
nielsdosericmann
nielsdos
authored and
ericmann
committed
Fix GHSA-9fcc-425m-g385: bypass CVE-2024-1874
The old code checked for suffixes but didn't take into account trailing whitespace. Furthermore, there is peculiar behaviour with trailing dots too. This all happens because of the special path-handling code inside CreateProcessW. By studying Wine's code, we can see that CreateProcessInternalW calls get_file_name [1] in our case because we haven't provided an application name. That code gets the first whitespace-delimited string into app_name excluding the quotes. It's then passed to create_process_params [2] where there is the path handling code that transforms the command line argument to an image path [3]. Inside Wine, the extension check if performed after these transformations [4]. By doing the same thing in PHP we match the behaviour and can properly match the extension even in the given edge cases. [1] https://github.com/wine-mirror/wine/blob/166895ae3ad3890ad946a309d0fd85e89ea3630e/dlls/kernelbase/process.c#L542-L543 [2] https://github.com/wine-mirror/wine/blob/166895ae3ad3890ad946a309d0fd85e89ea3630e/dlls/kernelbase/process.c#L565 [3] https://github.com/wine-mirror/wine/blob/166895ae3ad3890ad946a309d0fd85e89ea3630e/dlls/kernelbase/process.c#L150-L151 [4] https://github.com/wine-mirror/wine/blob/166895ae3ad3890ad946a309d0fd85e89ea3630e/dlls/kernelbase/process.c#L647-L654
1 parent b2c0db1 commit e6a82ad

File tree

4 files changed

+697
-34
lines changed

4 files changed

+697
-34
lines changed

‎ext/standard/proc_open.c

Lines changed: 25 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -592,48 +592,39 @@ static void append_win_escaped_arg(smart_str *str, zend_string *arg, bool is_cmd
592592
smart_str_appendc(str, '"');
593593
}
594594

595-
static inline int stricmp_end(const char* suffix, const char* str) {
596-
size_t suffix_len = strlen(suffix);
597-
size_t str_len = strlen(str);
595+
static bool is_executed_by_cmd(const char *prog_name, size_t prog_name_length)
596+
{
597+
size_t out_len;
598+
WCHAR long_name[MAX_PATH];
599+
WCHAR full_name[MAX_PATH];
600+
LPWSTR file_part = NULL;
598601

599-
if (suffix_len > str_len) {
600-
return -1; /* Suffix is longer than string, cannot match. */
601-
}
602+
wchar_t *prog_name_wide = php_win32_cp_conv_any_to_w(prog_name, prog_name_length, &out_len);
602603

603-
/* Compare the end of the string with the suffix, ignoring case. */
604-
return _stricmp(str + (str_len - suffix_len), suffix);
605-
}
604+
if (GetLongPathNameW(prog_name_wide, long_name, MAX_PATH) == 0) {
605+
/* This can fail for example with ERROR_FILE_NOT_FOUND (short path resolution only works for existing files)
606+
* in which case we'll pass the path verbatim to the FullPath transformation. */
607+
lstrcpynW(long_name, prog_name_wide, MAX_PATH);
608+
}
606609

607-
static bool is_executed_by_cmd(const char *prog_name)
608-
{
609-
/* If program name is cmd.exe, then return true. */
610-
if (_stricmp("cmd.exe", prog_name) == 0 || _stricmp("cmd", prog_name) == 0
611-
|| stricmp_end("\\cmd.exe", prog_name) == 0 || stricmp_end("\\cmd", prog_name) == 0) {
612-
return true;
613-
}
610+
free(prog_name_wide);
611+
prog_name_wide = NULL;
614612

615-
/* Find the last occurrence of the directory separator (backslash or forward slash). */
616-
char *last_separator = strrchr(prog_name, '\\');
617-
char *last_separator_fwd = strrchr(prog_name, '/');
618-
if (last_separator_fwd && (!last_separator || last_separator < last_separator_fwd)) {
619-
last_separator = last_separator_fwd;
613+
if (GetFullPathNameW(long_name, MAX_PATH, full_name, &file_part) == 0 || file_part == NULL) {
614+
return false;
620615
}
621616

622-
/* Find the last dot in the filename after the last directory separator. */
623-
char *extension = NULL;
624-
if (last_separator != NULL) {
625-
extension = strrchr(last_separator, '.');
617+
bool uses_cmd = false;
618+
if (_wcsicmp(file_part, L"cmd.exe") == 0 || _wcsicmp(file_part, L"cmd") == 0) {
619+
uses_cmd = true;
626620
} else {
627-
extension = strrchr(prog_name, '.');
628-
}
629-
630-
if (extension == NULL || extension == prog_name) {
631-
/* No file extension found, it is not batch file. */
632-
return false;
621+
const WCHAR *extension_dot = wcsrchr(file_part, L'.');
622+
if (extension_dot && (_wcsicmp(extension_dot, L".bat") == 0 || _wcsicmp(extension_dot, L".cmd") == 0)) {
623+
uses_cmd = true;
624+
}
633625
}
634626

635-
/* Check if the file extension is ".bat" or ".cmd" which is always executed by cmd.exe. */
636-
return _stricmp(extension, ".bat") == 0 || _stricmp(extension, ".cmd") == 0;
627+
return uses_cmd;
637628
}
638629

639630
static zend_string *create_win_command_from_args(HashTable *args)
@@ -652,7 +643,7 @@ static zend_string *create_win_command_from_args(HashTable *args)
652643
}
653644

654645
if (is_prog_name) {
655-
is_cmd_execution = is_executed_by_cmd(ZSTR_VAL(arg_str));
646+
is_cmd_execution = is_executed_by_cmd(ZSTR_VAL(arg_str), ZSTR_LEN(arg_str));
656647
} else {
657648
smart_str_appendc(&str, ' ');
658649
}

‎ext/standard/tests/general_functions/ghsa-9fcc-425m-g385_001.phpt

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
--TEST--
2+
GHSA-9fcc-425m-g385 - bypass CVE-2024-1874 - batch file variation
3+
--SKIPIF--
4+
<?php
5+
if( substr(PHP_OS, 0, 3) != "WIN" )
6+
die('skip Run only on Windows');
7+
if (getenv("SKIP_SLOW_TESTS")) die("skip slow test");
8+
?>
9+
--FILE--
10+
<?php
11+
12+
$batch_file_content = <<<EOT
13+
@echo off
14+
powershell -Command "Write-Output '%0%'"
15+
powershell -Command "Write-Output '%1%'"
16+
EOT;
17+
$batch_file_path = __DIR__ . '/ghsa-9fcc-425m-g385_001.bat';
18+
19+
file_put_contents($batch_file_path, $batch_file_content);
20+
21+
$descriptorspec = [STDIN, STDOUT, STDOUT];
22+
23+
$proc = proc_open([$batch_file_path . ".", "\"&notepad.exe"], $descriptorspec, $pipes);
24+
proc_close($proc);
25+
$proc = proc_open([$batch_file_path . " ", "\"&notepad.exe"], $descriptorspec, $pipes);
26+
proc_close($proc);
27+
$proc = proc_open([$batch_file_path . ". ", "\"&notepad.exe"], $descriptorspec, $pipes);
28+
proc_close($proc);
29+
$proc = proc_open([$batch_file_path . ". ... ", "\"&notepad.exe"], $descriptorspec, $pipes);
30+
proc_close($proc);
31+
$proc = proc_open([$batch_file_path . ". ... . ", "\"&notepad.exe"], $descriptorspec, $pipes);
32+
proc_close($proc);
33+
$proc = proc_open([$batch_file_path . ". ... . .", "\"&notepad.exe"], $descriptorspec, $pipes);
34+
proc_close($proc);
35+
proc_open([$batch_file_path . ". .\\.. . .", "\"&notepad.exe"], $descriptorspec, $pipes);
36+
37+
?>
38+
--EXPECTF--
39+
'"%sghsa-9fcc-425m-g385_001.bat."' is not recognized as an internal or external command,
40+
operable program or batch file.
41+
%sghsa-9fcc-425m-g385_001.bat
42+
"&notepad.exe
43+
%sghsa-9fcc-425m-g385_001.bat.
44+
"&notepad.exe
45+
%sghsa-9fcc-425m-g385_001.bat. ...
46+
"&notepad.exe
47+
%sghsa-9fcc-425m-g385_001.bat. ... .
48+
"&notepad.exe
49+
'"%sghsa-9fcc-425m-g385_001.bat. ... . ."' is not recognized as an internal or external command,
50+
operable program or batch file.
51+
52+
Warning: proc_open(): CreateProcess failed, error code: 2 in %s on line %d
53+
--CLEAN--
54+
<?php
55+
@unlink(__DIR__ . '/ghsa-9fcc-425m-g385_001.bat');
56+
?>

‎ext/standard/tests/general_functions/ghsa-9fcc-425m-g385_002.phpt

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
--TEST--
2+
GHSA-9fcc-425m-g385 - bypass CVE-2024-1874 - cmd.exe variation
3+
--SKIPIF--
4+
<?php
5+
if( substr(PHP_OS, 0, 3) != "WIN" )
6+
die('skip Run only on Windows');
7+
if (getenv("SKIP_SLOW_TESTS")) die("skip slow test");
8+
?>
9+
--FILE--
10+
<?php
11+
12+
$batch_file_content = <<<EOT
13+
@echo off
14+
powershell -Command "Write-Output '%0%'"
15+
powershell -Command "Write-Output '%1%'"
16+
EOT;
17+
$batch_file_path = __DIR__ . '/ghsa-9fcc-425m-g385_002.bat';
18+
19+
file_put_contents($batch_file_path, $batch_file_content);
20+
21+
$descriptorspec = [STDIN, STDOUT, STDOUT];
22+
23+
$proc = proc_open(["cmd.exe", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
24+
proc_close($proc);
25+
$proc = proc_open(["cmd.exe ", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
26+
proc_close($proc);
27+
$proc = proc_open(["cmd.exe. ", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
28+
proc_close($proc);
29+
$proc = proc_open(["cmd.exe. ... ", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
30+
proc_close($proc);
31+
$proc = proc_open(["\\cmd.exe. ... ", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
32+
33+
$proc = proc_open(["cmd", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
34+
proc_close($proc);
35+
$proc = proc_open(["cmd ", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
36+
proc_close($proc);
37+
$proc = proc_open(["cmd. ", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
38+
$proc = proc_open(["cmd. ... ", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
39+
$proc = proc_open(["\\cmd. ... ", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
40+
41+
?>
42+
--EXPECTF--
43+
%sghsa-9fcc-425m-g385_002.bat
44+
"&notepad.exe
45+
%sghsa-9fcc-425m-g385_002.bat
46+
"&notepad.exe
47+
%sghsa-9fcc-425m-g385_002.bat
48+
"&notepad.exe
49+
%sghsa-9fcc-425m-g385_002.bat
50+
"&notepad.exe
51+
52+
Warning: proc_open(): CreateProcess failed, error code: 2 in %s on line %d
53+
%sghsa-9fcc-425m-g385_002.bat
54+
"&notepad.exe
55+
%sghsa-9fcc-425m-g385_002.bat
56+
"&notepad.exe
57+
58+
Warning: proc_open(): CreateProcess failed, error code: 2 in %s on line %d
59+
60+
Warning: proc_open(): CreateProcess failed, error code: 2 in %s on line %d
61+
62+
Warning: proc_open(): CreateProcess failed, error code: 2 in %s on line %d
63+
--CLEAN--
64+
<?php
65+
@unlink(__DIR__ . '/ghsa-9fcc-425m-g385_002.bat');
66+
?>

0 commit comments

Comments
 (0)