Fix phpGH-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 phpGH-17058.

Author: ndossche

NEWS

@@ -2,6 +2,9 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.3.16
 
+- Iconv:
+  . Fixed bug GH-17047 (UAF on iconv filter failure). (nielsdos)
+
 - Streams:
   . Fixed bug GH-17037 (UAF in user filter when adding existing filter name due
     to incorrect error handling). (nielsdos)

ext/iconv/iconv.c

@@ -2536,7 +2536,8 @@ static php_stream_filter_status_t php_iconv_stream_filter_do_filter(
 		if (php_iconv_stream_filter_append_bucket(self, stream, filter,
 				buckets_out, bucket->buf, bucket->buflen, &consumed,
 				php_stream_is_persistent(stream)) != SUCCESS) {
-			goto out_failure;
+			php_stream_bucket_delref(bucket);
+			return PSFS_ERR_FATAL;
 		}
 
 		php_stream_bucket_delref(bucket);
@@ -2546,7 +2547,7 @@ static php_stream_filter_status_t php_iconv_stream_filter_do_filter(
 		if (php_iconv_stream_filter_append_bucket(self, stream, filter,
 				buckets_out, NULL, 0, &consumed,
 				php_stream_is_persistent(stream)) != SUCCESS) {
-			goto out_failure;
+			return PSFS_ERR_FATAL;
 		}
 	}
 
@@ -2555,12 +2556,6 @@ static php_stream_filter_status_t php_iconv_stream_filter_do_filter(
 	}
 
 	return PSFS_PASS_ON;
-
-out_failure:
-	if (bucket != NULL) {
-		php_stream_bucket_delref(bucket);
-	}
-	return PSFS_ERR_FATAL;
 }
 /* }}} */
 

ext/iconv/tests/gh17047.phpt

@@ -0,0 +1,17 @@
+--TEST--
+GH-17047 (UAF on iconv filter failure)
+--EXTENSIONS--
+iconv
+--FILE--
+<?php
+$stream = fopen('php://temp', 'w+');
+stream_filter_append($stream, 'convert.iconv.UTF-16BE.UTF-8');
+stream_filter_append($stream, 'convert.iconv.UTF-16BE.UTF-16BE');
+fputs($stream, 'test');
+rewind($stream);
+var_dump(stream_get_contents($stream));
+fclose($stream);
+?>
+--EXPECTF--
+Warning: stream_get_contents(): iconv stream filter ("UTF-16BE"=>"UTF-16BE"): invalid multibyte sequence in %s on line %d
+string(0) ""