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.

5 files changed, 51 insertions, 28 deletions

diff --git a/NEWS b/NEWS
index 7bf6fbf3f7..d91bdda54e 100644
--- a/NEWS
+++ b/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
 
diff --git a/ext/standard/image.c b/ext/standard/image.c
index f20482bde4..d58d543abd 100644
--- a/ext/standard/image.c
+++ b/ext/standard/image.c
@@ -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:
diff --git a/ext/standard/tests/image/bug13213.phpt b/ext/standard/tests/image/bug13213.phpt
index c97b7016b4..d6b82a4f0c 100644
--- a/ext/standard/tests/image/bug13213.phpt
+++ b/ext/standard/tests/image/bug13213.phpt
@@ -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)
diff --git a/ext/standard/tests/image/bug72278.jpg b/ext/standard/tests/image/bug72278.jpg
new file mode 100644
index 0000000000..2ef32410cf
--- /dev/null
+++ b/ext/standard/tests/image/bug72278.jpg
diff --git a/ext/standard/tests/image/bug72278.phpt b/ext/standard/tests/image/bug72278.phpt
new file mode 100644
index 0000000000..1ee194681f
--- /dev/null
+++ b/ext/standard/tests/image/bug72278.phpt
@@ -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===