Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

PNGMetadataExtractor: skip oversize chunks instead of aborting

Browse source 
Bug: T286273
Change-Id: Iceaf92647e74a1e20f94fc36822d0735f70764dc
This commit is contained in:
Tim Starling 2021-07-28 13:31:56 +10:00
parent 2f69c21326
commit d0d73ff1f9
2 changed files with 38 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
includes/media/PNGMetadataExtractor.php
View file

@ -106,8 +106,13 @@ class PNGMetadataExtractor {
$buf = self::read( $fh, 4 ); $buf = self::read( $fh, 4 );
$chunk_size = unpack( "N", $buf )[1]; $chunk_size = unpack( "N", $buf )[1];
if ( $chunk_size < 0 ) { if ( $chunk_size < 0 || $chunk_size > self::MAX_CHUNK_SIZE ) {
throw new Exception( __METHOD__ . ": Chunk size too big for unpack" ); 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 ); $chunk_type = self::read( $fh, 4 );
@ -399,10 +404,6 @@ class PNGMetadataExtractor {
* @return string The chunk. * @return string The chunk.
*/ */
private static function read( $fh, $size ) { 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 ) { if ( $size === 0 ) {
return ''; return '';
} }

31
tests/phpunit/includes/media/PNGMetadataExtractorTest.php
View file

@ -144,4 +144,35 @@ class PNGMetadataExtractorTest extends MediaWikiIntegrationTestCase {
$this->assertEquals( 10, $meta['width'] ); $this->assertEquals( 10, $meta['width'] );
$this->assertEquals( 10, $meta['height'] ); $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 ) ) );
}
} }