Skip to content

Commit 3d9941f

Browse files
ramseyramsey
committed
Merge branch 'PHP-8.2' into PHP-8.3
2 parents f56ba62 + a9ffc44 commit 3d9941f

File tree

9 files changed

+276
-19
lines changed

9 files changed

+276
-19
lines changed

‎NEWS

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,12 @@ PHP NEWS
111111
(SakiTakamachi)
112112
. Fixed bug GH-13203 (file_put_contents fail on strings over 4GB on Windows).
113113
(divinity76)
114+
. Fixed bug GHSA-pc52-254m-w9w7 (Command injection via array-ish $command
115+
parameter of proc_open). (CVE-2024-1874) (Jakub Zelenka)
116+
. Fixed bug GHSA-wpj3-hf5j-x4v4 (__Host-/__Secure- cookie bypass due to
117+
partial CVE-2022-31629 fix). (CVE-2024-2756) (nielsdos)
118+
. Fixed bug GHSA-h746-cjrr-wfmr (password_verify can erroneously return true,
119+
opening ATO risk). (CVE-2024-3096) (Jakub Zelenka)
114120

115121
14 Mar 2024, PHP 8.3.4
116122

‎ext/standard/password.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,11 @@ static zend_string* php_password_bcrypt_hash(const zend_string *password, zend_a
180180
zval *zcost;
181181
zend_long cost = PHP_PASSWORD_BCRYPT_COST;
182182

183+
if (memchr(ZSTR_VAL(password), '\0', ZSTR_LEN(password))) {
184+
zend_value_error("Bcrypt password must not contain null character");
185+
return NULL;
186+
}
187+
183188
if (options && (zcost = zend_hash_str_find(options, "cost", sizeof("cost")-1)) != NULL) {
184189
cost = zval_get_long(zcost);
185190
}

‎ext/standard/proc_open.c

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -538,11 +538,32 @@ static void append_backslashes(smart_str *str, size_t num_bs)
538538
}
539539
}
540540

541-
/* See https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments */
542-
static void append_win_escaped_arg(smart_str *str, zend_string *arg)
541+
const char *special_chars = "()!^\"<>&|%";
542+
543+
static bool is_special_character_present(const zend_string *arg)
544+
{
545+
for (size_t i = 0; i < ZSTR_LEN(arg); ++i) {
546+
if (strchr(special_chars, ZSTR_VAL(arg)[i]) != NULL) {
547+
return true;
548+
}
549+
}
550+
return false;
551+
}
552+
553+
/* See https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments and
554+
* https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way */
555+
static void append_win_escaped_arg(smart_str *str, zend_string *arg, bool is_cmd_argument)
543556
{
544557
size_t num_bs = 0;
558+
bool has_special_character = false;
545559

560+
if (is_cmd_argument) {
561+
has_special_character = is_special_character_present(arg);
562+
if (has_special_character) {
563+
/* Escape double quote with ^ if executed by cmd.exe. */
564+
smart_str_appendc(str, '^');
565+
}
566+
}
546567
smart_str_appendc(str, '"');
547568
for (size_t i = 0; i < ZSTR_LEN(arg); ++i) {
548569
char c = ZSTR_VAL(arg)[i];
@@ -556,18 +577,71 @@ static void append_win_escaped_arg(smart_str *str, zend_string *arg)
556577
num_bs = num_bs * 2 + 1;
557578
}
558579
append_backslashes(str, num_bs);
580+
if (has_special_character && strchr(special_chars, c) != NULL) {
581+
/* Escape special chars with ^ if executed by cmd.exe. */
582+
smart_str_appendc(str, '^');
583+
}
559584
smart_str_appendc(str, c);
560585
num_bs = 0;
561586
}
562587
append_backslashes(str, num_bs * 2);
588+
if (has_special_character) {
589+
/* Escape double quote with ^ if executed by cmd.exe. */
590+
smart_str_appendc(str, '^');
591+
}
563592
smart_str_appendc(str, '"');
564593
}
565594

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);
598+
599+
if (suffix_len > str_len) {
600+
return -1; /* Suffix is longer than string, cannot match. */
601+
}
602+
603+
/* Compare the end of the string with the suffix, ignoring case. */
604+
return _stricmp(str + (str_len - suffix_len), suffix);
605+
}
606+
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+
}
614+
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;
620+
}
621+
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, '.');
626+
} 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;
633+
}
634+
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;
637+
}
638+
566639
static zend_string *create_win_command_from_args(HashTable *args)
567640
{
568641
smart_str str = {0};
569642
zval *arg_zv;
570-
bool is_prog_name = 1;
643+
bool is_prog_name = true;
644+
bool is_cmd_execution = false;
571645
int elem_num = 0;
572646

573647
ZEND_HASH_FOREACH_VAL(args, arg_zv) {
@@ -577,11 +651,13 @@ static zend_string *create_win_command_from_args(HashTable *args)
577651
return NULL;
578652
}
579653

580-
if (!is_prog_name) {
654+
if (is_prog_name) {
655+
is_cmd_execution = is_executed_by_cmd(ZSTR_VAL(arg_str));
656+
} else {
581657
smart_str_appendc(&str, ' ');
582658
}
583659

584-
append_win_escaped_arg(&str, arg_str);
660+
append_win_escaped_arg(&str, arg_str, !is_prog_name && is_cmd_execution);
585661

586662
is_prog_name = 0;
587663
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+
?>

‎ext/standard/tests/ghsa-wpj3-hf5j-x4v4.phpt

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
--TEST--
2+
ghsa-wpj3-hf5j-x4v4 (__Host-/__Secure- cookie bypass due to partial CVE-2022-31629 fix)
3+
--COOKIE--
4+
..Host-test=ignore_1;
5+
._Host-test=ignore_2;
6+
.[Host-test=ignore_3;
7+
_.Host-test=ignore_4;
8+
__Host-test=ignore_5;
9+
_[Host-test=ignore_6;
10+
[.Host-test=ignore_7;
11+
[_Host-test=ignore_8;
12+
[[Host-test=ignore_9;
13+
..Host-test[]=ignore_10;
14+
._Host-test[]=ignore_11;
15+
.[Host-test[]=ignore_12;
16+
_.Host-test[]=ignore_13;
17+
__Host-test[]=legitimate_14;
18+
_[Host-test[]=legitimate_15;
19+
[.Host-test[]=ignore_16;
20+
[_Host-test[]=ignore_17;
21+
[[Host-test[]=ignore_18;
22+
..Secure-test=ignore_1;
23+
._Secure-test=ignore_2;
24+
.[Secure-test=ignore_3;
25+
_.Secure-test=ignore_4;
26+
__Secure-test=ignore_5;
27+
_[Secure-test=ignore_6;
28+
[.Secure-test=ignore_7;
29+
[_Secure-test=ignore_8;
30+
[[Secure-test=ignore_9;
31+
..Secure-test[]=ignore_10;
32+
._Secure-test[]=ignore_11;
33+
.[Secure-test[]=ignore_12;
34+
_.Secure-test[]=ignore_13;
35+
__Secure-test[]=legitimate_14;
36+
_[Secure-test[]=legitimate_15;
37+
[.Secure-test[]=ignore_16;
38+
[_Secure-test[]=ignore_17;
39+
[[Secure-test[]=ignore_18;
40+
--FILE--
41+
<?php
42+
var_dump($_COOKIE);
43+
?>
44+
--EXPECT--
45+
array(3) {
46+
["__Host-test"]=>
47+
array(1) {
48+
[0]=>
49+
string(13) "legitimate_14"
50+
}
51+
["_"]=>
52+
array(2) {
53+
["Host-test["]=>
54+
string(13) "legitimate_15"
55+
["Secure-test["]=>
56+
string(13) "legitimate_15"
57+
}
58+
["__Secure-test"]=>
59+
array(1) {
60+
[0]=>
61+
string(13) "legitimate_14"
62+
}
63+
}

‎ext/standard/tests/password/password_bcrypt_errors.phpt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,14 @@ try {
1414
} catch (ValueError $exception) {
1515
echo $exception->getMessage() . "\n";
1616
}
17+
18+
try {
19+
var_dump(password_hash("null\0password", PASSWORD_BCRYPT));
20+
} catch (ValueError $e) {
21+
echo $e->getMessage(), "\n";
22+
}
1723
?>
1824
--EXPECT--
1925
Invalid bcrypt cost parameter specified: 3
2026
Invalid bcrypt cost parameter specified: 32
27+
Bcrypt password must not contain null character

‎main/php_variables.c

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,21 @@ PHPAPI void php_register_known_variable(const char *var_name, size_t var_name_le
8989
php_register_variable_quick(var_name, var_name_len, value, symbol_table);
9090
}
9191

92+
/* Discard variable if mangling made it start with __Host-, where pre-mangling it did not start with __Host-
93+
* Discard variable if mangling made it start with __Secure-, where pre-mangling it did not start with __Secure- */
94+
static bool php_is_forbidden_variable_name(const char *mangled_name, size_t mangled_name_len, const char *pre_mangled_name)
95+
{
96+
if (mangled_name_len >= sizeof("__Host-")-1 && strncmp(mangled_name, "__Host-", sizeof("__Host-")-1) == 0 && strncmp(pre_mangled_name, "__Host-", sizeof("__Host-")-1) != 0) {
97+
return true;
98+
}
99+
100+
if (mangled_name_len >= sizeof("__Secure-")-1 && strncmp(mangled_name, "__Secure-", sizeof("__Secure-")-1) == 0 && strncmp(pre_mangled_name, "__Secure-", sizeof("__Secure-")-1) != 0) {
101+
return true;
102+
}
103+
104+
return false;
105+
}
106+
92107
PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *track_vars_array)
93108
{
94109
char *p = NULL;
@@ -139,20 +154,6 @@ PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *trac
139154
}
140155
var_len = p - var;
141156

142-
/* Discard variable if mangling made it start with __Host-, where pre-mangling it did not start with __Host- */
143-
if (strncmp(var, "__Host-", sizeof("__Host-")-1) == 0 && strncmp(var_name, "__Host-", sizeof("__Host-")-1) != 0) {
144-
zval_ptr_dtor_nogc(val);
145-
free_alloca(var_orig, use_heap);
146-
return;
147-
}
148-
149-
/* Discard variable if mangling made it start with __Secure-, where pre-mangling it did not start with __Secure- */
150-
if (strncmp(var, "__Secure-", sizeof("__Secure-")-1) == 0 && strncmp(var_name, "__Secure-", sizeof("__Secure-")-1) != 0) {
151-
zval_ptr_dtor_nogc(val);
152-
free_alloca(var_orig, use_heap);
153-
return;
154-
}
155-
156157
if (var_len==0) { /* empty variable name, or variable name with a space in it */
157158
zval_ptr_dtor_nogc(val);
158159
free_alloca(var_orig, use_heap);
@@ -256,6 +257,12 @@ PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *trac
256257
return;
257258
}
258259
} else {
260+
if (php_is_forbidden_variable_name(index, index_len, var_name)) {
261+
zval_ptr_dtor_nogc(val);
262+
free_alloca(var_orig, use_heap);
263+
return;
264+
}
265+
259266
gpc_element_p = zend_symtable_str_find(symtable1, index, index_len);
260267
if (!gpc_element_p) {
261268
zval tmp;
@@ -293,6 +300,12 @@ PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *trac
293300
zval_ptr_dtor_nogc(val);
294301
}
295302
} else {
303+
if (php_is_forbidden_variable_name(index, index_len, var_name)) {
304+
zval_ptr_dtor_nogc(val);
305+
free_alloca(var_orig, use_heap);
306+
return;
307+
}
308+
296309
zend_ulong idx;
297310

298311
/*

0 commit comments

Comments
 (0)