Skip to content

Commit f77e579

Browse files
nielsdosericmann
authored andcommitted
Fix GHSA-wpj3-hf5j-x4v4: __Host-/__Secure- cookie bypass due to partial CVE-2022-31629 fix
The check happened too early as later code paths may perform more mangling rules. Move the check downwards right before adding the actual variable.
1 parent 0d89b54 commit f77e579

File tree

2 files changed

+90
-14
lines changed

2 files changed

+90
-14
lines changed
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+
}

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)