Skip to content

Commit ddbd396

Browse files
committed
Fix GH-17047: UAF on iconv filter failure
The first while loop sets the bucket variable, and this is freed in out_failure. However, when the second "goto out_failure" is triggered then bucket still refers to the bucket from the first while loop, causing a UAF. Fix this by separating the error paths. Closes GH-17058.
1 parent 6bac907 commit ddbd396

File tree

3 files changed

+23
-8
lines changed

3 files changed

+23
-8
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ PHP NEWS
22
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
33
?? ??? ????, PHP 8.3.16
44

5+
- Iconv:
6+
. Fixed bug GH-17047 (UAF on iconv filter failure). (nielsdos)
7+
58
- Streams:
69
. Fixed bug GH-17037 (UAF in user filter when adding existing filter name due
710
to incorrect error handling). (nielsdos)

ext/iconv/iconv.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2536,7 +2536,8 @@ static php_stream_filter_status_t php_iconv_stream_filter_do_filter(
25362536
if (php_iconv_stream_filter_append_bucket(self, stream, filter,
25372537
buckets_out, bucket->buf, bucket->buflen, &consumed,
25382538
php_stream_is_persistent(stream)) != SUCCESS) {
2539-
goto out_failure;
2539+
php_stream_bucket_delref(bucket);
2540+
return PSFS_ERR_FATAL;
25402541
}
25412542

25422543
php_stream_bucket_delref(bucket);
@@ -2546,7 +2547,7 @@ static php_stream_filter_status_t php_iconv_stream_filter_do_filter(
25462547
if (php_iconv_stream_filter_append_bucket(self, stream, filter,
25472548
buckets_out, NULL, 0, &consumed,
25482549
php_stream_is_persistent(stream)) != SUCCESS) {
2549-
goto out_failure;
2550+
return PSFS_ERR_FATAL;
25502551
}
25512552
}
25522553

@@ -2555,12 +2556,6 @@ static php_stream_filter_status_t php_iconv_stream_filter_do_filter(
25552556
}
25562557

25572558
return PSFS_PASS_ON;
2558-
2559-
out_failure:
2560-
if (bucket != NULL) {
2561-
php_stream_bucket_delref(bucket);
2562-
}
2563-
return PSFS_ERR_FATAL;
25642559
}
25652560
/* }}} */
25662561

ext/iconv/tests/gh17047.phpt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
GH-17047 (UAF on iconv filter failure)
3+
--EXTENSIONS--
4+
iconv
5+
--FILE--
6+
<?php
7+
$stream = fopen('php://temp', 'w+');
8+
stream_filter_append($stream, 'convert.iconv.UTF-16BE.UTF-8');
9+
stream_filter_append($stream, 'convert.iconv.UTF-16BE.UTF-16BE');
10+
fputs($stream, 'test');
11+
rewind($stream);
12+
var_dump(stream_get_contents($stream));
13+
fclose($stream);
14+
?>
15+
--EXPECTF--
16+
Warning: stream_get_contents(): iconv stream filter ("UTF-16BE"=>"UTF-16BE"): invalid multibyte sequence in %s on line %d
17+
string(0) ""

0 commit comments

Comments
 (0)