Commit e461e70c70d for php.net
commit e461e70c70d5ef21764aac9b352fac6fafbbe9f8
Author: Gina Peter Banyard <girgias@php.net>
Date: Thu Jul 30 19:10:18 2026 +0100
streams: check class exists when attempting to register stream filter (#22823)
And do a few other checks such as is the class even instantiable.
It might make sense in the future to raise a deprecation if the class does not extend php_user_filter or create a new StreamFilter interface so that we know the various methods exist
diff --git a/ext/standard/tests/filters/001.phpt b/ext/standard/tests/filters/001.phpt
index 3f0d5f5eb44..0a91b721344 100644
--- a/ext/standard/tests/filters/001.phpt
+++ b/ext/standard/tests/filters/001.phpt
@@ -3,30 +3,27 @@
--FILE--
<?php
try {
- stream_filter_register("", "");
-} catch (ValueError $exception) {
- echo $exception->getMessage() . "\n";
+ stream_filter_register("", "stdClass");
+} catch (Throwable $e) {
+ echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}
try {
- stream_filter_register("test", "");
-} catch (ValueError $exception) {
- echo $exception->getMessage() . "\n";
+ stream_filter_register("test", "Throwable");
+} catch (Throwable $e) {
+ echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}
try {
- stream_filter_register("", "test");
-} catch (ValueError $exception) {
- echo $exception->getMessage() . "\n";
+ var_dump(stream_filter_register("------", "nonexistentclass"));
+} catch (Throwable $e) {
+ echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}
-var_dump(stream_filter_register("------", "nonexistentclass"));
-
echo "Done\n";
?>
--EXPECT--
-stream_filter_register(): Argument #1 ($filter_name) must be a non-empty string
-stream_filter_register(): Argument #2 ($class) must be a non-empty string
-stream_filter_register(): Argument #1 ($filter_name) must be a non-empty string
-bool(true)
+ValueError: stream_filter_register(): Argument #1 ($filter_name) must be a non-empty string
+ValueError: stream_filter_register(): Argument #2 ($class) must be a concrete class
+TypeError: stream_filter_register(): Argument #2 ($class) must be a valid class name, nonexistentclass given
Done
diff --git a/ext/standard/tests/filters/gh17037.phpt b/ext/standard/tests/filters/gh17037.phpt
index 21319ba26bf..c6124c0fc2b 100644
--- a/ext/standard/tests/filters/gh17037.phpt
+++ b/ext/standard/tests/filters/gh17037.phpt
@@ -2,7 +2,13 @@
GH-17037 (UAF in user filter when adding existing filter name due to incorrect error handling)
--FILE--
<?php
-var_dump(stream_filter_register('string.toupper', 'filter_string_toupper'));
+
+class filter_string_toupper {}
+try {
+ var_dump(stream_filter_register('string.toupper', 'filter_string_toupper'));
+} catch (Throwable $e) {
+ echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
?>
--EXPECT--
bool(false)
diff --git a/ext/standard/tests/filters/stream_filter_register_non_existing_class.phpt b/ext/standard/tests/filters/stream_filter_register_non_existing_class.phpt
index 1c08201cff0..b404d45524b 100644
--- a/ext/standard/tests/filters/stream_filter_register_non_existing_class.phpt
+++ b/ext/standard/tests/filters/stream_filter_register_non_existing_class.phpt
@@ -3,7 +3,11 @@
--FILE--
<?php
-var_dump(stream_filter_register("not_existing_filter", "not_existing"));
+try {
+ var_dump(stream_filter_register("not_existing_filter", "not_existing"));
+} catch (Throwable $e) {
+ echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
stream_filter_append(STDOUT, "not_existing_filter");
@@ -12,10 +16,8 @@
?>
--EXPECTF--
-bool(true)
+TypeError: stream_filter_register(): Argument #2 ($class) must be a valid class name, not_existing given
-Warning: stream_filter_append(): User-filter "not_existing_filter" requires class "not_existing", but that class is not defined in %s on line %d
-
-Warning: stream_filter_append(): Unable to create or locate filter "not_existing_filter" in %s on line %d
+Warning: stream_filter_append(): Unable to locate filter "not_existing_filter" in %s on line %d
Hello
int(6)
diff --git a/ext/standard/user_filters.c b/ext/standard/user_filters.c
index 126b434713a..9711ad1f83f 100644
--- a/ext/standard/user_filters.c
+++ b/ext/standard/user_filters.c
@@ -23,12 +23,6 @@
#define PHP_STREAM_BRIGADE_RES_NAME "userfilter.bucket brigade"
#define PHP_STREAM_BUCKET_RES_NAME "userfilter.bucket"
-struct php_user_filter_data {
- zend_class_entry *ce;
- /* variable length; this *must* be last in the structure */
- zend_string *classname;
-};
-
/* to provide context for calling into the next filter from user-space */
static int le_bucket_brigade;
static int le_bucket;
@@ -328,7 +322,6 @@ static const php_stream_filter_ops userfilter_ops = {
static php_stream_filter *user_filter_factory_create(const char *filtername,
zval *filterparams, bool persistent)
{
- struct php_user_filter_data *fdat = NULL;
php_stream_filter *filter;
zval obj;
zval retval;
@@ -347,8 +340,9 @@ static php_stream_filter *user_filter_factory_create(const char *filtername,
len = strlen(filtername);
- /* determine the classname/class entry */
- if (NULL == (fdat = zend_hash_str_find_ptr(BG(user_filter_map), filtername, len))) {
+ /* determine the class entry */
+ /* const */ zend_class_entry *ce = zend_hash_str_find_ptr(BG(user_filter_map), filtername, len);
+ if (UNEXPECTED(ce == NULL)) {
const char *period;
/* Userspace Filters using ambiguous wildcards could cause problems.
@@ -366,7 +360,8 @@ static php_stream_filter *user_filter_factory_create(const char *filtername,
ZEND_ASSERT(new_period[0] == '.');
new_period[1] = '*';
new_period[2] = '\0';
- if (NULL != (fdat = zend_hash_str_find_ptr(BG(user_filter_map), wildcard, strlen(wildcard)))) {
+ ce = zend_hash_str_find_ptr(BG(user_filter_map), wildcard, strlen(wildcard));
+ if (NULL != ce) {
new_period = NULL;
} else {
*new_period = '\0';
@@ -375,21 +370,11 @@ static php_stream_filter *user_filter_factory_create(const char *filtername,
}
efree(wildcard);
}
- ZEND_ASSERT(fdat);
- }
-
- /* bind the classname to the actual class */
- if (fdat->ce == NULL) {
- if (NULL == (fdat->ce = zend_lookup_class(fdat->classname))) {
- php_error_docref(NULL, E_WARNING,
- "User-filter \"%s\" requires class \"%s\", but that class is not defined",
- filtername, ZSTR_VAL(fdat->classname));
- return NULL;
- }
+ ZEND_ASSERT(ce);
}
/* create the object */
- if (object_init_ex(&obj, fdat->ce) == FAILURE) {
+ if (object_init_ex(&obj, ce) == FAILURE) {
return NULL;
}
@@ -435,13 +420,6 @@ static const php_stream_filter_factory user_filter_factory = {
user_filter_factory_create
};
-static void filter_item_dtor(zval *zv)
-{
- struct php_user_filter_data *fdat = Z_PTR_P(zv);
- zend_string_release_ex(fdat->classname, 0);
- efree(fdat);
-}
-
/* {{{ Return a bucket object from the brigade for operating on */
PHP_FUNCTION(stream_bucket_make_writeable)
{
@@ -604,12 +582,12 @@ PHP_FUNCTION(stream_get_filters)
/* {{{ Registers a custom filter handler class */
PHP_FUNCTION(stream_filter_register)
{
- zend_string *filtername, *classname;
- struct php_user_filter_data *fdat;
+ zend_string *filtername;
+ zend_class_entry *ce = NULL;
ZEND_PARSE_PARAMETERS_START(2, 2)
Z_PARAM_STR(filtername)
- Z_PARAM_STR(classname)
+ Z_PARAM_CLASS(ce)
ZEND_PARSE_PARAMETERS_END();
if (!ZSTR_LEN(filtername)) {
@@ -617,28 +595,24 @@ PHP_FUNCTION(stream_filter_register)
RETURN_THROWS();
}
- if (!ZSTR_LEN(classname)) {
- zend_argument_value_error(2, "must be a non-empty string");
+ /* TODO: Check class is a child of php_user_filter? */
+ if (UNEXPECTED(ce->ce_flags & ZEND_ACC_UNINSTANTIABLE)) {
+ zend_argument_value_error(2, "must be a concrete class");
RETURN_THROWS();
}
if (!BG(user_filter_map)) {
BG(user_filter_map) = (HashTable*) emalloc(sizeof(HashTable));
- zend_hash_init(BG(user_filter_map), 8, NULL, (dtor_func_t) filter_item_dtor, 0);
+ /* We don't need a destructor as we are only storing a CE which should be never modified */
+ zend_hash_init(BG(user_filter_map), 8, NULL, NULL, 0);
}
- 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) {
+ if (zend_hash_add_ptr(BG(user_filter_map), filtername, ce) != 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);
}
RETURN_FALSE;