From 9044533b487a08831f08a6188fc46d3333645a76 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 18 Jul 2023 12:01:57 +0200 Subject: [PATCH 1/2] Fix use-after-free when unregistering user stream wrapper from itself Fixes GH-11735 --- Zend/tests/gh11735_1.phpt | 14 ++++++++++++++ Zend/tests/gh11735_2.phpt | 14 ++++++++++++++ main/streams/userspace.c | 10 ++++++---- 3 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 Zend/tests/gh11735_1.phpt create mode 100644 Zend/tests/gh11735_2.phpt diff --git a/Zend/tests/gh11735_1.phpt b/Zend/tests/gh11735_1.phpt new file mode 100644 index 0000000000000..244c839bd0389 --- /dev/null +++ b/Zend/tests/gh11735_1.phpt @@ -0,0 +1,14 @@ +--TEST-- +GH-11735: Use-after-free when unregistering user stream wrapper from user stream wrapper +--FILE-- + +--EXPECT-- diff --git a/Zend/tests/gh11735_2.phpt b/Zend/tests/gh11735_2.phpt new file mode 100644 index 0000000000000..7fb0f0825d4ac --- /dev/null +++ b/Zend/tests/gh11735_2.phpt @@ -0,0 +1,14 @@ +--TEST-- +GH-11735: Use-after-free when unregistering user stream wrapper from user stream wrapper +--FILE-- + +--EXPECT-- diff --git a/main/streams/userspace.c b/main/streams/userspace.c index 1b113423d7140..6835863a80bea 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(uwrap->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(uwrap->resource); efree(us); } zval_ptr_dtor(&zretval); From f4d4a79979386c9efdaa7447ad9cb85e339d50fb Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 18 Jul 2023 14:54:35 +0200 Subject: [PATCH 2/2] Improve tests --- Zend/tests/gh11735_1.phpt | 6 ++++-- Zend/tests/gh11735_2.phpt | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Zend/tests/gh11735_1.phpt b/Zend/tests/gh11735_1.phpt index 244c839bd0389..3ddf0185e9845 100644 --- a/Zend/tests/gh11735_1.phpt +++ b/Zend/tests/gh11735_1.phpt @@ -3,12 +3,14 @@ GH-11735: Use-after-free when unregistering user stream wrapper from user stream --FILE-- ---EXPECT-- +--EXPECTF-- +resource(%d) of type (stream) diff --git a/Zend/tests/gh11735_2.phpt b/Zend/tests/gh11735_2.phpt index 7fb0f0825d4ac..b568b6f6df378 100644 --- a/Zend/tests/gh11735_2.phpt +++ b/Zend/tests/gh11735_2.phpt @@ -3,12 +3,15 @@ GH-11735: Use-after-free when unregistering user stream wrapper from user stream --FILE-- ---EXPECT-- +--EXPECTF-- +Warning: fopen(foo://bar): Failed to open stream: "FooWrapper::stream_open" call failed in %s on line %d +bool(false)