Heap Buffer Overflow in iptcembed

[012git012]

Description

The following code (Terminal A):

$ rm /tmp/pipe.jpg
$ mkfifo /tmp/pipe.jpg
$ env USE_ZEND_ALLOC=0 ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 ./php -n -r '$d=str_repeat("A",1); var_dump(iptcembed($d, "/tmp/pipe.jpg"));'

The following code (Terminal B):

perl -e 'binmode STDOUT; print "\xFF\xD8"; print "\xFF\xE0\x00\x10","JFIF", "\x00"x9; print "\xFF\xDA\x00\x08", "\x00"x6; print "A" x (8*1024*1024);' > /tmp/pipe.jpg

Resulted in this output:

$ env USE_ZEND_ALLOC=0 ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 ./php -n -r '$d=str_repeat("A",1); var_dump(iptcembed($d, "/tmp/pipe.jpg"));'
=================================================================
==851236==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5190000013bf at pc 0x57ebf35b2e2d bp 0x7fffb5c5ee70 sp 0x7fffb5c5ee60
WRITE of size 1 at 0x5190000013bf thread T0
    #0 0x57ebf35b2e2c in php_iptc_get1 /home/administrator/php/php-src/ext/standard/iptc.c:102
    #1 0x57ebf35b2fd1 in php_iptc_next_marker /home/administrator/php/php-src/ext/standard/iptc.c:150
    #2 0x57ebf35b3dfb in zif_iptcembed /home/administrator/php/php-src/ext/standard/iptc.c:233
    #3 0x57ebf3a24aa8 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/administrator/php/php-src/Zend/zend_vm_execute.h:1421
    #4 0x57ebf3b89bf4 in execute_ex /home/administrator/php/php-src/Zend/zend_vm_execute.h:116441
    #5 0x57ebf3b9ed7f in zend_execute /home/administrator/php/php-src/Zend/zend_vm_execute.h:121924
    #6 0x57ebf39ed03c in zend_eval_stringl /home/administrator/php/php-src/Zend/zend_execute_API.c:1367
    #7 0x57ebf39ed4a2 in zend_eval_stringl_ex /home/administrator/php/php-src/Zend/zend_execute_API.c:1409
    #8 0x57ebf39ed53f in zend_eval_string_ex /home/administrator/php/php-src/Zend/zend_execute_API.c:1419
    #9 0x57ebf3d085d5 in do_cli /home/administrator/php/php-src/sapi/cli/php_cli.c:981
    #10 0x57ebf3d0a9e7 in main /home/administrator/php/php-src/sapi/cli/php_cli.c:1362
    #11 0x7bdace82a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #12 0x7bdace82a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #13 0x57ebf2a06ad4 in _start (/home/administrator/php/php-src/sapi/cli/php+0x606ad4) (BuildId: 23b3cb5fecdb3cc4703718b56e46f0909b26acaa)

0x5190000013bf is located 0 bytes after 1087-byte region [0x519000000f80,0x5190000013bf)
allocated by thread T0 here:
    #0 0x7bdaceefd9c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x57ebf38ea114 in __zend_malloc /home/administrator/php/php-src/Zend/zend_alloc.c:3543
    #2 0x57ebf38e5fec in _emalloc /home/administrator/php/php-src/Zend/zend_alloc.c:2780
    #3 0x57ebf38e6392 in _safe_emalloc /home/administrator/php/php-src/Zend/zend_alloc.c:2824
    #4 0x57ebf35b1d47 in zend_string_safe_alloc /home/administrator/php/php-src/Zend/zend_string.h:178
    #5 0x57ebf35b3ad8 in zif_iptcembed /home/administrator/php/php-src/ext/standard/iptc.c:211
    #6 0x57ebf3a24aa8 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/administrator/php/php-src/Zend/zend_vm_execute.h:1421
    #7 0x57ebf3b89bf4 in execute_ex /home/administrator/php/php-src/Zend/zend_vm_execute.h:116441
    #8 0x57ebf3b9ed7f in zend_execute /home/administrator/php/php-src/Zend/zend_vm_execute.h:121924
    #9 0x57ebf39ed03c in zend_eval_stringl /home/administrator/php/php-src/Zend/zend_execute_API.c:1367
    #10 0x57ebf39ed4a2 in zend_eval_stringl_ex /home/administrator/php/php-src/Zend/zend_execute_API.c:1409
    #11 0x57ebf39ed53f in zend_eval_string_ex /home/administrator/php/php-src/Zend/zend_execute_API.c:1419
    #12 0x57ebf3d085d5 in do_cli /home/administrator/php/php-src/sapi/cli/php_cli.c:981
    #13 0x57ebf3d0a9e7 in main /home/administrator/php/php-src/sapi/cli/php_cli.c:1362
    #14 0x7bdace82a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #15 0x7bdace82a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #16 0x57ebf2a06ad4 in _start (/home/administrator/php/php-src/sapi/cli/php+0x606ad4) (BuildId: 23b3cb5fecdb3cc4703718b56e46f0909b26acaa)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/administrator/php/php-src/ext/standard/iptc.c:102 in php_iptc_get1
Shadow bytes around the buggy address:
  0x519000001100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x519000001180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x519000001200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x519000001280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x519000001300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x519000001380: 00 00 00 00 00 00 00[07]fa fa fa fa fa fa fa fa
  0x519000001400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x519000001480: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x519000001500: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x519000001580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x519000001600: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==851236==ABORTING
Aborted (core dumped)

Build configuration:

$ ./configure CFLAGS="-fsanitize=address -g" CXXFLAGS="-fsanitize=address -g" LDFLAGS="-fsanitize=address" --enable-debug

Researcher:
Nikita Sveshnikov (Positive Technologies)

PHP Version

PHP 8.6.0-dev (cli) (built: Nov 18 2025 10:57:54) (NTS DEBUG)
Copyright (c) The PHP Group
Zend Engine v4.6.0-dev, Copyright (c) Zend Technologies
    with Zend OPcache v8.6.0-dev, Copyright (c), by Zend Technologies

Operating System

Ubuntu 24.04.2 LTS

[ndossche]

It's a form of TOC-TOU: the buffer is allocated based on the initial size. However, writing more data to the file (or pipe in this case) will make sure that getc() will not return EOF before going over the initial size.
An extra bounds check is probably the right way to go.

[ndossche]

I propose this, but will need to port the test code to PHP:

diff --git a/ext/standard/iptc.c b/ext/standard/iptc.c
index 44dd33bab10..0f46ecd19bc 100644
--- 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 {