Skip to content

Commit 77f2d12

Browse files
alexandre-dauboisalexandre-daubois
authored
Fix phpGH-20370: forbid user stream filters to violate typed property constraints (php#20373)
1 parent a960edc commit 77f2d12

6 files changed

Lines changed: 203 additions & 9 deletions

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ PHP NEWS
176176
- Streams:
177177
. Fixed bug GH-19798: XP_SOCKET XP_SSL (Socket stream modules): Incorrect
178178
condition for Win32/Win64. (Jakub Zelenka)
179+
. Fixed bug GH-20370 (User stream filters could violate typed property
180+
constraints). (alexandre-daubois)
179181

180182
- Tidy:
181183
. Fixed GH-19021 (improved tidyOptGetCategory detection).

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
GH-20370 (User filters should respect typed properties)
3+
--FILE--
4+
<?php
5+
6+
class pass_filter
7+
{
8+
public $filtername;
9+
public $params;
10+
public int $stream = 1;
11+
12+
function filter($in, $out, &$consumed, $closing): int
13+
{
14+
while ($bucket = stream_bucket_make_writeable($in)) {
15+
$consumed += $bucket->datalen;
16+
stream_bucket_append($out, $bucket);
17+
}
18+
return PSFS_PASS_ON;
19+
}
20+
}
21+
22+
stream_filter_register("pass", "pass_filter");
23+
$fp = fopen("php://memory", "w");
24+
stream_filter_append($fp, "pass");
25+
26+
try {
27+
fwrite($fp, "data");
28+
} catch (TypeError $e) {
29+
echo $e::class, ": ", $e->getMessage(), "\n";
30+
}
31+
32+
try {
33+
fclose($fp);
34+
} catch (TypeError $e) {
35+
echo $e::class, ": ", $e->getMessage(), "\n";
36+
}
37+
38+
unset($fp); // prevent cleanup at shutdown
39+
40+
?>
41+
--EXPECTF--
42+
Warning: fwrite(): Unprocessed filter buckets remaining on input brigade in %s on line %d
43+
TypeError: Cannot assign resource to property pass_filter::$stream of type int
44+
TypeError: Cannot assign resource to property pass_filter::$stream of type int

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
--TEST--
2+
GH-20370 (User filters should update dynamic stream property if it exists)
3+
--FILE--
4+
<?php
5+
6+
#[\AllowDynamicProperties]
7+
class pass_filter
8+
{
9+
public $filtername;
10+
public $params;
11+
12+
function onCreate(): bool
13+
{
14+
$this->stream = null;
15+
return true;
16+
}
17+
18+
function filter($in, $out, &$consumed, $closing): int
19+
{
20+
while ($bucket = stream_bucket_make_writeable($in)) {
21+
$consumed += $bucket->datalen;
22+
stream_bucket_append($out, $bucket);
23+
}
24+
var_dump(property_exists($this, 'stream'));
25+
if (is_resource($this->stream)) {
26+
var_dump(get_resource_type($this->stream));
27+
}
28+
return PSFS_PASS_ON;
29+
}
30+
}
31+
32+
stream_filter_register("pass", "pass_filter");
33+
$fp = fopen("php://memory", "w");
34+
stream_filter_append($fp, "pass");
35+
36+
fwrite($fp, "data");
37+
rewind($fp);
38+
echo fread($fp, 1024) . "\n";
39+
40+
?>
41+
--EXPECTF--
42+
bool(true)
43+
string(6) "stream"
44+
bool(true)
45+
string(6) "stream"
46+
bool(true)
47+
string(6) "stream"
48+
bool(true)
49+
string(6) "stream"
50+
data
51+
bool(true)

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
GH-20370 (User filters should not create stream property if not declared)
3+
--FILE--
4+
<?php
5+
6+
class pass_filter
7+
{
8+
public $filtername;
9+
public $params;
10+
11+
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+
18+
var_dump(property_exists($this, 'stream'));
19+
return PSFS_PASS_ON;
20+
}
21+
}
22+
23+
stream_filter_register("pass", "pass_filter");
24+
$fp = fopen("php://memory", "w");
25+
stream_filter_append($fp, "pass");
26+
fwrite($fp, "data");
27+
rewind($fp);
28+
echo fread($fp, 1024) . "\n";
29+
30+
?>
31+
--EXPECT--
32+
bool(false)
33+
bool(false)
34+
bool(false)
35+
bool(false)
36+
data
37+
bool(false)

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
GH-20370 (User filters should handle private stream property correctly)
3+
--FILE--
4+
<?php
5+
6+
class pass_filter
7+
{
8+
public $filtername;
9+
public $params;
10+
private $stream;
11+
12+
function filter($in, $out, &$consumed, $closing): int
13+
{
14+
while ($bucket = stream_bucket_make_writeable($in)) {
15+
$consumed += $bucket->datalen;
16+
stream_bucket_append($out, $bucket);
17+
}
18+
return PSFS_PASS_ON;
19+
}
20+
21+
function onClose()
22+
{
23+
var_dump($this->stream); // should be null
24+
}
25+
}
26+
27+
stream_filter_register("pass", "pass_filter");
28+
$fp = fopen("php://memory", "w");
29+
stream_filter_append($fp, "pass", STREAM_FILTER_WRITE);
30+
31+
fwrite($fp, "data");
32+
rewind($fp);
33+
echo fread($fp, 1024) . "\n";
34+
35+
?>
36+
--EXPECT--
37+
data
38+
NULL

‎ext/standard/user_filters.c‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,31 @@ php_stream_filter_status_t userfilter_filter(
147147
uint32_t orig_no_fclose = stream->flags & PHP_STREAM_FLAG_NO_FCLOSE;
148148
stream->flags |= PHP_STREAM_FLAG_NO_FCLOSE;
149149

150-
zval *stream_prop = zend_hash_str_find_ind(Z_OBJPROP_P(obj), "stream", sizeof("stream")-1);
151-
if (stream_prop) {
152-
/* Give the userfilter class a hook back to the stream */
153-
zval_ptr_dtor(stream_prop);
154-
php_stream_to_zval(stream, stream_prop);
155-
Z_ADDREF_P(stream_prop);
150+
/* Give the userfilter class a hook back to the stream */
151+
zend_class_entry *old_scope = EG(fake_scope);
152+
EG(fake_scope) = Z_OBJCE_P(obj);
153+
154+
zend_string *stream_name = ZSTR_INIT_LITERAL("stream", 0);
155+
bool stream_property_exists = Z_OBJ_HT_P(obj)->has_property(Z_OBJ_P(obj), stream_name, ZEND_PROPERTY_EXISTS, NULL);
156+
if (stream_property_exists) {
157+
zval stream_zval;
158+
php_stream_to_zval(stream, &stream_zval);
159+
zend_update_property_ex(Z_OBJCE_P(obj), Z_OBJ_P(obj), stream_name, &stream_zval);
160+
/* If property update threw an exception, skip filter execution */
161+
if (EG(exception)) {
162+
EG(fake_scope) = old_scope;
163+
if (buckets_in->head) {
164+
php_error_docref(NULL, E_WARNING, "Unprocessed filter buckets remaining on input brigade");
165+
}
166+
zend_string_release(stream_name);
167+
stream->flags &= ~PHP_STREAM_FLAG_NO_FCLOSE;
168+
stream->flags |= orig_no_fclose;
169+
return PSFS_ERR_FATAL;
170+
}
156171
}
157172

173+
EG(fake_scope) = old_scope;
174+
158175
ZVAL_STRINGL(&func_name, "filter", sizeof("filter")-1);
159176

160177
/* Setup calling arguments */
@@ -195,11 +212,16 @@ php_stream_filter_status_t userfilter_filter(
195212

196213
/* filter resources are cleaned up by the stream destructor,
197214
* keeping a reference to the stream resource here would prevent it
198-
* from being destroyed properly */
199-
if (stream_prop) {
200-
convert_to_null(stream_prop);
215+
* from being destroyed properly.
216+
* Since the property accepted a resource assignment above, it must have
217+
* no type hint or be typed as mixed, so we can safely assign null.
218+
*/
219+
if (stream_property_exists) {
220+
zend_update_property_null(Z_OBJCE_P(obj), Z_OBJ_P(obj), ZSTR_VAL(stream_name), ZSTR_LEN(stream_name));
201221
}
202222

223+
zend_string_release(stream_name);
224+
203225
zval_ptr_dtor(&args[3]);
204226
zval_ptr_dtor(&args[2]);
205227
zval_ptr_dtor(&args[1]);

0 commit comments

Comments
 (0)