Merge branch 'PHP-8.4' into PHP-8.5

* PHP-8.4: Fix GH-20582: Heap Buffer Overflow in iptcembed
2026-03-24 00:02:20 +01:00 · 2025-12-26 22:44:35 +01:00
parent 31ae40d5e1 48b4289dff
commit 4ed8fce458
3 changed files with 105 additions and 28 deletions
--- a/1
+++ b/1
@@ -56,6 +56,7 @@ PHP                                                                        NEWS
 - Standard:
  . Fix error check for proc_open() command. (ndossche)
  . Fix memory leak in mail() when header key is numeric. (Girgias)
+  . Fixed bug GH-20582 (Heap Buffer Overflow in iptcembed). (ndossche)

 - URI:
  . Fixed bug GH-20771 (Assertion failure when getUnicodeHost() returns
--- a/ext/standard/iptc.c
+++ b/ext/standard/iptc.c
@@ -73,19 +73,24 @@
 #define M_APP15 0xef

 /* {{{ php_iptc_put1 */
-static int php_iptc_put1(FILE *fp, int spool, unsigned char c, unsigned char **spoolbuf)
+static int php_iptc_put1(FILE *fp, int spool, unsigned char c, unsigned char **spoolbuf, const unsigned char *spoolbuf_end)
 {
 	if (spool > 0)
 		PUTC(c);

-	if (spoolbuf) *(*spoolbuf)++ = c;
+	if (spoolbuf) {
+		if (UNEXPECTED(*spoolbuf >= spoolbuf_end)) {
+			return EOF;
+		}
+		*(*spoolbuf)++ = c;
+	}

 	return c;
 }
 /* }}} */

 /* {{{ php_iptc_get1 */
-static int php_iptc_get1(FILE *fp, int spool, unsigned char **spoolbuf)
+static int php_iptc_get1(FILE *fp, int spool, unsigned char **spoolbuf, const unsigned char *spoolbuf_end)
 {
 	int c;
 	char cc;
@@ -99,66 +104,71 @@ static int php_iptc_get1(FILE *fp, int spool, unsigned char **spoolbuf)
 		PUTC(cc);
 	}

-	if (spoolbuf) *(*spoolbuf)++ = c;
+	if (spoolbuf) {
+		if (UNEXPECTED(*spoolbuf >= spoolbuf_end)) {
+			return EOF;
+		}
+		*(*spoolbuf)++ = c;
+	}

 	return c;
 }
 /* }}} */

 /* {{{ php_iptc_read_remaining */
-static int php_iptc_read_remaining(FILE *fp, int spool, unsigned char **spoolbuf)
+static int php_iptc_read_remaining(FILE *fp, int spool, unsigned char **spoolbuf, const unsigned char *spoolbuf_end)
 {
-	while (php_iptc_get1(fp, spool, spoolbuf) != EOF) continue;
+	while (php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end) != EOF) continue;

 	return M_EOI;
 }
 /* }}} */

 /* {{{ php_iptc_skip_variable */
-static int php_iptc_skip_variable(FILE *fp, int spool, unsigned char **spoolbuf)
+static int php_iptc_skip_variable(FILE *fp, int spool, unsigned char **spoolbuf, const unsigned char *spoolbuf_end)
 {
 	unsigned int  length;
 	int c1, c2;

-	if ((c1 = php_iptc_get1(fp, spool, spoolbuf)) == EOF) return M_EOI;
+	if ((c1 = php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end)) == EOF) return M_EOI;

-	if ((c2 = php_iptc_get1(fp, spool, spoolbuf)) == EOF) return M_EOI;
+	if ((c2 = php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end)) == EOF) return M_EOI;

 	length = (((unsigned char) c1) << 8) + ((unsigned char) c2);

 	length -= 2;

 	while (length--)
-		if (php_iptc_get1(fp, spool, spoolbuf) == EOF) return M_EOI;
+		if (php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end) == EOF) return M_EOI;

 	return 0;
 }
 /* }}} */

 /* {{{ php_iptc_next_marker */
-static int php_iptc_next_marker(FILE *fp, int spool, unsigned char **spoolbuf)
+static int php_iptc_next_marker(FILE *fp, int spool, unsigned char **spoolbuf, const unsigned char *spoolbuf_end)
 {
 	int c;

 	/* skip unimportant stuff */

-	c = php_iptc_get1(fp, spool, spoolbuf);
+	c = php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end);

 	if (c == EOF) return M_EOI;

 	while (c != 0xff) {
-		if ((c = php_iptc_get1(fp, spool, spoolbuf)) == EOF)
+		if ((c = php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end)) == EOF)
 			return M_EOI; /* we hit EOF */
 	}

 	/* get marker byte, swallowing possible padding */
 	do {
-		c = php_iptc_get1(fp, 0, 0);
+		c = php_iptc_get1(fp, 0, 0, NULL);
 		if (c == EOF)
 			return M_EOI;       /* we hit EOF */
 		else
 		if (c == 0xff)
-			php_iptc_put1(fp, spool, (unsigned char)c, spoolbuf);
+			php_iptc_put1(fp, spool, (unsigned char)c, spoolbuf, spoolbuf_end);
 	} while (c == 0xff);

 	return (unsigned int) c;
@@ -178,6 +188,7 @@ PHP_FUNCTION(iptcembed)
 	size_t inx;
 	zend_string *spoolbuf = NULL;
 	unsigned char *poi = NULL;
+	unsigned char *spoolbuf_end = NULL;
 	zend_stat_t sb = {0};
 	bool written = 0;

@@ -210,10 +221,11 @@ PHP_FUNCTION(iptcembed)

 		spoolbuf = zend_string_safe_alloc(1, iptcdata_len + sizeof(psheader) + 1024 + 1, sb.st_size, 0);
 		poi = (unsigned char*)ZSTR_VAL(spoolbuf);
+		spoolbuf_end = poi + ZSTR_LEN(spoolbuf);
 		memset(poi, 0, iptcdata_len + sizeof(psheader) + sb.st_size + 1024 + 1);
 	}

-	if (php_iptc_get1(fp, spool, poi?&poi:0) != 0xFF) {
+	if (php_iptc_get1(fp, spool, poi?&poi:0, spoolbuf_end) != 0xFF) {
 		fclose(fp);
 		if (spoolbuf) {
 			zend_string_efree(spoolbuf);
@@ -221,7 +233,8 @@ PHP_FUNCTION(iptcembed)
 		RETURN_FALSE;
 	}

-	if (php_iptc_get1(fp, spool, poi?&poi:0) != 0xD8) {
+	if (php_iptc_get1(fp, spool, poi?&poi:0, spoolbuf_end) != 0xD8) {
+err:
 		fclose(fp);
 		if (spoolbuf) {
 			zend_string_efree(spoolbuf);
@@ -230,20 +243,22 @@ PHP_FUNCTION(iptcembed)
 	}

 	while (!done) {
-		marker = php_iptc_next_marker(fp, spool, poi?&poi:0);
+		marker = php_iptc_next_marker(fp, spool, poi?&poi:0, spoolbuf_end);

 		if (marker == M_EOI) { /* EOF */
 			break;
 		} else if (marker != M_APP13) {
-			php_iptc_put1(fp, spool, (unsigned char)marker, poi?&poi:0);
+			if (php_iptc_put1(fp, spool, (unsigned char)marker, poi?&poi:0, spoolbuf_end) < 0) {
+				goto err;
+			}
 		}

 		switch (marker) {
 			case M_APP13:
 				/* we are going to write a new APP13 marker, so don't output the old one */
-				php_iptc_skip_variable(fp, 0, 0);
+				php_iptc_skip_variable(fp, 0, 0, spoolbuf_end);
 				fgetc(fp); /* skip already copied 0xFF byte */
-				php_iptc_read_remaining(fp, spool, poi?&poi:0);
+				php_iptc_read_remaining(fp, spool, poi?&poi:0, spoolbuf_end);
 				done = 1;
 				break;

@@ -256,7 +271,7 @@ PHP_FUNCTION(iptcembed)
 				}
 				written = 1;

-				php_iptc_skip_variable(fp, spool, poi?&poi:0);
+				php_iptc_skip_variable(fp, spool, poi?&poi:0, spoolbuf_end);

 				if (iptcdata_len & 1) {
 					iptcdata_len++; /* make the length even */
@@ -266,25 +281,33 @@ PHP_FUNCTION(iptcembed)
 				psheader[ 3 ] = (iptcdata_len+28)&0xff;

 				for (inx = 0; inx < 28; inx++) {
-					php_iptc_put1(fp, spool, psheader[inx], poi?&poi:0);
+					if (php_iptc_put1(fp, spool, psheader[inx], poi?&poi:0, spoolbuf_end) < 0) {
+						goto err;
+					}
 				}

-				php_iptc_put1(fp, spool, (unsigned char)(iptcdata_len>>8), poi?&poi:0);
-				php_iptc_put1(fp, spool, (unsigned char)(iptcdata_len&0xff), poi?&poi:0);
+				if (php_iptc_put1(fp, spool, (unsigned char)(iptcdata_len>>8), poi?&poi:0, spoolbuf_end) < 0) {
+					goto err;
+				}
+				if (php_iptc_put1(fp, spool, (unsigned char)(iptcdata_len&0xff), poi?&poi:0, spoolbuf_end) < 0) {
+					goto err;
+				}

 				for (inx = 0; inx < iptcdata_len; inx++) {
-					php_iptc_put1(fp, spool, iptcdata[inx], poi?&poi:0);
+					if (php_iptc_put1(fp, spool, iptcdata[inx], poi?&poi:0, spoolbuf_end) < 0) {
+						goto err;
+					}
 				}
 				break;

 			case M_SOS:
 				/* we hit data, no more marker-inserting can be done! */
-				php_iptc_read_remaining(fp, spool, poi?&poi:0);
+				php_iptc_read_remaining(fp, spool, poi?&poi:0, spoolbuf_end);
 				done = 1;
 				break;

 			default:
-				php_iptc_skip_variable(fp, spool, poi?&poi:0);
+				php_iptc_skip_variable(fp, spool, poi?&poi:0, spoolbuf_end);
 				break;
 		}
 	}
@@ -292,6 +315,7 @@ PHP_FUNCTION(iptcembed)
 	fclose(fp);

 	if (spool < 2) {
+		*poi = '\0';
 		spoolbuf = zend_string_truncate(spoolbuf, poi - (unsigned char*)ZSTR_VAL(spoolbuf), 0);
 		RETURN_NEW_STR(spoolbuf);
 	} else {
--- a/ext/standard/tests/image/gh20582.phpt
+++ b/ext/standard/tests/image/gh20582.phpt
@@ -0,0 +1,52 @@
+--TEST--
+GH-20582 (Heap Buffer Overflow in iptcembed)
+--CREDITS--
+Nikita Sveshnikov (Positive Technologies)
+ndossche
+--SKIPIF--
+<?php
+if (PHP_OS_FAMILY === "Windows") die("skip Only for platforms with FIFO pipes");
+?>
+--FILE--
+<?php
+
+$pipe = __DIR__.'/gh20582.pipe.jpg';
+
+// Create named pipe (FIFO)
+if (!file_exists($pipe)) {
+    if (!posix_mkfifo($pipe, 0666)) {
+        die("Failed to create FIFO\n");
+    }
+}
+
+$descriptorspec = array(
+   0 => STDIN,
+   1 => STDOUT,
+   2 => STDOUT,
+);
+$pipes = [];
+$proc = proc_open([PHP_BINARY, '-n', '-r', "var_dump(iptcembed('A', '$pipe'));"], $descriptorspec, $pipes);
+
+// Blocks until a reader opens it
+$fp = fopen($pipe, 'wb') or die("Failed to open FIFO");
+
+// Write header
+$data  = "\xFF\xD8";                    // SOI marker
+$data .= "\xFF\xE0\x00\x10";            // APP0 marker (JFIF)
+$data .= "JFIF" . str_repeat("\x00", 9);
+$data .= "\xFF\xDA\x00\x08";            // SOS marker
+$data .= str_repeat("\x00", 6);
+fwrite($fp, $data);
+
+// Write garbage
+fwrite($fp, str_repeat("A", 5120));
+
+fclose($fp);
+
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__.'/gh20582.pipe.jpg');
+?>
+--EXPECTF--
+string(1055) "ÿØÿà%0JFIF%0%0%0%0%0%0%0%0%0ÿÿí%0Photoshop 3.0%08BIM%0%0%0%0%0A%0Ú%0%0%0%0%0%0%0AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"