diff --git a/ext/standard/iptc.c b/ext/standard/iptc.c index 44dd33bab10a..0f46ecd19bc7 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 { diff --git a/ext/standard/tests/image/gh20582.phpt b/ext/standard/tests/image/gh20582.phpt new file mode 100644 index 000000000000..63561534b2fd --- /dev/null +++ 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-- + +--FILE-- + 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-- + +--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"