From c3ccc363c60ccd03fc8b6d121d7f6735f853c026 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 18 Jul 2023 12:01:57 +0200 Subject: [PATCH] Fix use-after-free when unregistering user stream wrapper from itself Fixes GH-11735 Closes GH-11737 --- NEWS | 4 ++++ Zend/tests/gh11735_1.phpt | 16 ++++++++++++++++ Zend/tests/gh11735_2.phpt | 17 +++++++++++++++++ main/streams/userspace.c | 10 ++++++---- 4 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 Zend/tests/gh11735_1.phpt create mode 100644 Zend/tests/gh11735_2.phpt diff --git a/NEWS b/NEWS index 0d4fd6f2a35..22748fc8881 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,10 @@ PHP NEWS . Fix use-of-uninitialized-value in hash_pbkdf2(), fix missing $options parameter in signature. (ilutov) +- Streams: + . Fixed bug GH-11735 (Use-after-free when unregistering user stream wrapper + from itself). (ilutov) + 03 Aug 2023, PHP 8.2.9 - Build: diff --git a/Zend/tests/gh11735_1.phpt b/Zend/tests/gh11735_1.phpt new file mode 100644 index 00000000000..3ddf0185e98 --- /dev/null +++ b/Zend/tests/gh11735_1.phpt @@ -0,0 +1,16 @@ +--TEST-- +GH-11735: Use-after-free when unregistering user stream wrapper from user stream wrapper +--FILE-- + +--EXPECTF-- +resource(%d) of type (stream) diff --git a/Zend/tests/gh11735_2.phpt b/Zend/tests/gh11735_2.phpt new file mode 100644 index 00000000000..b568b6f6df3 --- /dev/null +++ b/Zend/tests/gh11735_2.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-11735: Use-after-free when unregistering user stream wrapper from user stream wrapper +--FILE-- + +--EXPECTF-- +Warning: fopen(foo://bar): Failed to open stream: "FooWrapper::stream_open" call failed in %s on line %d +bool(false) diff --git a/main/streams/userspace.c b/main/streams/userspace.c index 1b113423d71..f7223eab129 100644 --- a/main/streams/userspace.c +++ b/main/streams/userspace.c @@ -341,6 +341,8 @@ static php_stream *user_wrapper_opener(php_stream_wrapper *wrapper, const char * us = emalloc(sizeof(*us)); us->wrapper = uwrap; + /* call_method_if_exists() may unregister the stream wrapper. Hold on to it. */ + GC_ADDREF(us->wrapper->resource); user_stream_create_object(uwrap, context, &us->object); if (Z_TYPE(us->object) == IS_UNDEF) { @@ -376,8 +378,6 @@ static php_stream *user_wrapper_opener(php_stream_wrapper *wrapper, const char * /* set wrapper data to be a reference to our object */ ZVAL_COPY(&stream->wrapperdata, &us->object); - - GC_ADDREF(us->wrapper->resource); } else { php_stream_wrapper_log_error(wrapper, options, "\"%s::" USERSTREAM_OPEN "\" call failed", ZSTR_VAL(us->wrapper->ce->name)); @@ -387,6 +387,7 @@ static php_stream *user_wrapper_opener(php_stream_wrapper *wrapper, const char * if (stream == NULL) { zval_ptr_dtor(&us->object); ZVAL_UNDEF(&us->object); + zend_list_delete(us->wrapper->resource); efree(us); } zval_ptr_dtor(&zretval); @@ -429,6 +430,8 @@ static php_stream *user_wrapper_opendir(php_stream_wrapper *wrapper, const char us = emalloc(sizeof(*us)); us->wrapper = uwrap; + /* call_method_if_exists() may unregister the stream wrapper. Hold on to it. */ + GC_ADDREF(us->wrapper->resource); user_stream_create_object(uwrap, context, &us->object); if (Z_TYPE(us->object) == IS_UNDEF) { @@ -451,8 +454,6 @@ static php_stream *user_wrapper_opendir(php_stream_wrapper *wrapper, const char /* set wrapper data to be a reference to our object */ ZVAL_COPY(&stream->wrapperdata, &us->object); - - GC_ADDREF(us->wrapper->resource); } else { php_stream_wrapper_log_error(wrapper, options, "\"%s::" USERSTREAM_DIR_OPEN "\" call failed", ZSTR_VAL(us->wrapper->ce->name)); @@ -462,6 +463,7 @@ static php_stream *user_wrapper_opendir(php_stream_wrapper *wrapper, const char if (stream == NULL) { zval_ptr_dtor(&us->object); ZVAL_UNDEF(&us->object); + zend_list_delete(us->wrapper->resource); efree(us); } zval_ptr_dtor(&zretval);