Skip to content

Commit 2e07a3a

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)
1 parent 07fa081 commit 2e07a3a

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
@@ -54,6 +54,21 @@ static zend_always_inline void php_register_variable_quick(const char *name, siz
5454
zend_string_release_ex(key, 0);
5555
}
5656

57+
/* Discard variable if mangling made it start with __Host-, where pre-mangling it did not start with __Host-
58+
* Discard variable if mangling made it start with __Secure-, where pre-mangling it did not start with __Secure- */
59+
static bool php_is_forbidden_variable_name(const char *mangled_name, size_t mangled_name_len, const char *pre_mangled_name)
60+
{
61+
if (mangled_name_len >= sizeof("__Host-")-1 && strncmp(mangled_name, "__Host-", sizeof("__Host-")-1) == 0 && strncmp(pre_mangled_name, "__Host-", sizeof("__Host-")-1) != 0) {
62+
return true;
63+
}
64+
65+
if (mangled_name_len >= sizeof("__Secure-")-1 && strncmp(mangled_name, "__Secure-", sizeof("__Secure-")-1) == 0 && strncmp(pre_mangled_name, "__Secure-", sizeof("__Secure-")-1) != 0) {
66+
return true;
67+
}
68+
69+
return false;
70+
}
71+
5772
PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *track_vars_array)
5873
{
5974
char *p = NULL;
@@ -104,20 +119,6 @@ PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *trac
104119
}
105120
var_len = p - var;
106121

107-
/* Discard variable if mangling made it start with __Host-, where pre-mangling it did not start with __Host- */
108-
if (strncmp(var, "__Host-", sizeof("__Host-")-1) == 0 && strncmp(var_name, "__Host-", sizeof("__Host-")-1) != 0) {
109-
zval_ptr_dtor_nogc(val);
110-
free_alloca(var_orig, use_heap);
111-
return;
112-
}
113-
114-
/* Discard variable if mangling made it start with __Secure-, where pre-mangling it did not start with __Secure- */
115-
if (strncmp(var, "__Secure-", sizeof("__Secure-")-1) == 0 && strncmp(var_name, "__Secure-", sizeof("__Secure-")-1) != 0) {
116-
zval_ptr_dtor_nogc(val);
117-
free_alloca(var_orig, use_heap);
118-
return;
119-
}
120-
121122
if (var_len==0) { /* empty variable name, or variable name with a space in it */
122123
zval_ptr_dtor_nogc(val);
123124
free_alloca(var_orig, use_heap);
@@ -221,6 +222,12 @@ PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *trac
221222
return;
222223
}
223224
} else {
225+
if (php_is_forbidden_variable_name(index, index_len, var_name)) {
226+
zval_ptr_dtor_nogc(val);
227+
free_alloca(var_orig, use_heap);
228+
return;
229+
}
230+
224231
gpc_element_p = zend_symtable_str_find(symtable1, index, index_len);
225232
if (!gpc_element_p) {
226233
zval tmp;
@@ -258,6 +265,12 @@ PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *trac
258265
zval_ptr_dtor_nogc(val);
259266
}
260267
} else {
268+
if (php_is_forbidden_variable_name(index, index_len, var_name)) {
269+
zval_ptr_dtor_nogc(val);
270+
free_alloca(var_orig, use_heap);
271+
return;
272+
}
273+
261274
zend_ulong idx;
262275

263276
/*

0 commit comments

Comments
 (0)