diff --git a/includes/media/PNGMetadataExtractor.php b/includes/media/PNGMetadataExtractor.php index 7f184259338..d18a05a3717 100644 --- a/includes/media/PNGMetadataExtractor.php +++ b/includes/media/PNGMetadataExtractor.php @@ -106,8 +106,13 @@ class PNGMetadataExtractor { $buf = self::read( $fh, 4 ); $chunk_size = unpack( "N", $buf )[1]; - if ( $chunk_size < 0 ) { - throw new Exception( __METHOD__ . ": Chunk size too big for unpack" ); + if ( $chunk_size < 0 || $chunk_size > self::MAX_CHUNK_SIZE ) { + wfDebug( __METHOD__ . ': Chunk size of ' . $chunk_size . + ' too big, skipping. Max size is: ' . self::MAX_CHUNK_SIZE ); + if ( fseek( $fh, 4 + $chunk_size + self::$crcSize, SEEK_CUR ) !== 0 ) { + throw new Exception( __METHOD__ . ': seek error' ); + } + continue; } $chunk_type = self::read( $fh, 4 ); @@ -399,10 +404,6 @@ class PNGMetadataExtractor { * @return string The chunk. */ private static function read( $fh, $size ) { - if ( $size > self::MAX_CHUNK_SIZE ) { - throw new Exception( __METHOD__ . ': Chunk size of ' . $size . - ' too big. Max size is: ' . self::MAX_CHUNK_SIZE ); - } if ( $size === 0 ) { return ''; } diff --git a/tests/phpunit/includes/media/PNGMetadataExtractorTest.php b/tests/phpunit/includes/media/PNGMetadataExtractorTest.php index 568a8267c26..c547325c5cc 100644 --- a/tests/phpunit/includes/media/PNGMetadataExtractorTest.php +++ b/tests/phpunit/includes/media/PNGMetadataExtractorTest.php @@ -144,4 +144,35 @@ class PNGMetadataExtractorTest extends MediaWikiIntegrationTestCase { $this->assertEquals( 10, $meta['width'] ); $this->assertEquals( 10, $meta['height'] ); } + + /** + * T286273 -- oversize chunk + */ + public function testPngOversizeChunk() { + // Write a temporary file consisting of a normal PNG plus an extra tEXt chunk. + // Try to hold the chunk in memory only once. + $path = $this->getNewTempFile(); + copy( $this->filePath . '1bit-png.png', $path ); + $chunkTypeAndData = "tEXtkey\0value" . str_repeat( '.', 10000000 ); + $crc = crc32( $chunkTypeAndData ); + $chunkLength = strlen( $chunkTypeAndData ) - 4; + $file = fopen( $path, 'r+' ); + fseek( $file, -12, SEEK_END ); + $iend = fread( $file, 12 ); + fseek( $file, -12, SEEK_END ); + fwrite( $file, pack( 'N', $chunkLength ) ); + fwrite( $file, $chunkTypeAndData ); + fwrite( $file, pack( 'N', $crc ) ); + fwrite( $file, $iend ); + fclose( $file ); + + // Extract the metadata + $meta = PNGMetadataExtractor::getMetadata( $path ); + $this->assertEquals( 50, $meta['width'] ); + $this->assertEquals( 50, $meta['height'] ); + + // Verify that the big chunk didn't end up in the metadata + $this->assertLessThan( 100000, strlen( serialize( $meta ) ) ); + } + }