From db7b1beea1805812d62ab787ebea44a918df84b9 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) (cherry picked from commit 2e07a3acd7a6b53c55325b94bed97748d7697b53) (cherry picked from commit a6c1c62a25ac23b08a86af11d68f0e2eaafc102b) (cherry picked from commit 46b570a1e4aeb4a414898fcc09503ac388d16256) (cherry picked from commit c213de619a532d35e8f7abe4a245433dbf21c960) (cherry picked from commit a1b0060906bc4eedaf5bb3577a0d6d4b0e6b9dfd) (cherry picked from commit ec9b61593fa2b9400d4519b9969645c1266a381d) --- 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 0000000000..77fcb68089 --- /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 fb58986f20..fbd9562e8d 100644 --- a/main/php_variables.c +++ b/main/php_variables.c @@ -56,6 +56,21 @@ PHPAPI void php_register_variable_safe(char *var, char *strval, int str_len, zva php_register_variable_ex(var, &new_entry, track_vars_array TSRMLS_CC); } +/* 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 zend_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 1; + } + + 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 1; + } + + return 0; +} + PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars_array TSRMLS_DC) { char *p = NULL; @@ -106,20 +121,6 @@ PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars } 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_dtor(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_dtor(val); - free_alloca(var_orig, use_heap); - return; - } - if (var_len==0) { /* empty variable name, or variable name with a space in it */ zval_dtor(val); free_alloca(var_orig, use_heap); @@ -198,6 +199,12 @@ PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars return; } } else { + if (php_is_forbidden_variable_name(index, index_len, var_name)) { + zval_dtor(val); + free_alloca(var_orig, use_heap); + return; + } + if (zend_symtable_find(symtable1, index, index_len + 1, (void **) &gpc_element_p) == FAILURE || Z_TYPE_PP(gpc_element_p) != IS_ARRAY) { MAKE_STD_ZVAL(gpc_element); @@ -228,6 +235,12 @@ plain_var: zval_ptr_dtor(&gpc_element); } } else { + if (php_is_forbidden_variable_name(index, index_len, var_name)) { + zval_dtor(val); + free_alloca(var_orig, use_heap); + return; + } + /* * According to rfc2965, more specific paths are listed above the less specific ones. * If we encounter a duplicate cookie name, we should skip it, since it is not possible -- 2.44.0 From ea294ad880a4e1f7ba788150538c0ee405d32d7c Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Wed, 10 Apr 2024 08:59:32 +0200 Subject: [PATCH 2/4] NEWS (cherry picked from commit 366cc249b7d54707572beb7096e8f6c65ee79719) (cherry picked from commit dcdd49ef3bfbd8ccc778850d6a0f9b98adf625d4) (cherry picked from commit 8642473b624f809b768180b104c013f74e3a99a0) (cherry picked from commit ee591001f7a3db7405b4fa027659768c2355df6d) (cherry picked from commit 035bc48bafe5d567f4ab8de6d1752a724e361690) (cherry picked from commit d8e42d4a8471e19710dbb60018ed956eed34af90) --- NEWS | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS b/NEWS index 0e7d9abf11..69736ccb08 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +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) + Backported from 8.0.30 - Libxml: -- 2.44.0