Fix infinite loop in JpegMetadataExtractor

One of the skip-over loops was missing an feof() check and could
cause infinite loops.

Includes test file created by truncating a tiny tiny .jpeg at
the right place...

With the fix, it doesn't loop but dies on an exception, which
is good!

Bug: T184048
Change-Id: Ica13d6b68c3c12f7ce414edd081bf0886714e465

diff --git a/includes/media/JpegMetadataExtractor.php b/includes/media/JpegMetadataExtractor.php
index 229a891..0bd01cd 100644 (file)
--- a/includes/media/JpegMetadataExtractor.php
+++ b/includes/media/JpegMetadataExtractor.php
@@ -82,7 +82,7 @@ class JpegMetadataExtractor {
                                // this is just a sanity check
                                throw new MWException( 'Too many jpeg segments. Aborting' );
                        }
-                       while ( $buffer !== "\xFF" ) {
+                       while ( $buffer !== "\xFF" && !feof( $fh ) ) {
                                // In theory JPEG files are not allowed to contain anything between the sections,
                                // but in practice they sometimes do. It's customary to ignore the garbage data.
                                $buffer = fread( $fh, 1 );

diff --git a/tests/phpunit/data/media/jpeg-xmp-loop.jpg b/tests/phpunit/data/media/jpeg-xmp-loop.jpg
new file mode 100644 (file)
index 0000000..962f3fe
Binary files /dev/null and b/tests/phpunit/data/media/jpeg-xmp-loop.jpg differ

diff --git a/tests/phpunit/includes/media/JpegMetadataExtractorTest.php b/tests/phpunit/includes/media/JpegMetadataExtractorTest.php
index 0991254..9792172 100644 (file)
--- a/tests/phpunit/includes/media/JpegMetadataExtractorTest.php
+++ b/tests/phpunit/includes/media/JpegMetadataExtractorTest.php
@@ -108,4 +108,10 @@ class JpegMetadataExtractorTest extends MediaWikiTestCase {
                $expected = 'BE';
                $this->assertEquals( $expected, $res['byteOrder'] );
        }
+
+       public function testInfiniteRead() {
+               // Should get past infinite loop and throw in wfUnpack()
+               $this->setExpectedException( 'MWException' );
+               $res = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-xmp-loop.jpg' );
+       }
 }