From ddbd396aa284297d2348c4ff1b6a36d7b61a557f Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Thu, 5 Dec 2024 21:13:55 +0100
Subject: [PATCH] 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.
---
 NEWS                         |  3 +++
 ext/iconv/iconv.c            | 11 +++--------
 ext/iconv/tests/gh17047.phpt | 17 +++++++++++++++++
 3 files changed, 23 insertions(+), 8 deletions(-)
 create mode 100644 ext/iconv/tests/gh17047.phpt

diff --git a/NEWS b/NEWS
index a139c14513a..4a86a80264c 100644
--- a/NEWS
+++ b/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)
diff --git a/ext/iconv/iconv.c b/ext/iconv/iconv.c
index 75f78dd1742..8ac3bc9b3c4 100644
--- a/ext/iconv/iconv.c
+++ b/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;
 }
 /* }}} */
 
diff --git a/ext/iconv/tests/gh17047.phpt b/ext/iconv/tests/gh17047.phpt
new file mode 100644
index 00000000000..a0307ddbe55
--- /dev/null
+++ b/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) ""