Skip to content

Commit 408692d

Browse files
Jan-Eremicollet
Jan-E
authored and
remicollet
committed
Add proc_open escaping for cmd file execution Backport CVE-2024-1874: Command injection via array-ish $command parameter of proc_open See remicollet/php-src-security#14 
1 parent 027bdbc commit 408692d

File tree

4 files changed

+170
-5
lines changed

4 files changed

+170
-5
lines changed

‎ext/standard/proc_open.c

Lines changed: 83 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -420,10 +420,33 @@ static void append_backslashes(smart_string *str, size_t num_bs) {
420420
}
421421
}
422422

423-
/* See https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments */
424-
static void append_win_escaped_arg(smart_string *str, char *arg) {
423+
/* See https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments and
424+
* https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way */
425+
const char *special_chars = "()!^\"<>&|%";
426+
427+
static BOOL is_special_character_present(const char *arg)
428+
{
429+
for (size_t i = 0; i < strlen(arg); ++i) {
430+
if (strchr(special_chars, arg[i]) != NULL) {
431+
return 1;
432+
}
433+
}
434+
return 0;
435+
}
436+
437+
static void append_win_escaped_arg(smart_string *str, char *arg, BOOL is_cmd_argument)
438+
{
425439
char c;
426440
size_t num_bs = 0;
441+
BOOL has_special_character = 0;
442+
443+
if (is_cmd_argument) {
444+
has_special_character = is_special_character_present(arg);
445+
if (has_special_character) {
446+
/* Escape double quote with ^ if executed by cmd.exe. */
447+
smart_string_appendc(str, '^');
448+
}
449+
}
427450
smart_string_appendc(str, '"');
428451
while ((c = *arg)) {
429452
if (c == '\\') {
@@ -434,19 +457,72 @@ static void append_win_escaped_arg(smart_string *str, char *arg) {
434457
num_bs = num_bs * 2 + 1;
435458
}
436459
append_backslashes(str, num_bs);
460+
if (has_special_character && strchr(special_chars, c) != NULL) {
461+
/* Escape special chars with ^ if executed by cmd.exe. */
462+
smart_string_appendc(str, '^');
463+
}
437464
smart_string_appendc(str, c);
438465
num_bs = 0;
439466
}
440467
arg++;
441468
}
442469
append_backslashes(str, num_bs * 2);
470+
if (has_special_character) {
471+
/* Escape double quote with ^ if executed by cmd.exe. */
472+
smart_string_appendc(str, '^');
473+
}
443474
smart_string_appendc(str, '"');
444475
}
445476

477+
static BOOL stricmp_end(const char* suffix, const char* str) {
478+
size_t suffix_len = strlen(suffix);
479+
size_t str_len = strlen(str);
480+
481+
if (suffix_len > str_len) {
482+
return -1; /* Suffix is longer than string, cannot match. */
483+
}
484+
485+
/* Compare the end of the string with the suffix, ignoring case. */
486+
return _stricmp(str + (str_len - suffix_len), suffix);
487+
}
488+
489+
static BOOL is_executed_by_cmd(const char *prog_name)
490+
{
491+
/* If program name is cmd.exe, then return true. */
492+
if (_stricmp("cmd.exe", prog_name) == 0 || _stricmp("cmd", prog_name) == 0
493+
|| stricmp_end("\\cmd.exe", prog_name) == 0 || stricmp_end("\\cmd", prog_name) == 0) {
494+
return 1;
495+
}
496+
497+
/* Find the last occurrence of the directory separator (backslash or forward slash). */
498+
char *last_separator = strrchr(prog_name, '\\');
499+
char *last_separator_fwd = strrchr(prog_name, '/');
500+
if (last_separator_fwd && (!last_separator || last_separator < last_separator_fwd)) {
501+
last_separator = last_separator_fwd;
502+
}
503+
504+
/* Find the last dot in the filename after the last directory separator. */
505+
char *extension = NULL;
506+
if (last_separator != NULL) {
507+
extension = strrchr(last_separator, '.');
508+
} else {
509+
extension = strrchr(prog_name, '.');
510+
}
511+
512+
if (extension == NULL || extension == prog_name) {
513+
/* No file extension found, it is not batch file. */
514+
return 0;
515+
}
516+
517+
/* Check if the file extension is ".bat" or ".cmd" which is always executed by cmd.exe. */
518+
return (_stricmp(extension, ".bat") == 0 || _stricmp(extension, ".cmd") == 0) ? 1 : 0;
519+
}
520+
446521
static char *create_win_command_from_args(HashTable *args) {
447522
smart_string str = {0};
448523
zval *arg_zv;
449-
zend_bool is_prog_name = 1;
524+
BOOL is_prog_name = 1;
525+
BOOL is_cmd_execution = 0;
450526
int elem_num = 0;
451527

452528
ZEND_HASH_FOREACH_VAL(args, arg_zv) {
@@ -456,11 +532,13 @@ static char *create_win_command_from_args(HashTable *args) {
456532
return NULL;
457533
}
458534

459-
if (!is_prog_name) {
535+
if (is_prog_name) {
536+
is_cmd_execution = is_executed_by_cmd(ZSTR_VAL(arg_str));
537+
} else {
460538
smart_string_appendc(&str, ' ');
461539
}
462540

463-
append_win_escaped_arg(&str, ZSTR_VAL(arg_str));
541+
append_win_escaped_arg(&str, ZSTR_VAL(arg_str), !is_prog_name && is_cmd_execution);
464542

465543
is_prog_name = 0;
466544
zend_string_release(arg_str);

‎ext/standard/tests/general_functions/ghsa-pc52-254m-w9w7_1.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GHSA-54hq-v5wp-fqgv - proc_open does not correctly escape args for bat files
3+
--SKIPIF--
4+
<?php
5+
if( substr(PHP_OS, 0, 3) != "WIN" )
6+
die('skip Run only on Windows');
7+
?>
8+
--FILE--
9+
<?php
10+
11+
$batch_file_content = <<<EOT
12+
@echo off
13+
powershell -Command "Write-Output '%1%'"
14+
EOT;
15+
$batch_file_path = __DIR__ . '/ghsa-54hq-v5wp-fqgv.bat';
16+
17+
file_put_contents($batch_file_path, $batch_file_content);
18+
19+
$descriptorspec = [STDIN, STDOUT, STDOUT];
20+
$proc = proc_open([$batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
21+
proc_close($proc);
22+
23+
?>
24+
--EXPECT--
25+
"&notepad.exe
26+
--CLEAN--
27+
<?php
28+
@unlink(__DIR__ . '/ghsa-54hq-v5wp-fqgv.bat');
29+
?>

‎ext/standard/tests/general_functions/ghsa-pc52-254m-w9w7_2.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GHSA-54hq-v5wp-fqgv - proc_open does not correctly escape args for cmd files
3+
--SKIPIF--
4+
<?php
5+
if( substr(PHP_OS, 0, 3) != "WIN" )
6+
die('skip Run only on Windows');
7+
?>
8+
--FILE--
9+
<?php
10+
11+
$batch_file_content = <<<EOT
12+
@echo off
13+
powershell -Command "Write-Output '%1%'"
14+
EOT;
15+
$batch_file_path = __DIR__ . '/ghsa-54hq-v5wp-fqgv.cmd';
16+
17+
file_put_contents($batch_file_path, $batch_file_content);
18+
19+
$descriptorspec = [STDIN, STDOUT, STDOUT];
20+
$proc = proc_open([$batch_file_path, "\"&notepad<>^()!.exe"], $descriptorspec, $pipes);
21+
proc_close($proc);
22+
23+
?>
24+
--EXPECT--
25+
"&notepad<>^()!.exe
26+
--CLEAN--
27+
<?php
28+
@unlink(__DIR__ . '/ghsa-54hq-v5wp-fqgv.cmd');
29+
?>

‎ext/standard/tests/general_functions/ghsa-pc52-254m-w9w7_3.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GHSA-54hq-v5wp-fqgv - proc_open does not correctly escape args for cmd executing batch files
3+
--SKIPIF--
4+
<?php
5+
if( substr(PHP_OS, 0, 3) != "WIN" )
6+
die('skip Run only on Windows');
7+
?>
8+
--FILE--
9+
<?php
10+
11+
$batch_file_content = <<<EOT
12+
@echo off
13+
powershell -Command "Write-Output '%1%'"
14+
EOT;
15+
$batch_file_path = __DIR__ . '/ghsa-54hq-v5wp-fqgv.bat';
16+
17+
file_put_contents($batch_file_path, $batch_file_content);
18+
19+
$descriptorspec = [STDIN, STDOUT, STDOUT];
20+
$proc = proc_open(["cmd.exe", "/c", $batch_file_path, "\"&notepad.exe"], $descriptorspec, $pipes);
21+
proc_close($proc);
22+
23+
?>
24+
--EXPECT--
25+
"&notepad.exe
26+
--CLEAN--
27+
<?php
28+
@unlink(__DIR__ . '/ghsa-54hq-v5wp-fqgv.bat');
29+
?>

0 commit comments

Comments
 (0)