Skip to content

Commit c213de6

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) (cherry picked from commit a6c1c62) (cherry picked from commit 46b570a)
1 parent 79c0bf8 commit c213de6

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
@@ -59,6 +59,21 @@ PHPAPI void php_register_variable_safe(char *var, char *strval, size_t str_len,
5959
php_register_variable_ex(var, &new_entry, track_vars_array);
6060
}
6161

62+
/* Discard variable if mangling made it start with __Host-, where pre-mangling it did not start with __Host-
63+
* Discard variable if mangling made it start with __Secure-, where pre-mangling it did not start with __Secure- */
64+
static zend_bool php_is_forbidden_variable_name(const char *mangled_name, size_t mangled_name_len, const char *pre_mangled_name)
65+
{
66+
if (mangled_name_len >= sizeof("__Host-")-1 && strncmp(mangled_name, "__Host-", sizeof("__Host-")-1) == 0 && strncmp(pre_mangled_name, "__Host-", sizeof("__Host-")-1) != 0) {
67+
return 1;
68+
}
69+
70+
if (mangled_name_len >= sizeof("__Secure-")-1 && strncmp(mangled_name, "__Secure-", sizeof("__Secure-")-1) == 0 && strncmp(pre_mangled_name, "__Secure-", sizeof("__Secure-")-1) != 0) {
71+
return 1;
72+
}
73+
74+
return 0;
75+
}
76+
6277
PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars_array)
6378
{
6479
char *p = NULL;
@@ -109,20 +124,6 @@ PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars
109124
}
110125
var_len = p - var;
111126

112-
/* Discard variable if mangling made it start with __Host-, where pre-mangling it did not start with __Host- */
113-
if (strncmp(var, "__Host-", sizeof("__Host-")-1) == 0 && strncmp(var_name, "__Host-", sizeof("__Host-")-1) != 0) {
114-
zval_ptr_dtor_nogc(val);
115-
free_alloca(var_orig, use_heap);
116-
return;
117-
}
118-
119-
/* Discard variable if mangling made it start with __Secure-, where pre-mangling it did not start with __Secure- */
120-
if (strncmp(var, "__Secure-", sizeof("__Secure-")-1) == 0 && strncmp(var_name, "__Secure-", sizeof("__Secure-")-1) != 0) {
121-
zval_ptr_dtor_nogc(val);
122-
free_alloca(var_orig, use_heap);
123-
return;
124-
}
125-
126127
if (var_len==0) { /* empty variable name, or variable name with a space in it */
127128
zval_dtor(val);
128129
free_alloca(var_orig, use_heap);
@@ -220,6 +221,12 @@ PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars
220221
return;
221222
}
222223
} else {
224+
if (php_is_forbidden_variable_name(index, index_len, var_name)) {
225+
zval_ptr_dtor_nogc(val);
226+
free_alloca(var_orig, use_heap);
227+
return;
228+
}
229+
223230
gpc_element_p = zend_symtable_str_find(symtable1, index, index_len);
224231
if (!gpc_element_p) {
225232
zval tmp;
@@ -258,6 +265,12 @@ PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars
258265
zval_ptr_dtor(&gpc_element);
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
/*
262275
* According to rfc2965, more specific paths are listed above the less specific ones.
263276
* If we encounter a duplicate cookie name, we should skip it, since it is not possible

0 commit comments

Comments
 (0)