Skip to content
Permalink
Browse files

Fix #72278: getimagesize returning FALSE on valid jpg

getimagesize() is rather strict about the length of the marker payload data,
and fails if there are extraneous bytes before the next marker. Only a very
special case reported in bug #13213 is catered to.

libjpeg is rather resilient to such corrupted JPEG files, and raises a
recoverable error in this case. Other image processors also accept such
JPEG files, so we adapt getimagesize() to skip (but warn about) such
extraneous bytes.
  • Loading branch information...
cmb69 committed Aug 13, 2016
1 parent ae3b207 commit 82df4e263886a0da21a00c98189649287666ba5c
1 NEWS
@@ -11,6 +11,7 @@ PHP NEWS

- Standard:
. Fixed bug #72823 (strtr out-of-bound access). (cmb)
. Fixed bug #72278 (getimagesize returning FALSE on valid jpg). (cmb)

18 Aug 2016, PHP 5.6.25

@@ -374,48 +374,36 @@ static unsigned short php_read2(php_stream * stream TSRMLS_DC)

/* {{{ php_next_marker
* get next marker byte from file */
static unsigned int php_next_marker(php_stream * stream, int last_marker, int comment_correction, int ff_read TSRMLS_DC)
static unsigned int php_next_marker(php_stream * stream, int last_marker, int ff_read TSRMLS_DC)
{
int a=0, marker;

/* get marker byte, swallowing possible padding */
if (last_marker==M_COM && comment_correction) {
/* some software does not count the length bytes of COM section */
/* one company doing so is very much envolved in JPEG... so we accept too */
/* by the way: some of those companies changed their code now... */
comment_correction = 2;
} else {
last_marker = 0;
comment_correction = 0;
}
if (ff_read) {
a = 1; /* already read 0xff in filetype detection */
if (!ff_read) {
size_t extraneous = 0;

while ((marker = php_stream_getc(stream)) != 0xff) {
if (marker == EOF) {
return M_EOI;/* we hit EOF */
}
extraneous++;
}
if (extraneous) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "corrupt JPEG data: %zu extraneous bytes before marker", extraneous);
}
}
a = 1;
do {
if ((marker = php_stream_getc(stream)) == EOF)
{
return M_EOI;/* we hit EOF */
}
if (last_marker==M_COM && comment_correction>0)
{
if (marker != 0xFF)
{
marker = 0xff;
comment_correction--;
} else {
last_marker = M_PSEUDO; /* stop skipping non 0xff for M_COM */
}
}
a++;
} while (marker == 0xff);
if (a < 2)
{
return M_EOI; /* at least one 0xff is needed before marker code */
}
if ( last_marker==M_COM && comment_correction)
{
return M_EOI; /* ah illegal: char after COM section not 0xFF */
}
return (unsigned int)marker;
}
/* }}} */
@@ -478,7 +466,7 @@ static struct gfxinfo *php_handle_jpeg (php_stream * stream, zval *info TSRMLS_D
unsigned short length, ff_read=1;

for (;;) {
marker = php_next_marker(stream, marker, 1, ff_read TSRMLS_CC);
marker = php_next_marker(stream, marker, ff_read TSRMLS_CC);
ff_read = 0;
switch (marker) {
case M_SOF0:
@@ -4,7 +4,8 @@ Bug #13213 (GetImageSize and wrong JPEG Comments)
<?php
var_dump(GetImageSize(dirname(__FILE__).'/bug13213.jpg'));
?>
--EXPECT--
--EXPECTF--
Warning: getimagesize(): corrupt JPEG data: 2 extraneous bytes before marker in %s%ebug13213.php on line %d
array(7) {
[0]=>
int(1)
Binary file not shown.
@@ -0,0 +1,33 @@
--TEST--
Bug #72278 (getimagesize returning FALSE on valid jpg)
--SKIPIF--
<?php
if (!defined('IMAGETYPE_JPEG')) die('skip images of type JPEG not supported');
?>
--FILE--
<?php
define('FILENAME', __DIR__ . DIRECTORY_SEPARATOR . 'bug72278.jpg');
var_dump(getimagesize(FILENAME));
?>
===DONE===
--EXPECTF--

Warning: getimagesize(): corrupt JPEG data: 3 extraneous bytes before marker in %s%ebug72278.php on line %d
array(7) {
[0]=>
int(300)
[1]=>
int(300)
[2]=>
int(2)
[3]=>
string(24) "width="300" height="300""
["bits"]=>
int(8)
["channels"]=>
int(3)
["mime"]=>
string(10) "image/jpeg"
}
===DONE===

0 comments on commit 82df4e2

Please sign in to comment.
You can’t perform that action at this time.