From 2e07a3acd7a6b53c55325b94bed97748d7697b53 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 17 Mar 2024 21:04:47 +0100 Subject: [PATCH 1/4] 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) --- ext/standard/tests/ghsa-wpj3-hf5j-x4v4.phpt | 63 +++++++++++++++++++++ main/php_variables.c | 41 +++++++++----- 2 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 ext/standard/tests/ghsa-wpj3-hf5j-x4v4.phpt diff --git a/ext/standard/tests/ghsa-wpj3-hf5j-x4v4.phpt b/ext/standard/tests/ghsa-wpj3-hf5j-x4v4.phpt new file mode 100644 index 00000000000..77fcb680894 --- /dev/null +++ b/ext/standard/tests/ghsa-wpj3-hf5j-x4v4.phpt @@ -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-- + +--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" + } +} diff --git a/main/php_variables.c b/main/php_variables.c index 27a9ad089e7..dc888bdfc64 100644 --- a/main/php_variables.c +++ b/main/php_variables.c @@ -54,6 +54,21 @@ static zend_always_inline void php_register_variable_quick(const char *name, siz 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) { 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; - /* 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 */ zval_ptr_dtor_nogc(val); 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; } } 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); if (!gpc_element_p) { zval tmp; @@ -258,6 +265,12 @@ plain_var: zval_ptr_dtor_nogc(val); } } 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; /* -- 2.44.0 From 366cc249b7d54707572beb7096e8f6c65ee79719 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Wed, 10 Apr 2024 08:59:32 +0200 Subject: [PATCH 2/4] NEWS --- NEWS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS b/NEWS index 8147a7e517c..14fda3a58b9 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,12 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| + +Backported from 8.1.28 + +- Standard: + . Fixed bug GHSA-wpj3-hf5j-x4v4 (__Host-/__Secure- cookie bypass due to + partial CVE-2022-31629 fix). (CVE-2024-2756) (nielsdos) + 03 Aug 2023, PHP 8.0.30 - Libxml: -- 2.44.0