From 225dae4ab511f6142fafb7a801fdb8ae14aa603f Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 21 Feb 2018 22:45:35 +0000 Subject: [PATCH] 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 --- includes/media/JpegMetadataExtractor.php | 2 +- tests/phpunit/data/media/jpeg-xmp-loop.jpg | Bin 0 -> 20 bytes .../includes/media/JpegMetadataExtractorTest.php | 6 ++++++ 3 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 tests/phpunit/data/media/jpeg-xmp-loop.jpg diff --git a/includes/media/JpegMetadataExtractor.php b/includes/media/JpegMetadataExtractor.php index 229a891db8..0bd01cd6c9 100644 --- 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 index 0000000000000000000000000000000000000000..962f3fe0e7bfa7e8a49d22ca29190cfbdf4bb6a0 GIT binary patch literal 20 acmex=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' ); + } } -- 2.20.1