From 9f5b5e34c3b4a6e57b23ed933558210c7dc9e5fc Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 17 Oct 2024 12:06:07 +0100 Subject: [PATCH 1/3] Fix GH-16477 (Segmentation fault when calling __debugInfo() after failed SplFileObject::__constructor) Closes GH-16480 Closes GH-16604 --- ext/spl/spl_directory.c | 19 ++++++++++++------- ext/spl/tests/gh16477-2.phpt | 19 +++++++++++++++++++ ext/spl/tests/gh16477.phpt | 19 +++++++++++++++++++ 3 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 ext/spl/tests/gh16477-2.phpt create mode 100644 ext/spl/tests/gh16477.phpt diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 0440ff59c8ba9..83653f65e9f5f 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -2042,22 +2042,21 @@ static void spl_filesystem_file_rewind(zval * this_ptr, spl_filesystem_object *i PHP_METHOD(SplFileObject, __construct) { spl_filesystem_object *intern = spl_filesystem_from_obj(Z_OBJ_P(ZEND_THIS)); + zend_string *file_name = NULL; zend_string *open_mode = ZSTR_CHAR('r'); + zval *stream_context = NULL; bool use_include_path = 0; size_t path_len; zend_error_handling error_handling; - intern->u.file.open_mode = ZSTR_CHAR('r'); - - if (zend_parse_parameters(ZEND_NUM_ARGS(), "P|Sbr!", - &intern->file_name, &open_mode, - &use_include_path, &intern->u.file.zcontext) == FAILURE) { - intern->u.file.open_mode = NULL; - intern->file_name = NULL; + if (zend_parse_parameters(ZEND_NUM_ARGS(), "P|Sbr!", &file_name, &open_mode, &use_include_path, &stream_context) == FAILURE) { RETURN_THROWS(); } intern->u.file.open_mode = zend_string_copy(open_mode); + /* file_name and zcontext are copied by spl_filesystem_file_open() */ + intern->file_name = file_name; + intern->u.file.zcontext = stream_context; /* spl_filesystem_file_open() can generate E_WARNINGs which we want to promote to exceptions */ zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling); @@ -2096,6 +2095,12 @@ PHP_METHOD(SplTempFileObject, __construct) RETURN_THROWS(); } + /* Prevent reinitialization of Object */ + if (intern->u.file.stream) { + zend_throw_error(NULL, "Cannot call constructor twice"); + RETURN_THROWS(); + } + if (max_memory < 0) { file_name = zend_string_init("php://memory", sizeof("php://memory")-1, 0); } else if (ZEND_NUM_ARGS()) { diff --git a/ext/spl/tests/gh16477-2.phpt b/ext/spl/tests/gh16477-2.phpt new file mode 100644 index 0000000000000..a51b9408c2d44 --- /dev/null +++ b/ext/spl/tests/gh16477-2.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-16477-2: Memory leak when calling SplTempFileObject::__constructor() twice +--FILE-- +__construct(); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +$obj->__debugInfo(); + +?> +DONE +--EXPECT-- +Error: Cannot call constructor twice +DONE diff --git a/ext/spl/tests/gh16477.phpt b/ext/spl/tests/gh16477.phpt new file mode 100644 index 0000000000000..f35c9538e8556 --- /dev/null +++ b/ext/spl/tests/gh16477.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-16477: Segmentation fault when calling __debugInfo() after failed SplFileObject::__constructor +--FILE-- +__construct(); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +$obj->__debugInfo(); + +?> +DONE +--EXPECT-- +ArgumentCountError: SplFileObject::__construct() expects at least 1 argument, 0 given +DONE From a19029fc8bbd2e0681c53eb8ec9025fd0a3eabb1 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 17 Oct 2024 12:06:07 +0100 Subject: [PATCH 2/3] Fix GH-16477 (Segmentation fault when calling __debugInfo() after failed SplFileObject::__constructor) Closes GH-16480 Closes GH-16604 --- NEWS | 2 ++ ext/spl/spl_directory.c | 19 ++++++++++++------- ext/spl/tests/gh16477-2.phpt | 19 +++++++++++++++++++ ext/spl/tests/gh16477.phpt | 19 +++++++++++++++++++ 4 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 ext/spl/tests/gh16477-2.phpt create mode 100644 ext/spl/tests/gh16477.phpt diff --git a/NEWS b/NEWS index 65a257c2b780a..bc9fc62ec11ca 100644 --- a/NEWS +++ b/NEWS @@ -103,6 +103,8 @@ PHP NEWS . Fixed bug GH-16479 (Use-after-free in SplObjectStorage::setInfo()). (ilutov) . Fixed bug GH-16478 (Use-after-free in SplFixedArray::unset()). (ilutov) . Fixed bug GH-16588 (UAF in Observer->serialize). (nielsdos) + . Fix GH-16477 (Segmentation fault when calling __debugInfo() after failed + SplFileObject::__constructor). (Girgias) - Standard: . Fixed bug GH-16293 (Failed assertion when throwing in assert() callback with diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 0440ff59c8ba9..83653f65e9f5f 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -2042,22 +2042,21 @@ static void spl_filesystem_file_rewind(zval * this_ptr, spl_filesystem_object *i PHP_METHOD(SplFileObject, __construct) { spl_filesystem_object *intern = spl_filesystem_from_obj(Z_OBJ_P(ZEND_THIS)); + zend_string *file_name = NULL; zend_string *open_mode = ZSTR_CHAR('r'); + zval *stream_context = NULL; bool use_include_path = 0; size_t path_len; zend_error_handling error_handling; - intern->u.file.open_mode = ZSTR_CHAR('r'); - - if (zend_parse_parameters(ZEND_NUM_ARGS(), "P|Sbr!", - &intern->file_name, &open_mode, - &use_include_path, &intern->u.file.zcontext) == FAILURE) { - intern->u.file.open_mode = NULL; - intern->file_name = NULL; + if (zend_parse_parameters(ZEND_NUM_ARGS(), "P|Sbr!", &file_name, &open_mode, &use_include_path, &stream_context) == FAILURE) { RETURN_THROWS(); } intern->u.file.open_mode = zend_string_copy(open_mode); + /* file_name and zcontext are copied by spl_filesystem_file_open() */ + intern->file_name = file_name; + intern->u.file.zcontext = stream_context; /* spl_filesystem_file_open() can generate E_WARNINGs which we want to promote to exceptions */ zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling); @@ -2096,6 +2095,12 @@ PHP_METHOD(SplTempFileObject, __construct) RETURN_THROWS(); } + /* Prevent reinitialization of Object */ + if (intern->u.file.stream) { + zend_throw_error(NULL, "Cannot call constructor twice"); + RETURN_THROWS(); + } + if (max_memory < 0) { file_name = zend_string_init("php://memory", sizeof("php://memory")-1, 0); } else if (ZEND_NUM_ARGS()) { diff --git a/ext/spl/tests/gh16477-2.phpt b/ext/spl/tests/gh16477-2.phpt new file mode 100644 index 0000000000000..a51b9408c2d44 --- /dev/null +++ b/ext/spl/tests/gh16477-2.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-16477-2: Memory leak when calling SplTempFileObject::__constructor() twice +--FILE-- +__construct(); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +$obj->__debugInfo(); + +?> +DONE +--EXPECT-- +Error: Cannot call constructor twice +DONE diff --git a/ext/spl/tests/gh16477.phpt b/ext/spl/tests/gh16477.phpt new file mode 100644 index 0000000000000..f35c9538e8556 --- /dev/null +++ b/ext/spl/tests/gh16477.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-16477: Segmentation fault when calling __debugInfo() after failed SplFileObject::__constructor +--FILE-- +__construct(); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +$obj->__debugInfo(); + +?> +DONE +--EXPECT-- +ArgumentCountError: SplFileObject::__construct() expects at least 1 argument, 0 given +DONE From 5d993e9641cb197add86fa9088348763601050cb Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 17 Oct 2024 12:06:07 +0100 Subject: [PATCH 3/3] Fix GH-16477 (Segmentation fault when calling __debugInfo() after failed SplFileObject::__constructor) Closes GH-16480 Closes GH-16604 --- ext/spl/spl_directory.c | 19 ++++++++++++------- ext/spl/tests/gh16477-2.phpt | 19 +++++++++++++++++++ ext/spl/tests/gh16477.phpt | 19 +++++++++++++++++++ 3 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 ext/spl/tests/gh16477-2.phpt create mode 100644 ext/spl/tests/gh16477.phpt diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 10134c21e1e90..ca54db7ef3c9f 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -2016,22 +2016,21 @@ static void spl_filesystem_file_rewind(zval * this_ptr, spl_filesystem_object *i PHP_METHOD(SplFileObject, __construct) { spl_filesystem_object *intern = spl_filesystem_from_obj(Z_OBJ_P(ZEND_THIS)); + zend_string *file_name = NULL; zend_string *open_mode = ZSTR_CHAR('r'); + zval *stream_context = NULL; bool use_include_path = 0; size_t path_len; zend_error_handling error_handling; - intern->u.file.open_mode = ZSTR_CHAR('r'); - - if (zend_parse_parameters(ZEND_NUM_ARGS(), "P|Sbr!", - &intern->file_name, &open_mode, - &use_include_path, &intern->u.file.zcontext) == FAILURE) { - intern->u.file.open_mode = NULL; - intern->file_name = NULL; + if (zend_parse_parameters(ZEND_NUM_ARGS(), "P|Sbr!", &file_name, &open_mode, &use_include_path, &stream_context) == FAILURE) { RETURN_THROWS(); } intern->u.file.open_mode = zend_string_copy(open_mode); + /* file_name and zcontext are copied by spl_filesystem_file_open() */ + intern->file_name = file_name; + intern->u.file.zcontext = stream_context; /* spl_filesystem_file_open() can generate E_WARNINGs which we want to promote to exceptions */ zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling); @@ -2070,6 +2069,12 @@ PHP_METHOD(SplTempFileObject, __construct) RETURN_THROWS(); } + /* Prevent reinitialization of Object */ + if (intern->u.file.stream) { + zend_throw_error(NULL, "Cannot call constructor twice"); + RETURN_THROWS(); + } + if (max_memory < 0) { file_name = ZSTR_INIT_LITERAL("php://memory", 0); } else if (ZEND_NUM_ARGS()) { diff --git a/ext/spl/tests/gh16477-2.phpt b/ext/spl/tests/gh16477-2.phpt new file mode 100644 index 0000000000000..a51b9408c2d44 --- /dev/null +++ b/ext/spl/tests/gh16477-2.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-16477-2: Memory leak when calling SplTempFileObject::__constructor() twice +--FILE-- +__construct(); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +$obj->__debugInfo(); + +?> +DONE +--EXPECT-- +Error: Cannot call constructor twice +DONE diff --git a/ext/spl/tests/gh16477.phpt b/ext/spl/tests/gh16477.phpt new file mode 100644 index 0000000000000..f35c9538e8556 --- /dev/null +++ b/ext/spl/tests/gh16477.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-16477: Segmentation fault when calling __debugInfo() after failed SplFileObject::__constructor +--FILE-- +__construct(); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +$obj->__debugInfo(); + +?> +DONE +--EXPECT-- +ArgumentCountError: SplFileObject::__construct() expects at least 1 argument, 0 given +DONE