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"