Skip to content

Commit d67f39e

Browse files
EdmondDantesEdmondDantes
committed
Fix use-after-free when stream_filter_remove() runs inside a stream callback
stream_filter_remove() flushes the filter before it unlinks and frees it, and the flush can run PHP code: a user filter's filter() method, or a user wrapper's stream_write() for the flushed data. When that code removes the same filter, the outer call continues on freed memory. A user filter can also be removed from its own filter() method, or from other code while filter() is suspended in a Fiber; userfilter_filter() and the filter chain walks then use the freed filter after the callback returns. userfilter_filter() now counts the calls of each user filter in progress, a suspended Fiber included, in the u2 of the zval holding the filter object, so neither php_stream nor php_stream_filter changes. stream_filter_remove() refuses a filter whose count is not zero, before the flush and again after it, since a Fiber can enter the filter from the flush, and it checks after the flush that the filter is still there. Other filters of the stream can still be removed from a callback.
1 parent 357fcf1 commit d67f39e

9 files changed

Lines changed: 298 additions & 0 deletions

‎ext/standard/basic_functions.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ ZEND_API void php_get_highlight_struct(zend_syntax_highlighter_ini *syntax_highl
4444

4545
PHP_MINIT_FUNCTION(user_filters);
4646
PHP_RSHUTDOWN_FUNCTION(user_filters);
47+
/* Whether a call of this user filter is in progress; removing it then would free it under that call. */
48+
bool php_user_filter_is_running(const php_stream_filter *filter);
4749
PHP_RSHUTDOWN_FUNCTION(browscap);
4850

4951
/* Left for BC (not binary safe!) */

‎ext/standard/streamsfuncs.c‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1344,11 +1344,28 @@ PHP_FUNCTION(stream_filter_remove)
13441344
RETURN_THROWS();
13451345
}
13461346

1347+
if (php_user_filter_is_running(filter)) {
1348+
php_error_docref(NULL, E_WARNING, "Unable to remove a filter while it is running");
1349+
RETURN_FALSE;
1350+
}
1351+
13471352
if (php_stream_filter_flush(filter, 1) == FAILURE) {
13481353
php_error_docref(NULL, E_WARNING, "Unable to flush filter, not removing");
13491354
RETURN_FALSE;
13501355
}
13511356

1357+
/* The flush writes to the stream, which may run PHP code that removes this filter. */
1358+
if (Z_RES_TYPE_P(zfilter) != php_file_le_stream_filter()) {
1359+
php_error_docref(NULL, E_WARNING, "Filter has already been removed");
1360+
RETURN_FALSE;
1361+
}
1362+
1363+
/* A Fiber may have entered the filter during the flush and be suspended in it. */
1364+
if (php_user_filter_is_running(filter)) {
1365+
php_error_docref(NULL, E_WARNING, "Unable to remove a filter while it is running");
1366+
RETURN_FALSE;
1367+
}
1368+
13521369
zend_list_close(Z_RES_P(zfilter));
13531370
php_stream_filter_remove(filter, 1);
13541371
RETURN_TRUE;

‎ext/standard/tests/filters/stream_filter_remove_during_flush.phpt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
stream_filter_remove() of a filter that is removed while it is flushed
3+
--FILE--
4+
<?php
5+
class Sink {
6+
public $context;
7+
8+
public function stream_open($path, $mode, $options, &$opened_path): bool {
9+
return true;
10+
}
11+
12+
public function stream_write(string $data): int {
13+
if (isset($GLOBALS['remove'])) {
14+
$filter = $GLOBALS['remove'];
15+
unset($GLOBALS['remove']);
16+
var_dump(stream_filter_remove($filter));
17+
}
18+
19+
return strlen($data);
20+
}
21+
}
22+
23+
stream_wrapper_register('sink', 'Sink');
24+
25+
$stream = fopen('sink://', 'w');
26+
$filter = stream_filter_append($stream, 'convert.base64-encode', STREAM_FILTER_WRITE);
27+
fwrite($stream, 'ab');
28+
$GLOBALS['remove'] = $filter;
29+
var_dump(stream_filter_remove($filter));
30+
?>
31+
--EXPECTF--
32+
bool(true)
33+
34+
Warning: stream_filter_remove(): Filter has already been removed in %s on line %d
35+
bool(false)

‎ext/standard/tests/filters/stream_filter_remove_entered_during_flush.phpt‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
stream_filter_remove() of a user filter that a Fiber entered during its flush
3+
--FILE--
4+
<?php
5+
class Reader extends php_user_filter {
6+
public function filter($in, $out, &$consumed, bool $closing): int {
7+
global $stream, $fiber;
8+
9+
if ($closing && !$fiber) {
10+
$fiber = new Fiber(fn() => fread($stream, 1));
11+
$fiber->start();
12+
}
13+
14+
if ($fiber && Fiber::getCurrent() === $fiber) {
15+
Fiber::suspend();
16+
}
17+
18+
while ($bucket = stream_bucket_make_writeable($in)) {
19+
$consumed += $bucket->datalen;
20+
stream_bucket_append($out, $bucket);
21+
}
22+
23+
return PSFS_PASS_ON;
24+
}
25+
}
26+
27+
stream_filter_register('reader', 'Reader');
28+
29+
$stream = fopen('php://memory', 'w+');
30+
fwrite($stream, 'abc');
31+
rewind($stream);
32+
$filter = stream_filter_append($stream, 'reader', STREAM_FILTER_READ);
33+
34+
var_dump(stream_filter_remove($filter));
35+
$fiber->resume();
36+
var_dump($fiber->getReturn());
37+
var_dump(stream_filter_remove($filter));
38+
?>
39+
--EXPECTF--
40+
Warning: stream_filter_remove(): Unable to remove a filter while it is running in %s on line %d
41+
bool(false)
42+
string(1) "a"
43+
bool(true)

‎ext/standard/tests/filters/stream_filter_remove_in_filter.phpt‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
--TEST--
2+
stream_filter_remove() of a user filter while it is running
3+
--FILE--
4+
<?php
5+
class Remover extends php_user_filter {
6+
public function filter($in, $out, &$consumed, bool $closing): int {
7+
while ($bucket = stream_bucket_make_writeable($in)) {
8+
$consumed += $bucket->datalen;
9+
stream_bucket_append($out, $bucket);
10+
}
11+
12+
if ($closing && isset($GLOBALS['remove'])) {
13+
$filter = $GLOBALS['remove'];
14+
unset($GLOBALS['remove']);
15+
var_dump(stream_filter_remove($filter));
16+
}
17+
18+
if (Fiber::getCurrent()) {
19+
Fiber::suspend();
20+
}
21+
22+
return PSFS_PASS_ON;
23+
}
24+
}
25+
26+
stream_filter_register('remover', 'Remover');
27+
28+
echo "From the filter itself:\n";
29+
$stream = fopen('php://memory', 'w+');
30+
$filter = stream_filter_append($stream, 'remover', STREAM_FILTER_WRITE);
31+
fwrite($stream, 'hello');
32+
$GLOBALS['remove'] = $filter;
33+
var_dump(stream_filter_remove($filter));
34+
35+
echo "While the filter is suspended in a Fiber:\n";
36+
$stream = fopen('php://memory', 'w+');
37+
$filter = stream_filter_append($stream, 'remover', STREAM_FILTER_WRITE);
38+
$fiber = new Fiber(fn() => fwrite($stream, 'hello'));
39+
$fiber->start();
40+
var_dump(stream_filter_remove($filter));
41+
$fiber->resume();
42+
var_dump($fiber->getReturn());
43+
var_dump(stream_filter_remove($filter));
44+
?>
45+
--EXPECTF--
46+
From the filter itself:
47+
48+
Warning: stream_filter_remove(): Unable to remove a filter while it is running in %s on line %d
49+
bool(false)
50+
bool(true)
51+
While the filter is suspended in a Fiber:
52+
53+
Warning: stream_filter_remove(): Unable to remove a filter while it is running in %s on line %d
54+
bool(false)
55+
int(5)
56+
bool(true)

‎ext/standard/tests/filters/stream_filter_remove_in_multiple_fibers.phpt‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
stream_filter_remove() while the same user filter is suspended in several Fibers
3+
--FILE--
4+
<?php
5+
class Suspender extends php_user_filter {
6+
public function filter($in, $out, &$consumed, bool $closing): int {
7+
while ($bucket = stream_bucket_make_writeable($in)) {
8+
$consumed += $bucket->datalen;
9+
stream_bucket_append($out, $bucket);
10+
}
11+
12+
if (Fiber::getCurrent()) {
13+
Fiber::suspend();
14+
}
15+
16+
return PSFS_PASS_ON;
17+
}
18+
}
19+
20+
stream_filter_register('suspender', 'Suspender');
21+
22+
$stream = fopen('php://memory', 'w+');
23+
$filter = stream_filter_append($stream, 'suspender', STREAM_FILTER_WRITE);
24+
25+
$fiber1 = new Fiber(fn() => fwrite($stream, 'one'));
26+
$fiber2 = new Fiber(fn() => fwrite($stream, 'two'));
27+
$fiber1->start();
28+
$fiber2->start();
29+
$fiber1->resume();
30+
var_dump($fiber1->getReturn());
31+
32+
var_dump(stream_filter_remove($filter));
33+
34+
$fiber2->resume();
35+
var_dump($fiber2->getReturn());
36+
37+
var_dump(stream_filter_remove($filter));
38+
39+
rewind($stream);
40+
var_dump(stream_get_contents($stream));
41+
?>
42+
--EXPECTF--
43+
int(3)
44+
45+
Warning: stream_filter_remove(): Unable to remove a filter while it is running in %s on line %d
46+
bool(false)
47+
int(3)
48+
bool(true)
49+
string(6) "onetwo"

‎ext/standard/tests/filters/stream_filter_remove_in_multiple_fibers_reverse.phpt‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
stream_filter_remove() while the same user filter is suspended in several Fibers that finish in reverse order
3+
--FILE--
4+
<?php
5+
class Suspender extends php_user_filter {
6+
public function filter($in, $out, &$consumed, bool $closing): int {
7+
while ($bucket = stream_bucket_make_writeable($in)) {
8+
$consumed += $bucket->datalen;
9+
stream_bucket_append($out, $bucket);
10+
}
11+
12+
if (Fiber::getCurrent()) {
13+
Fiber::suspend();
14+
}
15+
16+
return PSFS_PASS_ON;
17+
}
18+
}
19+
20+
stream_filter_register('suspender', 'Suspender');
21+
22+
$stream = fopen('php://memory', 'w+');
23+
$filter = stream_filter_append($stream, 'suspender', STREAM_FILTER_WRITE);
24+
25+
$fiber1 = new Fiber(fn() => fwrite($stream, 'one'));
26+
$fiber2 = new Fiber(fn() => fwrite($stream, 'two'));
27+
$fiber1->start();
28+
$fiber2->start();
29+
$fiber2->resume();
30+
var_dump($fiber2->getReturn());
31+
32+
var_dump(stream_filter_remove($filter));
33+
34+
$fiber1->resume();
35+
var_dump($fiber1->getReturn());
36+
37+
var_dump(stream_filter_remove($filter));
38+
39+
rewind($stream);
40+
var_dump(stream_get_contents($stream));
41+
?>
42+
--EXPECTF--
43+
int(3)
44+
45+
Warning: stream_filter_remove(): Unable to remove a filter while it is running in %s on line %d
46+
bool(false)
47+
int(3)
48+
bool(true)
49+
string(6) "twoone"

‎ext/standard/tests/filters/stream_filter_remove_other_filter_in_filter.phpt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
stream_filter_remove() of another filter of the stream from a user filter's callback
3+
--FILE--
4+
<?php
5+
class Remover extends php_user_filter {
6+
public function filter($in, $out, &$consumed, bool $closing): int {
7+
if (isset($GLOBALS['remove'])) {
8+
$filter = $GLOBALS['remove'];
9+
unset($GLOBALS['remove']);
10+
var_dump(stream_filter_remove($filter));
11+
}
12+
13+
while ($bucket = stream_bucket_make_writeable($in)) {
14+
$consumed += $bucket->datalen;
15+
stream_bucket_append($out, $bucket);
16+
}
17+
18+
return PSFS_PASS_ON;
19+
}
20+
}
21+
22+
stream_filter_register('remover', 'Remover');
23+
24+
$stream = fopen('php://memory', 'w+');
25+
stream_filter_append($stream, 'remover', STREAM_FILTER_WRITE);
26+
$GLOBALS['remove'] = stream_filter_append($stream, 'string.toupper', STREAM_FILTER_WRITE);
27+
var_dump(fwrite($stream, 'hello'));
28+
29+
rewind($stream);
30+
var_dump(stream_get_contents($stream));
31+
?>
32+
--EXPECT--
33+
bool(true)
34+
int(5)
35+
string(5) "hello"

‎ext/standard/user_filters.c‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ static void userfilter_dtor(php_stream_filter *thisfilter)
123123
zval_ptr_dtor(obj);
124124
}
125125

126+
/* The calls of a user filter in progress, a Fiber suspended in one included. The u2 of the zval
127+
* holding the filter's object is unused, so php_stream_filter keeps its layout. */
128+
#define USERFILTER_CALLS(filter) Z_EXTRA((filter)->abstract)
129+
126130
static php_stream_filter_status_t userfilter_filter(
127131
php_stream *stream,
128132
php_stream_filter *thisfilter,
@@ -147,6 +151,7 @@ static php_stream_filter_status_t userfilter_filter(
147151
/* Make sure the stream is not closed while the filter callback executes. */
148152
uint32_t orig_no_fclose = stream->flags & PHP_STREAM_FLAG_NO_FCLOSE;
149153
stream->flags |= PHP_STREAM_FLAG_NO_FCLOSE;
154+
USERFILTER_CALLS(thisfilter)++;
150155

151156
/* Give the userfilter class a hook back to the stream */
152157
zend_class_entry *old_scope = EG(fake_scope);
@@ -171,6 +176,7 @@ static php_stream_filter_status_t userfilter_filter(
171176
zend_string_release(stream_name);
172177
stream->flags &= ~PHP_STREAM_FLAG_NO_FCLOSE;
173178
stream->flags |= orig_no_fclose;
179+
USERFILTER_CALLS(thisfilter)--;
174180
return PSFS_ERR_FATAL;
175181
}
176182
}
@@ -234,6 +240,7 @@ static php_stream_filter_status_t userfilter_filter(
234240

235241
stream->flags &= ~PHP_STREAM_FLAG_NO_FCLOSE;
236242
stream->flags |= orig_no_fclose;
243+
USERFILTER_CALLS(thisfilter)--;
237244

238245
return ret;
239246
}
@@ -244,6 +251,11 @@ static const php_stream_filter_ops userfilter_ops = {
244251
"user-filter"
245252
};
246253

254+
bool php_user_filter_is_running(const php_stream_filter *filter)
255+
{
256+
return filter->fops == &userfilter_ops && USERFILTER_CALLS(filter) > 0;
257+
}
258+
247259
static php_stream_filter *user_filter_factory_create(const char *filtername,
248260
zval *filterparams, uint8_t persistent)
249261
{

0 commit comments

Comments
 (0)