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.
This commit is contained in:
Niels Dossche 2024-03-17 21:04:47 +01:00 committed by Pierrick Charron
parent 7a5000a3f7
commit 2b8d049317
No known key found for this signature in database
GPG key ID: 286AF1F9897469DC
2 changed files with 90 additions and 14 deletions

View file

@ -0,0 +1,63 @@
--TEST--
ghsa-wpj3-hf5j-x4v4 (__Host-/__Secure- cookie bypass due to partial CVE-2022-31629 fix)
--COOKIE--
..Host-test=ignore_1;
._Host-test=ignore_2;
.[Host-test=ignore_3;
_.Host-test=ignore_4;
__Host-test=ignore_5;
_[Host-test=ignore_6;
[.Host-test=ignore_7;
[_Host-test=ignore_8;
[[Host-test=ignore_9;
..Host-test[]=ignore_10;
._Host-test[]=ignore_11;
.[Host-test[]=ignore_12;
_.Host-test[]=ignore_13;
__Host-test[]=legitimate_14;
_[Host-test[]=legitimate_15;
[.Host-test[]=ignore_16;
[_Host-test[]=ignore_17;
[[Host-test[]=ignore_18;
..Secure-test=ignore_1;
._Secure-test=ignore_2;
.[Secure-test=ignore_3;
_.Secure-test=ignore_4;
__Secure-test=ignore_5;
_[Secure-test=ignore_6;
[.Secure-test=ignore_7;
[_Secure-test=ignore_8;
[[Secure-test=ignore_9;
..Secure-test[]=ignore_10;
._Secure-test[]=ignore_11;
.[Secure-test[]=ignore_12;
_.Secure-test[]=ignore_13;
__Secure-test[]=legitimate_14;
_[Secure-test[]=legitimate_15;
[.Secure-test[]=ignore_16;
[_Secure-test[]=ignore_17;
[[Secure-test[]=ignore_18;
--FILE--
<?php
var_dump($_COOKIE);
?>
--EXPECT--
array(3) {
["__Host-test"]=>
array(1) {
[0]=>
string(13) "legitimate_14"
}
["_"]=>
array(2) {
["Host-test["]=>
string(13) "legitimate_15"
["Secure-test["]=>
string(13) "legitimate_15"
}
["__Secure-test"]=>
array(1) {
[0]=>
string(13) "legitimate_14"
}
}

View file

@ -89,6 +89,21 @@ PHPAPI void php_register_known_variable(const char *var_name, size_t var_name_le
php_register_variable_quick(var_name, var_name_len, value, symbol_table); php_register_variable_quick(var_name, var_name_len, value, symbol_table);
} }
/* Discard variable if mangling made it start with __Host-, where pre-mangling it did not start with __Host-
* Discard variable if mangling made it start with __Secure-, where pre-mangling it did not start with __Secure- */
static bool php_is_forbidden_variable_name(const char *mangled_name, size_t mangled_name_len, const char *pre_mangled_name)
{
if (mangled_name_len >= sizeof("__Host-")-1 && strncmp(mangled_name, "__Host-", sizeof("__Host-")-1) == 0 && strncmp(pre_mangled_name, "__Host-", sizeof("__Host-")-1) != 0) {
return true;
}
if (mangled_name_len >= sizeof("__Secure-")-1 && strncmp(mangled_name, "__Secure-", sizeof("__Secure-")-1) == 0 && strncmp(pre_mangled_name, "__Secure-", sizeof("__Secure-")-1) != 0) {
return true;
}
return false;
}
PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *track_vars_array) PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *track_vars_array)
{ {
char *p = NULL; char *p = NULL;
@ -139,20 +154,6 @@ PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *trac
} }
var_len = p - var; var_len = p - var;
/* Discard variable if mangling made it start with __Host-, where pre-mangling it did not start with __Host- */
if (strncmp(var, "__Host-", sizeof("__Host-")-1) == 0 && strncmp(var_name, "__Host-", sizeof("__Host-")-1) != 0) {
zval_ptr_dtor_nogc(val);
free_alloca(var_orig, use_heap);
return;
}
/* Discard variable if mangling made it start with __Secure-, where pre-mangling it did not start with __Secure- */
if (strncmp(var, "__Secure-", sizeof("__Secure-")-1) == 0 && strncmp(var_name, "__Secure-", sizeof("__Secure-")-1) != 0) {
zval_ptr_dtor_nogc(val);
free_alloca(var_orig, use_heap);
return;
}
if (var_len==0) { /* empty variable name, or variable name with a space in it */ if (var_len==0) { /* empty variable name, or variable name with a space in it */
zval_ptr_dtor_nogc(val); zval_ptr_dtor_nogc(val);
free_alloca(var_orig, use_heap); free_alloca(var_orig, use_heap);
@ -256,6 +257,12 @@ PHPAPI void php_register_variable_ex(const char *var_name, zval *val, zval *trac
return; return;
} }
} else { } else {
if (php_is_forbidden_variable_name(index, index_len, var_name)) {
zval_ptr_dtor_nogc(val);
free_alloca(var_orig, use_heap);
return;
}
gpc_element_p = zend_symtable_str_find(symtable1, index, index_len); gpc_element_p = zend_symtable_str_find(symtable1, index, index_len);
if (!gpc_element_p) { if (!gpc_element_p) {
zval tmp; zval tmp;
@ -293,6 +300,12 @@ plain_var:
zval_ptr_dtor_nogc(val); zval_ptr_dtor_nogc(val);
} }
} else { } else {
if (php_is_forbidden_variable_name(index, index_len, var_name)) {
zval_ptr_dtor_nogc(val);
free_alloca(var_orig, use_heap);
return;
}
zend_ulong idx; zend_ulong idx;
/* /*