Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit f744c82

Browse files
committed
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.
1 parent c583124 commit f744c82

File tree

3 files changed

+45
-2
lines changed

3 files changed

+45
-2
lines changed

‎NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ PHP NEWS
2323
. Fixed date_sunrise() and date_sunset() with partial-hour UTC offset.
2424
(ilutov)
2525

26+
- DBA:
27+
. Fixed bug GH-19706 (dba stream resource mismanagement). (nielsdos)
28+
2629
- DOM:
2730
. Fixed bug GH-19612 (Mitigate libxml2 tree dictionary bug). (nielsdos)
2831

‎ext/dba/dba.c

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,14 +256,14 @@ static void dba_close_info(dba_info *info)
256256
if (info->flags & DBA_PERSISTENT) {
257257
php_stream_pclose(info->fp);
258258
} else {
259-
php_stream_close(info->fp);
259+
php_stream_free(info->fp, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_RSRC_DTOR);
260260
}
261261
}
262262
if (info->lock.fp) {
263263
if (info->flags & DBA_PERSISTENT) {
264264
php_stream_pclose(info->lock.fp);
265265
} else {
266-
php_stream_close(info->lock.fp);
266+
php_stream_free(info->lock.fp, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_RSRC_DTOR);
267267
}
268268
}
269269

@@ -518,6 +518,17 @@ static zend_always_inline zend_string *php_dba_zend_string_dup_safe(zend_string
518518
}
519519
}
520520

521+
/* See mysqlnd_fixup_regular_list */
522+
static void php_dba_fixup_regular_list(php_stream *stream)
523+
{
524+
dtor_func_t origin_dtor = EG(regular_list).pDestructor;
525+
EG(regular_list).pDestructor = NULL;
526+
zend_hash_index_del(&EG(regular_list), stream->res->handle);
527+
EG(regular_list).pDestructor = origin_dtor;
528+
efree(stream->res);
529+
stream->res = NULL;
530+
}
531+
521532
/* {{{ php_dba_open */
522533
static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
523534
{
@@ -831,6 +842,9 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
831842
/* do not log errors for .lck file while in read only mode on .lck file */
832843
lock_file_mode = "rb";
833844
connection->info->lock.fp = php_stream_open_wrapper(lock_name, lock_file_mode, STREAM_MUST_SEEK|IGNORE_PATH|persistent_flag, &opened_path);
845+
if (connection->info->lock.fp && !persistent_flag) {
846+
php_dba_fixup_regular_list(connection->info->lock.fp);
847+
}
834848
if (opened_path) {
835849
zend_string_release_ex(opened_path, 0);
836850
}
@@ -844,6 +858,9 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
844858
zend_string *opened_path = NULL;
845859
connection->info->lock.fp = php_stream_open_wrapper(lock_name, lock_file_mode, STREAM_MUST_SEEK|REPORT_ERRORS|IGNORE_PATH|persistent_flag, &opened_path);
846860
if (connection->info->lock.fp) {
861+
if (!persistent_flag) {
862+
php_dba_fixup_regular_list(connection->info->lock.fp);
863+
}
847864
if (is_db_lock) {
848865
if (opened_path) {
849866
/* replace the path info with the real path of the opened file */
@@ -880,6 +897,9 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
880897
connection->info->fp = connection->info->lock.fp; /* use the same stream for locking and database access */
881898
} else {
882899
connection->info->fp = php_stream_open_wrapper(ZSTR_VAL(connection->info->path), file_mode, STREAM_MUST_SEEK|REPORT_ERRORS|IGNORE_PATH|persistent_flag, NULL);
900+
if (connection->info->fp && !persistent_flag) {
901+
php_dba_fixup_regular_list(connection->info->fp);
902+
}
883903
}
884904
if (!connection->info->fp) {
885905
/* stream operation already wrote an error message */

‎ext/dba/tests/gh19706.phpt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
GH-19706 (dba stream resource mismanagement)
3+
--EXTENSIONS--
4+
dba
5+
--FILE--
6+
<?php
7+
// Must be in the global scope such that it's part of the symbol table cleanup
8+
$db = dba_open(__DIR__ . '/gh19706.cdb', 'n', 'cdb_make');
9+
$db2 = $db;
10+
var_dump($db, $db2);
11+
?>
12+
--CLEAN--
13+
<?php
14+
@unlink(__DIR__ . '/gh19706.cdb');
15+
?>
16+
--EXPECT--
17+
object(Dba\Connection)#1 (0) {
18+
}
19+
object(Dba\Connection)#1 (0) {
20+
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /