Skip to content

Fix GH-20582: Heap Buffer Overflow in iptcembed #20591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: PHP-8.3
Choose a base branch
from
+104 −28
Open

Fix GH-20582: Heap Buffer Overflow in iptcembed #20591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 52 additions & 28 deletions ext/standard/iptc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -210,18 +221,20 @@ 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);
}
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);
Expand All @@ -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;

Expand All @@ -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 */
Expand All @@ -266,32 +281,41 @@ 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;
}
}

fclose(fp);

if (spool < 2) {
*poi = '\0';
spoolbuf = zend_string_truncate(spoolbuf, poi - (unsigned char*)ZSTR_VAL(spoolbuf), 0);
RETURN_NEW_STR(spoolbuf);
} else {
Expand Down
52 changes: 52 additions & 0 deletions ext/standard/tests/image/gh20582.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
Loading