Skip to content

Commit 27f17c3

Browse files
ndosschedevnexen
andcommitted
Fix GH-20286: use-after-destroy during userland stream_close()
Closes GH-20493. Co-authored-by: David Carlier <devnexen@gmail.com>
1 parent 9149c35 commit 27f17c3

File tree

4 files changed

+60
-1
lines changed

4 files changed

+60
-1
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ PHP NEWS
66
. Sync all boost.context files with release 1.86.0. (mvorisek)
77
. Fixed bug GH-20435 (SensitiveParameter doesn't work for named argument
88
passing to variadic parameter). (ndossche)
9+
. Fixed bug GH-20286 (use-after-destroy during userland stream_close()).
10+
(ndossche, David Carlier)
911

1012
- Bz2:
1113
. Fix assertion failures resulting in crashes with stream filter

Zend/zend_list.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,21 +214,34 @@ void zend_init_rsrc_plist(void)
214214

215215
void zend_close_rsrc_list(HashTable *ht)
216216
{
217-
/* Reload ht->arData on each iteration, as it may be reallocated. */
218217
uint32_t i = ht->nNumUsed;
218+
uint32_t num = ht->nNumUsed;
219219

220220
retry:
221221
zend_try {
222222
while (i-- > 0) {
223+
/* Reload ht->arData on each iteration, as it may be reallocated. */
223224
zval *p = ZEND_HASH_ELEMENT(ht, i);
224225
if (Z_TYPE_P(p) != IS_UNDEF) {
225226
zend_resource *res = Z_PTR_P(p);
226227
if (res->type >= 0) {
227228
zend_resource_dtor(res);
229+
230+
if (UNEXPECTED(ht->nNumUsed != num)) {
231+
/* New resources were added, reloop from the start.
232+
* We need to keep the top->down order to avoid freeing resources
233+
* in use by the newly created resources. */
234+
i = num = ht->nNumUsed;
235+
}
228236
}
229237
}
230238
}
231239
} zend_catch {
240+
if (UNEXPECTED(ht->nNumUsed != num)) {
241+
/* See above */
242+
i = num = ht->nNumUsed;
243+
}
244+
232245
/* If we have bailed, we probably executed user code (e.g. user stream
233246
* API). Keep closing resources so they don't leak. User handlers must be
234247
* called now so they aren't called in zend_deactivate() on
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
GH-20286 use after destroy on userland stream_close
3+
--CREDITS--
4+
vi3tL0u1s
5+
--SKIPIF--
6+
<?php
7+
if (substr(PHP_OS, 0, 3) == 'WIN') die('skip Aborts with STATUS_BAD_FUNCTION_TABLE on Windows');
8+
?>
9+
--FILE--
10+
<?php
11+
class lib {
12+
public $context;
13+
function stream_set() {}
14+
function stream_set_option() {}
15+
function stream_stat() {
16+
return true;
17+
}
18+
function stream_open() {
19+
return true;
20+
}
21+
22+
function stream_read($count) {
23+
function a() {}
24+
include('lib://');
25+
}
26+
27+
function stream_close() {
28+
static $count = 0;
29+
if ($count++ < 3) // Prevent infinite loop
30+
include('lib://');
31+
}
32+
}
33+
stream_wrapper_register('lib', lib::class);
34+
include('lib://test.php');
35+
?>
36+
--EXPECTF--
37+
Fatal error: Cannot redeclare a() (previously declared in %s:%d) in %s on line %d
38+
39+
Fatal error: Cannot redeclare a() (previously declared in %s on line %d
40+
41+
Fatal error: Cannot redeclare a() (previously declared in %s on line %d
42+
43+
Fatal error: Cannot redeclare a() (previously declared in %s on line %d

main/streams/userspace.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ typedef struct _php_userstream_data php_userstream_data_t;
254254
static zend_result call_method_if_exists(
255255
zval *object, zval *method_name, zval *retval, uint32_t param_count, zval *params)
256256
{
257+
ZEND_ASSERT(EG(active));
257258
return zend_call_method_if_exists(
258259
Z_OBJ_P(object), Z_STR_P(method_name), retval, param_count, params);
259260
}

0 commit comments

Comments
 (0)