From 57eff58f1973699deb12769e37088aeb514bfeac Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 3 Dec 2024 22:47:53 +0100 Subject: [PATCH] Fix GH-17037: UAF in user filter when adding existing filter name due to incorrect error handling There are two functions that can each fail in their own way. If the last function fails we have to remove the filter entry from the hash table, otherwise we risk a UAF. Note also that removing the entry from the table on failure will also free its memory. --- ext/standard/tests/filters/gh17037.phpt | 8 ++++++++ ext/standard/user_filters.c | 12 ++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 ext/standard/tests/filters/gh17037.phpt diff --git a/ext/standard/tests/filters/gh17037.phpt b/ext/standard/tests/filters/gh17037.phpt new file mode 100644 index 0000000000000..21319ba26bf99 --- /dev/null +++ b/ext/standard/tests/filters/gh17037.phpt @@ -0,0 +1,8 @@ +--TEST-- +GH-17037 (UAF in user filter when adding existing filter name due to incorrect error handling) +--FILE-- + +--EXPECT-- +bool(false) diff --git a/ext/standard/user_filters.c b/ext/standard/user_filters.c index 063895e2f4049..737237f6630cd 100644 --- a/ext/standard/user_filters.c +++ b/ext/standard/user_filters.c @@ -516,13 +516,17 @@ PHP_FUNCTION(stream_filter_register) fdat = ecalloc(1, sizeof(struct php_user_filter_data)); fdat->classname = zend_string_copy(classname); - if (zend_hash_add_ptr(BG(user_filter_map), filtername, fdat) != NULL && - php_stream_filter_register_factory_volatile(filtername, &user_filter_factory) == SUCCESS) { - RETVAL_TRUE; + if (zend_hash_add_ptr(BG(user_filter_map), filtername, fdat) != NULL) { + if (php_stream_filter_register_factory_volatile(filtername, &user_filter_factory) == SUCCESS) { + RETURN_TRUE; + } + + zend_hash_del(BG(user_filter_map), filtername); } else { zend_string_release_ex(classname, 0); efree(fdat); - RETVAL_FALSE; } + + RETURN_FALSE; } /* }}} */