From 00f4881e90f30a000483a301f0258a905e60882e 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. Closes GH-17038. --- NEWS | 3 +++ ext/standard/tests/filters/gh17037.phpt | 8 ++++++++ ext/standard/user_filters.c | 12 ++++++++---- 3 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 ext/standard/tests/filters/gh17037.phpt diff --git a/NEWS b/NEWS index e52764d78db..a139c14513a 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.3.16 +- Streams: + . Fixed bug GH-17037 (UAF in user filter when adding existing filter name due + to incorrect error handling). (nielsdos) 19 Dec 2024, PHP 8.3.15 diff --git a/ext/standard/tests/filters/gh17037.phpt b/ext/standard/tests/filters/gh17037.phpt new file mode 100644 index 00000000000..21319ba26bf --- /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 063895e2f40..737237f6630 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; } /* }}} */