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 Ben Ramsey
parent e3c784f2bf
commit 093c08af25
No known key found for this signature in database
GPG key ID: F9C39DC0B9698544
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

@ -54,6 +54,21 @@ static zend_always_inline void php_register_variable_quick(const char *name, siz
zend_string_release_ex(key, 0); zend_string_release_ex(key, 0);
} }
/* 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;
@ -104,20 +119,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);
@ -221,6 +222,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;
@ -258,6 +265,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;
/* /*