From f744c827a881deb248897640b6637f5f94e52971 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 4 Sep 2025 20:47:43 +0200 Subject: [PATCH] Fix GH-19706: dba stream resource mismanagement This regressed in 8.4 when dba started mixing objects and resources (streams). The streams are first destroyed at a first step in shutdown, and in slow shutdown then the symbol table is destroyed which destroys the dba objects. The dba objects still use the streams but they have been destroyed already, causing a UAF. Using dtor_obj instead of free_obj would work around this but would cause issues like memory leaks because dtor_obj may be skipped while free_obj may not be. Instead, use the same solution as mysqlnd uses in that we fully manage the stream lifecycle ourselves. This also avoids users from meddling with the stream through get_resources(). This would be fixed 'automatically' in the future when we are using objects for everything. Closes GH-19710. --- NEWS | 3 +++ ext/dba/dba.c | 24 ++++++++++++++++++++++-- ext/dba/tests/gh19706.phpt | 20 ++++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 ext/dba/tests/gh19706.phpt diff --git a/NEWS b/NEWS index 5dde4ba35aa..c50857a68ab 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,9 @@ PHP NEWS . Fixed date_sunrise() and date_sunset() with partial-hour UTC offset. (ilutov) +- DBA: + . Fixed bug GH-19706 (dba stream resource mismanagement). (nielsdos) + - DOM: . Fixed bug GH-19612 (Mitigate libxml2 tree dictionary bug). (nielsdos) diff --git a/ext/dba/dba.c b/ext/dba/dba.c index 7982e4255b0..0944e6e0b5f 100644 --- a/ext/dba/dba.c +++ b/ext/dba/dba.c @@ -256,14 +256,14 @@ static void dba_close_info(dba_info *info) if (info->flags & DBA_PERSISTENT) { php_stream_pclose(info->fp); } else { - php_stream_close(info->fp); + php_stream_free(info->fp, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_RSRC_DTOR); } } if (info->lock.fp) { if (info->flags & DBA_PERSISTENT) { php_stream_pclose(info->lock.fp); } else { - php_stream_close(info->lock.fp); + php_stream_free(info->lock.fp, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_RSRC_DTOR); } } @@ -518,6 +518,17 @@ static zend_always_inline zend_string *php_dba_zend_string_dup_safe(zend_string } } +/* See mysqlnd_fixup_regular_list */ +static void php_dba_fixup_regular_list(php_stream *stream) +{ + dtor_func_t origin_dtor = EG(regular_list).pDestructor; + EG(regular_list).pDestructor = NULL; + zend_hash_index_del(&EG(regular_list), stream->res->handle); + EG(regular_list).pDestructor = origin_dtor; + efree(stream->res); + stream->res = NULL; +} + /* {{{ php_dba_open */ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent) { @@ -831,6 +842,9 @@ restart: /* do not log errors for .lck file while in read only mode on .lck file */ lock_file_mode = "rb"; connection->info->lock.fp = php_stream_open_wrapper(lock_name, lock_file_mode, STREAM_MUST_SEEK|IGNORE_PATH|persistent_flag, &opened_path); + if (connection->info->lock.fp && !persistent_flag) { + php_dba_fixup_regular_list(connection->info->lock.fp); + } if (opened_path) { zend_string_release_ex(opened_path, 0); } @@ -844,6 +858,9 @@ restart: zend_string *opened_path = NULL; connection->info->lock.fp = php_stream_open_wrapper(lock_name, lock_file_mode, STREAM_MUST_SEEK|REPORT_ERRORS|IGNORE_PATH|persistent_flag, &opened_path); if (connection->info->lock.fp) { + if (!persistent_flag) { + php_dba_fixup_regular_list(connection->info->lock.fp); + } if (is_db_lock) { if (opened_path) { /* replace the path info with the real path of the opened file */ @@ -880,6 +897,9 @@ restart: connection->info->fp = connection->info->lock.fp; /* use the same stream for locking and database access */ } else { connection->info->fp = php_stream_open_wrapper(ZSTR_VAL(connection->info->path), file_mode, STREAM_MUST_SEEK|REPORT_ERRORS|IGNORE_PATH|persistent_flag, NULL); + if (connection->info->fp && !persistent_flag) { + php_dba_fixup_regular_list(connection->info->fp); + } } if (!connection->info->fp) { /* stream operation already wrote an error message */ diff --git a/ext/dba/tests/gh19706.phpt b/ext/dba/tests/gh19706.phpt new file mode 100644 index 00000000000..4cf3ef5f54d --- /dev/null +++ b/ext/dba/tests/gh19706.phpt @@ -0,0 +1,20 @@ +--TEST-- +GH-19706 (dba stream resource mismanagement) +--EXTENSIONS-- +dba +--FILE-- + +--CLEAN-- + +--EXPECT-- +object(Dba\Connection)#1 (0) { +} +object(Dba\Connection)#1 (0) { +}