From d148af82baf1ab19044ea344c3d45c725d6245d4 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Fri, 25 Sep 2026 00:58:57 -0400 Subject: [PATCH] ext/dba: Fix request memory kept in persistent dba handles A dba_popen() handle outlives the request, but flatfile kept the buffer of the last returned key, inifile its current and last-fetched lines, and cdb_make its hash list in it as emalloc'd memory, which the next request or the close at shutdown then freed or read. flatfile now frees the key right away and uses its file position as the iteration marker; inifile and cdb_make allocate what they keep with the persistence of the handle. --- NEWS | 2 + ext/dba/dba_flatfile.c | 34 +++++------ ext/dba/libcdb/cdb_make.c | 4 +- ext/dba/libflatfile/flatfile.c | 2 + ext/dba/libflatfile/flatfile.h | 1 - ext/dba/libinifile/inifile.c | 56 +++++++++++++---- .../tests/dba_cdb_make_persistent_insert.phpt | 60 +++++++++++++++++++ .../dba_flatfile_persistent_iteration.phpt | 51 ++++++++++++++++ .../dba_inifile_persistent_iteration.phpt | 53 ++++++++++++++++ 9 files changed, 228 insertions(+), 35 deletions(-) create mode 100644 ext/dba/tests/dba_cdb_make_persistent_insert.phpt create mode 100644 ext/dba/tests/dba_flatfile_persistent_iteration.phpt create mode 100644 ext/dba/tests/dba_inifile_persistent_iteration.phpt diff --git a/NEWS b/NEWS index daf52ae14d95..45f5252843fe 100644 --- a/NEWS +++ b/NEWS @@ -70,6 +70,8 @@ PHP NEWS - DBA: . Fixed bug GH-24641 (ndbm backend never checks dbm_open() return value, NULL deref on first use after a failed open). (Girgias) + . Fixed a crash with persistent flatfile, inifile, and cdb_make handles. + (Ilia Alshanetsky) - DOM: . Fixed a use-after-free when reflected attribute setters replace an diff --git a/ext/dba/dba_flatfile.c b/ext/dba/dba_flatfile.c index 997e6b9cc112..ca312db329e0 100644 --- a/ext/dba/dba_flatfile.c +++ b/ext/dba/dba_flatfile.c @@ -46,9 +46,6 @@ DBA_CLOSE_FUNC(flatfile) { flatfile *dba = info->dbf; - if (dba->nextkey.dptr) { - efree(dba->nextkey.dptr); - } pefree(dba, info->flags&DBA_PERSISTENT); } @@ -123,36 +120,33 @@ DBA_DELETE_FUNC(flatfile) return(flatfile_delete(dba, gkey) == -1 ? FAILURE : SUCCESS); } +static zend_string *flatfile_key_to_string(datum key) +{ + if (!key.dptr) { + return NULL; + } + + zend_string *str = zend_string_init(key.dptr, key.dsize, false); + efree(key.dptr); + return str; +} + DBA_FIRSTKEY_FUNC(flatfile) { flatfile *dba = info->dbf; - if (dba->nextkey.dptr) { - efree(dba->nextkey.dptr); - } - dba->nextkey = flatfile_firstkey(dba); - if (dba->nextkey.dptr) { - return zend_string_init(dba->nextkey.dptr, dba->nextkey.dsize, /* persistent */ false); - } - return NULL; + return flatfile_key_to_string(flatfile_firstkey(dba)); } DBA_NEXTKEY_FUNC(flatfile) { flatfile *dba = info->dbf; - if (!dba->nextkey.dptr) { + if (!dba->CurrentFlatFilePos) { return NULL; } - if (dba->nextkey.dptr) { - efree(dba->nextkey.dptr); - } - dba->nextkey = flatfile_nextkey(dba); - if (dba->nextkey.dptr) { - return zend_string_init(dba->nextkey.dptr, dba->nextkey.dsize, /* persistent */ false); - } - return NULL; + return flatfile_key_to_string(flatfile_nextkey(dba)); } DBA_OPTIMIZE_FUNC(flatfile) diff --git a/ext/dba/libcdb/cdb_make.c b/ext/dba/libcdb/cdb_make.c index 570e3625b0ce..d7ba71a7ea56 100644 --- a/ext/dba/libcdb/cdb_make.c +++ b/ext/dba/libcdb/cdb_make.c @@ -77,7 +77,7 @@ int cdb_make_addend(struct cdb_make *c, unsigned int keylen, unsigned int datale head = c->head; if (!head || (head->num >= CDB_HPLIST)) { - head = (struct cdb_hplist *) emalloc(sizeof(struct cdb_hplist)); + head = pemalloc(sizeof(struct cdb_hplist), php_stream_is_persistent(c->fp)); if (!head) return -1; head->num = 0; @@ -221,7 +221,7 @@ int cdb_make_finish(struct cdb_make *c) for (x = c->head; x; c->head = x) { x = x->next; - efree(c->head); + pefree(c->head, php_stream_is_persistent(c->fp)); } if (php_stream_flush(c->fp) != 0) diff --git a/ext/dba/libflatfile/flatfile.c b/ext/dba/libflatfile/flatfile.c index a68fed7e38c4..24cc291ffb62 100644 --- a/ext/dba/libflatfile/flatfile.c +++ b/ext/dba/libflatfile/flatfile.c @@ -226,6 +226,7 @@ datum flatfile_firstkey(flatfile *dba) { num = php_stream_read(dba->fp, buf, num); } efree(buf); + dba->CurrentFlatFilePos = 0; res.dptr = NULL; res.dsize = 0; return res; @@ -265,6 +266,7 @@ datum flatfile_nextkey(flatfile *dba) { } } efree(buf); + dba->CurrentFlatFilePos = 0; res.dptr = NULL; res.dsize = 0; return res; diff --git a/ext/dba/libflatfile/flatfile.h b/ext/dba/libflatfile/flatfile.h index 801ec8054f4f..13ec99d5fe28 100644 --- a/ext/dba/libflatfile/flatfile.h +++ b/ext/dba/libflatfile/flatfile.h @@ -27,7 +27,6 @@ typedef struct { int lockfd; php_stream *fp; size_t CurrentFlatFilePos; - datum nextkey; } flatfile; #define FLATFILE_INSERT 1 diff --git a/ext/dba/libinifile/inifile.c b/ext/dba/libinifile/inifile.c index 4dc687ea6681..8920b25900aa 100644 --- a/ext/dba/libinifile/inifile.c +++ b/ext/dba/libinifile/inifile.c @@ -76,6 +76,40 @@ void inifile_line_free(line_type *ln) } /* }}} */ +static void inifile_line_release(line_type *ln, bool persistent) +{ + if (ln->key.group) { + pefree(ln->key.group, persistent); + } + if (ln->key.name) { + pefree(ln->key.name, persistent); + } + if (ln->val.value) { + pefree(ln->val.value, persistent); + } + memset(ln, 0, sizeof(line_type)); +} + +static void inifile_line_store(line_type *dst, line_type *src, bool persistent) +{ + inifile_line_release(dst, persistent); + if (!persistent) { + *dst = *src; + return; + } + if (src->key.group) { + dst->key.group = pestrdup(src->key.group, true); + } + if (src->key.name) { + dst->key.name = pestrdup(src->key.name, true); + } + if (src->val.value) { + dst->val.value = pestrdup(src->val.value, true); + } + dst->pos = src->pos; + inifile_line_free(src); +} + /* {{{ inifile_alloc */ inifile * inifile_alloc(php_stream *fp, int readonly, int persistent) { @@ -100,8 +134,8 @@ inifile * inifile_alloc(php_stream *fp, int readonly, int persistent) void inifile_free(inifile *dba, int persistent) { if (dba) { - inifile_line_free(&dba->curr); - inifile_line_free(&dba->next); + inifile_line_release(&dba->curr, persistent); + inifile_line_release(&dba->next, persistent); pefree(dba, persistent); } } @@ -251,7 +285,7 @@ val_type inifile_fetch(inifile *dba, const key_type *key, int skip) { /* specific instance or not same key -> restart search */ /* the slow way: restart and seacrch */ php_stream_rewind(dba->fp); - inifile_line_free(&dba->next); + inifile_line_release(&dba->next, php_stream_is_persistent(dba->fp)); } if (skip == -1) { skip = 0; @@ -261,9 +295,8 @@ val_type inifile_fetch(inifile *dba, const key_type *key, int skip) { if (!skip) { val.value = estrdup(ln.val.value ? ln.val.value : ""); /* allow faster access by updating key read into next */ - inifile_line_free(&dba->next); - dba->next = ln; - dba->next.pos = php_stream_tell(dba->fp); + ln.pos = php_stream_tell(dba->fp); + inifile_line_store(&dba->next, &ln, php_stream_is_persistent(dba->fp)); return val; } skip--; @@ -282,7 +315,7 @@ val_type inifile_fetch(inifile *dba, const key_type *key, int skip) { /* {{{ inifile_firstkey */ int inifile_firstkey(inifile *dba) { - inifile_line_free(&dba->curr); + inifile_line_release(&dba->curr, php_stream_is_persistent(dba->fp)); dba->curr.pos = 0; return inifile_nextkey(dba); } @@ -296,9 +329,8 @@ int inifile_nextkey(inifile *dba) { php_stream_seek(dba->fp, dba->curr.pos, SEEK_SET); ln.key.group = estrdup(dba->curr.key.group ? dba->curr.key.group : ""); inifile_read(dba, &ln); - inifile_line_free(&dba->curr); - dba->curr = ln; - return ln.key.group || ln.key.name; + inifile_line_store(&dba->curr, &ln, php_stream_is_persistent(dba->fp)); + return dba->curr.key.group || dba->curr.key.name; } /* }}} */ @@ -325,8 +357,8 @@ static int inifile_find_group(inifile *dba, const key_type *key, size_t *pos_grp php_stream_flush(dba->fp); php_stream_seek(dba->fp, 0, SEEK_SET); - inifile_line_free(&dba->curr); - inifile_line_free(&dba->next); + inifile_line_release(&dba->curr, php_stream_is_persistent(dba->fp)); + inifile_line_release(&dba->next, php_stream_is_persistent(dba->fp)); if (key->group && strlen(key->group)) { int res; diff --git a/ext/dba/tests/dba_cdb_make_persistent_insert.phpt b/ext/dba/tests/dba_cdb_make_persistent_insert.phpt new file mode 100644 index 000000000000..1b9d8bc2895d --- /dev/null +++ b/ext/dba/tests/dba_cdb_make_persistent_insert.phpt @@ -0,0 +1,60 @@ +--TEST-- +dba_popen() cdb_make handle inserted into in consecutive requests +--EXTENSIONS-- +dba +--SKIPIF-- + +--CONFLICTS-- +server +--ENV-- +USE_ZEND_ALLOC=0 +USE_TRACKED_ALLOC=1 +--FILE-- +processHandle, 2); +for ($i = 0; $i < 100 && proc_get_status($info->processHandle)['running']; $i++) { + usleep(50000); +} + +$db = dba_open($db_file, 'r-', 'cdb'); +var_dump(dba_fetch('key1', $db), dba_fetch('key2', $db)); +dba_close($db); +?> +--CLEAN-- + +--EXPECT-- +Request 1: +bool(true) +Request 2: +bool(true) +string(6) "value1" +string(6) "value2" diff --git a/ext/dba/tests/dba_flatfile_persistent_iteration.phpt b/ext/dba/tests/dba_flatfile_persistent_iteration.phpt new file mode 100644 index 000000000000..10d8b9b67b9b --- /dev/null +++ b/ext/dba/tests/dba_flatfile_persistent_iteration.phpt @@ -0,0 +1,51 @@ +--TEST-- +dba_popen() flatfile handle iterated in consecutive requests +--EXTENSIONS-- +dba +--SKIPIF-- + +--CONFLICTS-- +server +--ENV-- +USE_ZEND_ALLOC=0 +USE_TRACKED_ALLOC=1 +--FILE-- + +--CLEAN-- + +--EXPECT-- +Request 1: +string(4) "key1" +string(4) "key2" +Request 2: +string(4) "key1" +string(4) "key2" diff --git a/ext/dba/tests/dba_inifile_persistent_iteration.phpt b/ext/dba/tests/dba_inifile_persistent_iteration.phpt new file mode 100644 index 000000000000..0f13d49ccb5b --- /dev/null +++ b/ext/dba/tests/dba_inifile_persistent_iteration.phpt @@ -0,0 +1,53 @@ +--TEST-- +dba_popen() inifile handle iterated in consecutive requests +--EXTENSIONS-- +dba +--SKIPIF-- + +--CONFLICTS-- +server +--ENV-- +USE_ZEND_ALLOC=0 +USE_TRACKED_ALLOC=1 +--FILE-- + +--CLEAN-- + +--EXPECT-- +Request 1: +string(6) "value2" +string(4) "key1" +string(4) "key2" +Request 2: +string(6) "value2" +string(4) "key1" +string(4) "key2"