Skip to content

Commit 0f8079b

Browse files
iliaaliliaal
committed
Fix GH-22063: stream filter chain UAF on self-removal during callback
A php_user_filter that calls stream_filter_remove() from inside its own filter() callback frees the php_stream_filter struct while the chain walker in _php_stream_filter_flush, _php_stream_write_filtered, and _php_stream_fill_read_buffer still holds it via current->next. Carry an in_iteration counter on php_stream_filter_chain that the four dispatch sites bracket around their loops; stream_filter_remove() refuses with E_WARNING + FALSE while the counter is non-zero, which also covers a callback removing a different filter on the same chain. Fixes GH-22063
1 parent 7827754 commit 0f8079b

6 files changed

Lines changed: 112 additions & 2 deletions

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,8 @@ PHP NEWS
220220
. Improved stream_socket_server() bind failure error reporting. (ilutov)
221221
. Fixed bug #49874 (ftell() and fseek() inconsistency when using stream
222222
filters). (Jakub Zelenka)
223+
. Fixed bug GH-22063 (Stream filter chain UAF on self-removal during
224+
callback). (iliaal)
223225

224226
- Zip:
225227
. Fixed ZipArchive callback being called after executor has shut down.

‎ext/standard/streamsfuncs.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,11 @@ PHP_FUNCTION(stream_filter_remove)
13041304
RETURN_THROWS();
13051305
}
13061306

1307+
if (filter->chain && filter->chain->in_iteration > 0) {
1308+
php_error_docref(NULL, E_WARNING, "Cannot remove filter while it is being applied");
1309+
RETURN_FALSE;
1310+
}
1311+
13071312
if (php_stream_filter_flush(filter, 1) == FAILURE) {
13081313
php_error_docref(NULL, E_WARNING, "Unable to flush filter, not removing");
13091314
RETURN_FALSE;

‎ext/standard/tests/streams/gh22063.phpt‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
--TEST--
2+
GH-22063 (Stream filter chain UAF via self-removal during callback)
3+
--FILE--
4+
<?php
5+
class SelfRemovingFilter extends php_user_filter {
6+
public $stream;
7+
public static ?string $key = null;
8+
public static ?string $other_key = null;
9+
public static bool $on_closing_only = false;
10+
11+
public function filter($in, $out, &$consumed, $closing): int
12+
{
13+
while ($bucket = stream_bucket_make_writeable($in)) {
14+
$consumed += $bucket->datalen;
15+
stream_bucket_append($out, $bucket);
16+
}
17+
if (self::$key !== null && (!self::$on_closing_only || $closing)) {
18+
$res = $GLOBALS[self::$key];
19+
$other = self::$other_key !== null ? $GLOBALS[self::$other_key] : null;
20+
self::$key = null;
21+
self::$other_key = null;
22+
var_dump(stream_filter_remove($res));
23+
if ($other) {
24+
var_dump(stream_filter_remove($other));
25+
}
26+
}
27+
return PSFS_PASS_ON;
28+
}
29+
}
30+
31+
stream_filter_register('self-removing', SelfRemovingFilter::class);
32+
33+
echo "write side:\n";
34+
$f = fopen('php://memory', 'r+');
35+
SelfRemovingFilter::$key = 'write_res';
36+
SelfRemovingFilter::$on_closing_only = false;
37+
$GLOBALS['write_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_WRITE);
38+
fwrite($f, 'hello');
39+
fwrite($f, ' world');
40+
rewind($f);
41+
echo stream_get_contents($f), "\n";
42+
43+
echo "cross-filter side:\n";
44+
$f = fopen('php://memory', 'r+');
45+
SelfRemovingFilter::$key = 'cross_res';
46+
SelfRemovingFilter::$other_key = 'cross_other';
47+
SelfRemovingFilter::$on_closing_only = false;
48+
$GLOBALS['cross_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_WRITE);
49+
$GLOBALS['cross_other'] = stream_filter_append($f, 'string.rot13', STREAM_FILTER_WRITE);
50+
fwrite($f, 'hello world');
51+
rewind($f);
52+
echo stream_get_contents($f), "\n";
53+
54+
echo "read side:\n";
55+
$f = fopen('php://memory', 'r+');
56+
fwrite($f, 'abcdefghij');
57+
rewind($f);
58+
SelfRemovingFilter::$key = 'read_res';
59+
SelfRemovingFilter::$on_closing_only = false;
60+
$GLOBALS['read_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_READ);
61+
echo fread($f, 4), '|', fread($f, 6), "\n";
62+
63+
echo "closing-flush side:\n";
64+
$f = fopen('php://memory', 'r+');
65+
SelfRemovingFilter::$key = 'close_res';
66+
SelfRemovingFilter::$on_closing_only = true;
67+
$GLOBALS['close_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_WRITE);
68+
var_dump(stream_filter_remove($GLOBALS['close_res']));
69+
?>
70+
--EXPECTF--
71+
write side:
72+
73+
Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
74+
bool(false)
75+
hello world
76+
cross-filter side:
77+
78+
Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
79+
bool(false)
80+
81+
Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
82+
bool(false)
83+
uryyb jbeyq
84+
read side:
85+
86+
Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
87+
bool(false)
88+
abcd|efghij
89+
closing-flush side:
90+
91+
Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
92+
bool(false)
93+
bool(true)

‎main/streams/filter.c‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,9 @@ PHPAPI zend_result php_stream_filter_append_ex(php_stream_filter_chain *chain, p
372372

373373
bucket = php_stream_bucket_new(stream, (char*) stream->readbuf + stream->readpos, stream->writepos - stream->readpos, 0, 0);
374374
php_stream_bucket_append(brig_inp, bucket);
375+
chain->in_iteration++;
375376
status = filter->fops->filter(stream, filter, brig_inp, brig_outp, &consumed, PSFS_FLAG_NORMAL);
377+
chain->in_iteration--;
376378

377379
if (stream->readpos + consumed > (uint32_t)stream->writepos) {
378380
/* No behaving filter should cause this. */
@@ -465,15 +467,17 @@ PHPAPI zend_result _php_stream_filter_flush(php_stream_filter *filter, bool fini
465467
chain = filter->chain;
466468
stream = chain->stream;
467469

468-
for(current = filter; current; current = current->next) {
470+
chain->in_iteration++;
471+
for (current = filter; current; current = current->next) {
469472
php_stream_filter_status_t status;
470473

471474
status = current->fops->filter(stream, current, inp, outp, NULL, flags);
472475
if (status == PSFS_FEED_ME) {
473-
/* We've flushed the data far enough */
476+
chain->in_iteration--;
474477
return SUCCESS;
475478
}
476479
if (status == PSFS_ERR_FATAL) {
480+
chain->in_iteration--;
477481
return FAILURE;
478482
}
479483
/* Otherwise we have data available to PASS_ON
@@ -486,6 +490,7 @@ PHPAPI zend_result _php_stream_filter_flush(php_stream_filter *filter, bool fini
486490

487491
flags = PSFS_FLAG_NORMAL;
488492
}
493+
chain->in_iteration--;
489494

490495
/* Last filter returned data via PSFS_PASS_ON
491496
Do something with it */

‎main/streams/php_stream_filter_api.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ typedef struct _php_stream_filter_chain {
111111

112112
/* Owning stream */
113113
php_stream *stream;
114+
uint32_t in_iteration;
114115
} php_stream_filter_chain;
115116

116117
struct _php_stream_filter {

‎main/streams/streams.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,7 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
580580
}
581581

582582
/* wind the handle... */
583+
stream->readfilters.in_iteration++;
583584
for (filter = stream->readfilters.head; filter; filter = filter->next) {
584585
status = filter->fops->filter(stream, filter, brig_inp, brig_outp, NULL, flags);
585586

@@ -595,6 +596,7 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
595596
brig_outp = brig_swap;
596597
memset(brig_outp, 0, sizeof(*brig_outp));
597598
}
599+
stream->readfilters.in_iteration--;
598600

599601
switch (status) {
600602
case PSFS_PASS_ON:
@@ -1233,6 +1235,7 @@ static ssize_t _php_stream_write_filtered(php_stream *stream, const char *buf, s
12331235
php_stream_bucket_append(&brig_in, bucket);
12341236
}
12351237

1238+
stream->writefilters.in_iteration++;
12361239
for (php_stream_filter *filter = stream->writefilters.head; filter; filter = filter->next) {
12371240
/* for our return value, we are interested in the number of bytes consumed from
12381241
* the first filter in the chain */
@@ -1250,6 +1253,7 @@ static ssize_t _php_stream_write_filtered(php_stream *stream, const char *buf, s
12501253
brig_outp = brig_swap;
12511254
memset(brig_outp, 0, sizeof(*brig_outp));
12521255
}
1256+
stream->writefilters.in_iteration--;
12531257

12541258
switch (status) {
12551259
case PSFS_PASS_ON:

0 commit comments

Comments
 (0)