Skip to content

Commit a6c1c62

Browse files
nielsdosremicollet
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. (cherry picked from commit 093c08af25fb323efa0c8e6154aa9fdeae3d3b53) (cherry picked from commit 2e07a3a)
1 parent 38c5fb2 commit a6c1c62

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
@@ -65,6 +65,21 @@ static zend_always_inline void php_register_variable_quick(const char *name, siz
6565
zend_string_release_ex(key, 0);
6666
}
6767

68+
/* Discard variable if mangling made it start with __Host-, where pre-mangling it did not start with __Host-
69+
* Discard variable if mangling made it start with __Secure-, where pre-mangling it did not start with __Secure- */
70+
static zend_bool php_is_forbidden_variable_name(const char *mangled_name, size_t mangled_name_len, const char *pre_mangled_name)
71+
{
72+
if (mangled_name_len >= sizeof("__Host-")-1 && strncmp(mangled_name, "__Host-", sizeof("__Host-")-1) == 0 && strncmp(pre_mangled_name, "__Host-", sizeof("__Host-")-1) != 0) {
73+
return 1;
74+
}
75+
76+
if (mangled_name_len >= sizeof("__Secure-")-1 && strncmp(mangled_name, "__Secure-", sizeof("__Secure-")-1) == 0 && strncmp(pre_mangled_name, "__Secure-", sizeof("__Secure-")-1) != 0) {
77+
return 1;
78+
}
79+
80+
return 0;
81+
}
82+
6883
PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars_array)
6984
{
7085
char *p = NULL;
@@ -115,20 +130,6 @@ PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars
115130
}
116131
var_len = p - var;
117132

118-
/* Discard variable if mangling made it start with __Host-, where pre-mangling it did not start with __Host- */
119-
if (strncmp(var, "__Host-", sizeof("__Host-")-1) == 0 && strncmp(var_name, "__Host-", sizeof("__Host-")-1) != 0) {
120-
zval_ptr_dtor_nogc(val);
121-
free_alloca(var_orig, use_heap);
122-
return;
123-
}
124-
125-
/* Discard variable if mangling made it start with __Secure-, where pre-mangling it did not start with __Secure- */
126-
if (strncmp(var, "__Secure-", sizeof("__Secure-")-1) == 0 && strncmp(var_name, "__Secure-", sizeof("__Secure-")-1) != 0) {
127-
zval_ptr_dtor_nogc(val);
128-
free_alloca(var_orig, use_heap);
129-
return;
130-
}
131-
132133
if (var_len==0) { /* empty variable name, or variable name with a space in it */
133134
zval_ptr_dtor_nogc(val);
134135
free_alloca(var_orig, use_heap);
@@ -226,6 +227,12 @@ PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars
226227
return;
227228
}
228229
} else {
230+
if (php_is_forbidden_variable_name(index, index_len, var_name)) {
231+
zval_ptr_dtor_nogc(val);
232+
free_alloca(var_orig, use_heap);
233+
return;
234+
}
235+
229236
gpc_element_p = zend_symtable_str_find(symtable1, index, index_len);
230237
if (!gpc_element_p) {
231238
zval tmp;
@@ -263,6 +270,12 @@ PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars
263270
zval_ptr_dtor_nogc(val);
264271
}
265272
} else {
273+
if (php_is_forbidden_variable_name(index, index_len, var_name)) {
274+
zval_ptr_dtor_nogc(val);
275+
free_alloca(var_orig, use_heap);
276+
return;
277+
}
278+
266279
zend_ulong idx;
267280

268281
/*

0 commit comments

Comments
 (0)