diff --git a/NEWS b/NEWS index 65b1f85ba378..e112f49a4326 100644 --- a/NEWS +++ b/NEWS @@ -220,6 +220,8 @@ PHP NEWS . Improved stream_socket_server() bind failure error reporting. (ilutov) . Fixed bug #49874 (ftell() and fseek() inconsistency when using stream filters). (Jakub Zelenka) + . Fixed bug GH-22063 (Stream filter chain UAF on self-removal during + callback). (iliaal) - Zip: . Fixed ZipArchive callback being called after executor has shut down. diff --git a/ext/standard/streamsfuncs.c b/ext/standard/streamsfuncs.c index 68f79783c6f8..ca97b3a5c67f 100644 --- a/ext/standard/streamsfuncs.c +++ b/ext/standard/streamsfuncs.c @@ -1304,6 +1304,11 @@ PHP_FUNCTION(stream_filter_remove) RETURN_THROWS(); } + if (filter->chain && filter->chain->in_iteration > 0) { + php_error_docref(NULL, E_WARNING, "Cannot remove filter while it is being applied"); + RETURN_FALSE; + } + if (php_stream_filter_flush(filter, 1) == FAILURE) { php_error_docref(NULL, E_WARNING, "Unable to flush filter, not removing"); RETURN_FALSE; diff --git a/ext/standard/tests/streams/gh22063.phpt b/ext/standard/tests/streams/gh22063.phpt new file mode 100644 index 000000000000..d2044d95b844 --- /dev/null +++ b/ext/standard/tests/streams/gh22063.phpt @@ -0,0 +1,93 @@ +--TEST-- +GH-22063 (Stream filter chain UAF via self-removal during callback) +--FILE-- +datalen; + stream_bucket_append($out, $bucket); + } + if (self::$key !== null && (!self::$on_closing_only || $closing)) { + $res = $GLOBALS[self::$key]; + $other = self::$other_key !== null ? $GLOBALS[self::$other_key] : null; + self::$key = null; + self::$other_key = null; + var_dump(stream_filter_remove($res)); + if ($other) { + var_dump(stream_filter_remove($other)); + } + } + return PSFS_PASS_ON; + } +} + +stream_filter_register('self-removing', SelfRemovingFilter::class); + +echo "write side:\n"; +$f = fopen('php://memory', 'r+'); +SelfRemovingFilter::$key = 'write_res'; +SelfRemovingFilter::$on_closing_only = false; +$GLOBALS['write_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_WRITE); +fwrite($f, 'hello'); +fwrite($f, ' world'); +rewind($f); +echo stream_get_contents($f), "\n"; + +echo "cross-filter side:\n"; +$f = fopen('php://memory', 'r+'); +SelfRemovingFilter::$key = 'cross_res'; +SelfRemovingFilter::$other_key = 'cross_other'; +SelfRemovingFilter::$on_closing_only = false; +$GLOBALS['cross_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_WRITE); +$GLOBALS['cross_other'] = stream_filter_append($f, 'string.rot13', STREAM_FILTER_WRITE); +fwrite($f, 'hello world'); +rewind($f); +echo stream_get_contents($f), "\n"; + +echo "read side:\n"; +$f = fopen('php://memory', 'r+'); +fwrite($f, 'abcdefghij'); +rewind($f); +SelfRemovingFilter::$key = 'read_res'; +SelfRemovingFilter::$on_closing_only = false; +$GLOBALS['read_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_READ); +echo fread($f, 4), '|', fread($f, 6), "\n"; + +echo "closing-flush side:\n"; +$f = fopen('php://memory', 'r+'); +SelfRemovingFilter::$key = 'close_res'; +SelfRemovingFilter::$on_closing_only = true; +$GLOBALS['close_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_WRITE); +var_dump(stream_filter_remove($GLOBALS['close_res'])); +?> +--EXPECTF-- +write side: + +Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d +bool(false) +hello world +cross-filter side: + +Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d +bool(false) + +Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d +bool(false) +uryyb jbeyq +read side: + +Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d +bool(false) +abcd|efghij +closing-flush side: + +Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d +bool(false) +bool(true) diff --git a/main/streams/filter.c b/main/streams/filter.c index 3a19f5ce918a..4635b531d9a8 100644 --- a/main/streams/filter.c +++ b/main/streams/filter.c @@ -372,7 +372,9 @@ PHPAPI zend_result php_stream_filter_append_ex(php_stream_filter_chain *chain, p bucket = php_stream_bucket_new(stream, (char*) stream->readbuf + stream->readpos, stream->writepos - stream->readpos, 0, 0); php_stream_bucket_append(brig_inp, bucket); + chain->in_iteration++; status = filter->fops->filter(stream, filter, brig_inp, brig_outp, &consumed, PSFS_FLAG_NORMAL); + chain->in_iteration--; if (stream->readpos + consumed > (uint32_t)stream->writepos) { /* No behaving filter should cause this. */ @@ -465,15 +467,17 @@ PHPAPI zend_result _php_stream_filter_flush(php_stream_filter *filter, bool fini chain = filter->chain; stream = chain->stream; - for(current = filter; current; current = current->next) { + chain->in_iteration++; + for (current = filter; current; current = current->next) { php_stream_filter_status_t status; status = current->fops->filter(stream, current, inp, outp, NULL, flags); if (status == PSFS_FEED_ME) { - /* We've flushed the data far enough */ + chain->in_iteration--; return SUCCESS; } if (status == PSFS_ERR_FATAL) { + chain->in_iteration--; return FAILURE; } /* 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 flags = PSFS_FLAG_NORMAL; } + chain->in_iteration--; /* Last filter returned data via PSFS_PASS_ON Do something with it */ diff --git a/main/streams/php_stream_filter_api.h b/main/streams/php_stream_filter_api.h index 111127a8ad1a..a7d75635d299 100644 --- a/main/streams/php_stream_filter_api.h +++ b/main/streams/php_stream_filter_api.h @@ -111,6 +111,7 @@ typedef struct _php_stream_filter_chain { /* Owning stream */ php_stream *stream; + uint32_t in_iteration; } php_stream_filter_chain; struct _php_stream_filter { diff --git a/main/streams/streams.c b/main/streams/streams.c index e638c52159a6..4e0a94219bab 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -580,6 +580,7 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size) } /* wind the handle... */ + stream->readfilters.in_iteration++; for (filter = stream->readfilters.head; filter; filter = filter->next) { status = filter->fops->filter(stream, filter, brig_inp, brig_outp, NULL, flags); @@ -595,6 +596,7 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size) brig_outp = brig_swap; memset(brig_outp, 0, sizeof(*brig_outp)); } + stream->readfilters.in_iteration--; switch (status) { case PSFS_PASS_ON: @@ -1233,6 +1235,7 @@ static ssize_t _php_stream_write_filtered(php_stream *stream, const char *buf, s php_stream_bucket_append(&brig_in, bucket); } + stream->writefilters.in_iteration++; for (php_stream_filter *filter = stream->writefilters.head; filter; filter = filter->next) { /* for our return value, we are interested in the number of bytes consumed from * the first filter in the chain */ @@ -1250,6 +1253,7 @@ static ssize_t _php_stream_write_filtered(php_stream *stream, const char *buf, s brig_outp = brig_swap; memset(brig_outp, 0, sizeof(*brig_outp)); } + stream->writefilters.in_iteration--; switch (status) { case PSFS_PASS_ON: