From 0a21e2de1242a08dda5d6d16cb1913ae7bc6b7ed Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Fri, 25 Feb 2011 04:51:17 +0000 Subject: [PATCH] * (bug 24230) Added JAR detection. ZIP archives containing a .class file will be rejected by default. Malformed ZIP archives will be rejected due to the danger of ambiguous parsing on the client side. * Removed the ZIP subtypes from $wgMimeTypeBlacklist, they no longer need to be there. * Added ZipDirectoryReader. Added some small ZIP files which are used to test its various error cases. Most were constructed with a hex editor. * Fixed getStatusArray() to return a consistent type regardless of whether the error message has parameters. This allows error messages with no parameters to work with the Status object conversion code in UploadBase::verifyFile(). --- RELEASE-NOTES | 4 + includes/AutoLoader.php | 1 + includes/DefaultSettings.php | 20 +- includes/Status.php | 2 +- includes/ZipDirectoryReader.php | 683 ++++++++++++++++++ includes/upload/UploadBase.php | 47 +- languages/messages/MessagesEn.php | 10 + maintenance/language/messages.inc | 9 + tests/phpunit/data/zip/cd-gap.zip | Bin 0 -> 182 bytes tests/phpunit/data/zip/cd-truncated.zip | Bin 0 -> 171 bytes .../phpunit/data/zip/class-trailing-null.zip | Bin 0 -> 173 bytes .../phpunit/data/zip/class-trailing-slash.zip | Bin 0 -> 173 bytes tests/phpunit/data/zip/class.zip | Bin 0 -> 173 bytes tests/phpunit/data/zip/empty.zip | Bin 0 -> 22 bytes tests/phpunit/data/zip/looks-like-zip64.zip | Bin 0 -> 173 bytes tests/phpunit/data/zip/nosig.zip | Bin 0 -> 173 bytes tests/phpunit/data/zip/split.zip | Bin 0 -> 196 bytes tests/phpunit/data/zip/trail.zip | Bin 0 -> 181 bytes .../phpunit/data/zip/wrong-cd-start-disk.zip | Bin 0 -> 173 bytes .../data/zip/wrong-central-entry-sig.zip | Bin 0 -> 173 bytes .../includes/ZipDirectoryReaderTest.php | 79 ++ 21 files changed, 838 insertions(+), 17 deletions(-) create mode 100644 includes/ZipDirectoryReader.php create mode 100644 tests/phpunit/data/zip/cd-gap.zip create mode 100644 tests/phpunit/data/zip/cd-truncated.zip create mode 100644 tests/phpunit/data/zip/class-trailing-null.zip create mode 100644 tests/phpunit/data/zip/class-trailing-slash.zip create mode 100644 tests/phpunit/data/zip/class.zip create mode 100644 tests/phpunit/data/zip/empty.zip create mode 100644 tests/phpunit/data/zip/looks-like-zip64.zip create mode 100644 tests/phpunit/data/zip/nosig.zip create mode 100644 tests/phpunit/data/zip/split.zip create mode 100644 tests/phpunit/data/zip/trail.zip create mode 100644 tests/phpunit/data/zip/wrong-cd-start-disk.zip create mode 100644 tests/phpunit/data/zip/wrong-central-entry-sig.zip create mode 100644 tests/phpunit/includes/ZipDirectoryReaderTest.php diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 52cc00a7d8..5ebd6df2c5 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -74,6 +74,10 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN * (bug 27159) Make email confirmation code expiration time configurable * CSS/JS for each user group is imported from MediaWiki:Sysop.js, MediaWiki:Autoconfirmed.css, etc. +* (bug 24230) Uploads of ZIP types, such as MS Office or OpenOffice can now be + safely enabled. A ZIP file reader was added which can scan a ZIP file for + potentially dangerous Java applets. This allows applets to be blocked + specifically, rather than all ZIP files being blocked. === Bug fixes in 1.18 === * (bug 23119) WikiError class and subclasses are now marked as deprecated diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 4b7c34642d..c43240b125 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -275,6 +275,7 @@ $wgAutoloadLocalClasses = array( 'XmlSelect' => 'includes/Xml.php', 'XmlTypeCheck' => 'includes/XmlTypeCheck.php', 'ZhClient' => 'includes/ZhClient.php', + 'ZipDirectoryReader' => 'includes/ZipDirectoryReader.php', # includes/api 'ApiBase' => 'includes/api/ApiBase.php', diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 3b2cf2114c..bdee3f984e 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -552,21 +552,15 @@ $wgMimeTypeBlacklist = array( 'text/scriptlet', 'application/x-msdownload', # Windows metafile, client-side vulnerability on some systems 'application/x-msmetafile', - # A ZIP file may be a valid Java archive containing an applet which exploits the - # same-origin policy to steal cookies - 'application/zip', - - # MS Office OpenXML and other Open Package Conventions files are zip files - # and thus blacklisted just as other zip files. If you remove these entries - # from the blacklist in your local configuration, a malicious file upload - # will be able to compromise the wiki's user accounts, and the user - # accounts of any other website in the same cookie domain. - 'application/x-opc+zip', - 'application/msword', - 'application/vnd.ms-powerpoint', - 'application/vnd.msexcel', ); +/** + * Allow Java archive uploads. + * This is not recommended for public wikis since a maliciously-constructed + * applet running on the same domain as the wiki can steal the user's cookies. + */ +$wgAllowJavaUploads = false; + /** * This is a flag to determine whether or not to check file extensions on upload. * diff --git a/includes/Status.php b/includes/Status.php index f285cf75ff..30cbb85179 100644 --- a/includes/Status.php +++ b/includes/Status.php @@ -281,7 +281,7 @@ class Status { if( $error['params'] ) { $result[] = array_merge( array( $error['message'] ), $error['params'] ); } else { - $result[] = $error['message']; + $result[] = array( $error['message'] ); } } } diff --git a/includes/ZipDirectoryReader.php b/includes/ZipDirectoryReader.php new file mode 100644 index 0000000000..85e5347192 --- /dev/null +++ b/includes/ZipDirectoryReader.php @@ -0,0 +1,683 @@ +execute(); + } + + /** The file name */ + var $fileName; + + /** The opened file resource */ + var $file; + + /** The cached length of the file, or null if it has not been loaded yet. */ + var $fileLength; + + /** A segmented cache of the file contents */ + var $buffer; + + /** The file data callback */ + var $callback; + + /** The ZIP64 mode */ + var $zip64 = false; + + /** Stored headers */ + var $eocdr, $eocdr64, $eocdr64Locator; + + /** The "extra field" ID for ZIP64 central directory entries */ + const ZIP64_EXTRA_HEADER = 0x0001; + + /** The segment size for the file contents cache */ + const SEGSIZE = 16384; + + /** The index of the "general field" bit for UTF-8 file names */ + const GENERAL_UTF8 = 11; + + /** The index of the "general field" bit for central directory encryption */ + const GENERAL_CD_ENCRYPTED = 13; + + + /** + * Private constructor + */ + protected function __construct( $fileName, $callback, $options ) { + $this->fileName = $fileName; + $this->callback = $callback; + + if ( isset( $options['zip64'] ) ) { + $this->zip64 = $options['zip64']; + } + } + + /** + * Read the directory according to settings in $this. + */ + function execute() { + $this->file = fopen( $this->fileName, 'r' ); + $this->data = array(); + if ( !$this->file ) { + return Status::newFatal( 'zip-file-open-error' ); + } + + $status = Status::newGood(); + try { + $this->readEndOfCentralDirectoryRecord(); + if ( $this->zip64 ) { + list( $offset, $size ) = $this->findZip64CentralDirectory(); + $this->readCentralDirectory( $offset, $size ); + } else { + if ( $this->eocdr['CD size'] == 0xffffffff + || $this->eocdr['CD offset'] == 0xffffffff + || $this->eocdr['CD entries total'] == 0xffff ) + { + $this->error( 'zip-unsupported', 'Central directory header indicates ZIP64, ' . + 'but we are in legacy mode. Rejecting this upload is necessary to avoid '. + 'opening vulnerabilities on clients using OpenJDK 7 or later.' ); + } + + list( $offset, $size ) = $this->findOldCentralDirectory(); + $this->readCentralDirectory( $offset, $size ); + } + } catch ( ZipDirectoryReaderError $e ) { + $status->fatal( $e->getErrorCode() ); + } + + fclose( $this->file ); + return $status; + } + + /** + * Throw an error, and log a debug message + */ + function error( $code, $debugMessage ) { + wfDebug( __CLASS__.": Fatal error: $debugMessage\n" ); + throw new ZipDirectoryReaderError( $code ); + } + + /** + * Read the header which is at the end of the central directory, + * unimaginatively called the "end of central directory record" by the ZIP + * spec. + */ + function readEndOfCentralDirectoryRecord() { + $info = array( + 'signature' => 4, + 'disk' => 2, + 'CD start disk' => 2, + 'CD entries this disk' => 2, + 'CD entries total' => 2, + 'CD size' => 4, + 'CD offset' => 4, + 'file comment length' => 2, + ); + $structSize = $this->getStructSize( $info ); + $startPos = $this->getFileLength() - 65536 - $structSize; + if ( $startPos < 0 ) { + $startPos = 0; + } + + $block = $this->getBlock( $startPos ); + $sigPos = strrpos( $block, "PK\x05\x06" ); + if ( $sigPos === false ) { + $this->error( 'zip-wrong-format', + "zip file lacks EOCDR signature. It probably isn't a zip file." ); + } + + $this->eocdr = $this->unpack( substr( $block, $sigPos ), $info ); + $this->eocdr['EOCDR size'] = $structSize + $this->eocdr['file comment length']; + + if ( $structSize + $this->eocdr['file comment length'] != strlen( $block ) - $sigPos ) { + $this->error( 'zip-bad', 'trailing bytes after the end of the file comment' ); + } + if ( $this->eocdr['disk'] !== 0 + || $this->eocdr['CD start disk'] !== 0 ) + { + $this->error( 'zip-unsupported', 'more than one disk (in EOCDR)' ); + } + $this->eocdr += $this->unpack( + $block, + array( 'file comment' => array( 'string', $this->eocdr['file comment length'] ) ), + $sigPos + $structSize ); + $this->eocdr['position'] = $startPos + $sigPos; + } + + /** + * Read the header called the "ZIP64 end of central directory locator". An + * error will be raised if it does not exist. + */ + function readZip64EndOfCentralDirectoryLocator() { + $info = array( + 'signature' => array( 'string', 4 ), + 'eocdr64 start disk' => 4, + 'eocdr64 offset' => 8, + 'number of disks' => 4, + ); + $structSize = $this->getStructSize( $info ); + + $block = $this->getBlock( $this->getFileLength() - $this->eocdr['EOCDR size'] + - $structSize, $structSize ); + $this->eocdr64Locator = $data = $this->unpack( $block, $info ); + + if ( $data['signature'] !== "PK\x06\x07" ) { + // Note: Java will allow this and continue to read the + // EOCDR64, so we have to reject the upload, we can't + // just use the EOCDR header instead. + $this->error( 'zip-bad', 'wrong signature on Zip64 end of central directory locator' ); + } + } + + /** + * Read the header called the "ZIP64 end of central directory record". It + * may replace the regular "end of central directory record" in ZIP64 files. + */ + function readZip64EndOfCentralDirectoryRecord() { + if ( $this->eocdr64Locator['eocdr64 start disk'] != 0 + || $this->eocdr64Locator['number of disks'] != 0 ) + { + $this->error( 'zip-unsupported', 'more than one disk (in EOCDR64 locator)' ); + } + + $info = array( + 'signature' => array( 'string', 4 ), + 'EOCDR64 size' => 8, + 'version made by' => 2, + 'version needed' => 2, + 'disk' => 4, + 'CD start disk' => 4, + 'CD entries this disk' => 8, + 'CD entries total' => 8, + 'CD size' => 8, + 'CD offset' => 8 + ); + $structSize = $this->getStructSize( $info ); + $block = $this->getBlock( $this->eocdr64Locator['eocdr64 offset'], $structSize ); + $this->eocdr64 = $data = $this->unpack( $block, $info ); + if ( $data['signature'] !== "PK\x06\x06" ) { + $this->error( 'zip-bad', 'wrong signature on Zip64 end of central directory record' ); + } + if ( $data['disk'] !== 0 + || $data['CD start disk'] !== 0 ) + { + $this->error( 'zip-unsupported', 'more than one disk (in EOCDR64)' ); + } + } + + /** + * Find the location of the central directory, as would be seen by a + * non-ZIP64 reader. + * + * @return List containing offset, size and end position. + */ + function findOldCentralDirectory() { + $size = $this->eocdr['CD size']; + $offset = $this->eocdr['CD offset']; + $endPos = $this->eocdr['position']; + + // Some readers use the EOCDR position instead of the offset field + // to find the directory, so to be safe, we check if they both agree. + if ( $offset + $size != $endPos ) { + $this->error( 'zip-bad', 'the central directory does not immediately precede the end ' . + 'of central directory record' ); + } + return array( $offset, $size ); + } + + /** + * Find the location of the central directory, as would be seen by a + * ZIP64-compliant reader. + * + * @return List containing offset, size and end position. + */ + function findZip64CentralDirectory() { + // The spec is ambiguous about the exact rules of precedence between the + // ZIP64 headers and the original headers. Here we follow zip_util.c + // from OpenJDK 7. + $size = $this->eocdr['CD size']; + $offset = $this->eocdr['CD offset']; + $numEntries = $this->eocdr['CD entries total']; + $endPos = $this->eocdr['position']; + if ( $size == 0xffffffff + || $offset == 0xffffffff + || $numEntries == 0xffff ) + { + $this->readZip64EndOfCentralDirectoryLocator(); + + if ( isset( $this->eocdr64Locator['eocdr64 offset'] ) ) { + $this->readZip64EndOfCentralDirectoryRecord(); + if ( isset( $this->eocdr64['CD offset'] ) ) { + $size = $this->eocdr64['CD size']; + $offset = $this->eocdr64['CD offset']; + $endPos = $this->eocdr64Locator['eocdr64 offset']; + } + } + } + // Some readers use the EOCDR position instead of the offset field + // to find the directory, so to be safe, we check if they both agree. + if ( $offset + $size != $endPos ) { + $this->error( 'zip-bad', 'the central directory does not immediately precede the end ' . + 'of central directory record' ); + } + return array( $offset, $size ); + } + + /** + * Read the central directory at the given location + */ + function readCentralDirectory( $offset, $size ) { + $block = $this->getBlock( $offset, $size ); + + $fixedInfo = array( + 'signature' => array( 'string', 4 ), + 'version made by' => 2, + 'version needed' => 2, + 'general bits' => 2, + 'compression method' => 2, + 'mod time' => 2, + 'mod date' => 2, + 'crc-32' => 4, + 'compressed size' => 4, + 'uncompressed size' => 4, + 'name length' => 2, + 'extra field length' => 2, + 'comment length' => 2, + 'disk number start' => 2, + 'internal attrs' => 2, + 'external attrs' => 4, + 'local header offset' => 4, + ); + $fixedSize = $this->getStructSize( $fixedInfo ); + + $pos = 0; + while ( $pos < $size ) { + $data = $this->unpack( $block, $fixedInfo, $pos ); + $pos += $fixedSize; + + if ( $data['signature'] !== "PK\x01\x02" ) { + $this->error( 'zip-bad', 'Invalid signature found in directory entry' ); + } + + $variableInfo = array( + 'name' => array( 'string', $data['name length'] ), + 'extra field' => array( 'string', $data['extra field length'] ), + 'comment' => array( 'string', $data['comment length'] ), + ); + $data += $this->unpack( $block, $variableInfo, $pos ); + $pos += $this->getStructSize( $variableInfo ); + + if ( $this->zip64 && ( + $data['compressed size'] == 0xffffffff + || $data['uncompressed size'] == 0xffffffff + || $data['local header offset'] == 0xffffffff ) ) + { + $zip64Data = $this->unpackZip64Extra( $data['extra field'] ); + if ( $zip64Data ) { + $data = $zip64Data + $data; + } + } + + if ( $this->testBit( $data['general bits'], self::GENERAL_CD_ENCRYPTED ) ) { + $this->error( 'zip-unsupported', 'central directory encryption is not supported' ); + } + + // Convert the timestamp into MediaWiki format + // For the format, please see the MS-DOS 2.0 Programmer's Reference, + // pages 3-5 and 3-6. + $time = $data['mod time']; + $date = $data['mod date']; + + $year = 1980 + ( $date >> 9 ); + $month = ( $date >> 5 ) & 15; + $day = $date & 31; + $hour = ( $time >> 11 ) & 31; + $minute = ( $time >> 5 ) & 63; + $second = ( $time & 31 ) * 2; + $timestamp = sprintf( "%04d%02d%02d%02d%02d%02d", + $year, $month, $day, $hour, $minute, $second ); + + // Convert the character set in the file name + if ( !function_exists( 'iconv' ) + || $this->testBit( $data['general bits'], self::GENERAL_UTF8 ) ) + { + $name = $data['name']; + } else { + $name = iconv( 'CP437', 'UTF-8', $data['name'] ); + } + + // Compile a data array for the user, with a sensible format + $userData = array( + 'name' => $name, + 'mtime' => $timestamp, + 'size' => $data['uncompressed size'], + ); + call_user_func( $this->callback, $userData ); + } + } + + /** + * Interpret ZIP64 "extra field" data and return an associative array. + */ + function unpackZip64Extra( $extraField ) { + $extraHeaderInfo = array( + 'id' => 2, + 'size' => 2, + ); + $extraHeaderSize = $this->getStructSize( $extraHeaderInfo ); + + $zip64ExtraInfo = array( + 'uncompressed size' => 8, + 'compressed size' => 8, + 'local header offset' => 8, + 'disk number start' => 4, + ); + + $extraPos = 0; + $extraField = $data['extra field']; + while ( $extraPos < strlen( $extraField ) ) { + $extra = $this->unpack( $extraField, $extraHeaderInfo, $extraPos ); + $extraPos += $extraHeaderSize; + $extra += $this->unpack( $extraField, + array( 'data' => array( 'string', $extra['size'] ) ), + $extraPos ); + $extraPos += $extra['size']; + + if ( $extra['id'] == self::ZIP64_EXTRA_HEADER ) { + return $this->unpack( $extra['data'], $zip64ExtraInfo ); + } + } + + return false; + } + + /** + * Get the length of the file. + */ + function getFileLength() { + if ( $this->fileLength === null ) { + $stat = fstat( $this->file ); + $this->fileLength = $stat['size']; + } + return $this->fileLength; + } + + /** + * Get the file contents from a given offset. If there are not enough bytes + * in the file to satisfy the request, an exception will be thrown. + * + * @param $start The byte offset of the start of the block. + * @param $length The number of bytes to return. If omitted, the remainder + * of the file will be returned. + * + * @return string + */ + function getBlock( $start, $length = null ) { + $fileLength = $this->getFileLength(); + if ( $start >= $fileLength ) { + $this->error( 'zip-bad', "getBlock() requested position $start, " . + "file length is $fileLength" ); + } + if ( $length === null ) { + $length = $fileLength - $start; + } + $end = $start + $length; + if ( $end > $fileLength ) { + $this->error( 'zip-bad', "getBlock() requested end position $end, " . + "file length is $fileLength" ); + } + $startSeg = floor( $start / self::SEGSIZE ); + $endSeg = ceil( $end / self::SEGSIZE ); + + $block = ''; + for ( $segIndex = $startSeg; $segIndex <= $endSeg; $segIndex++ ) { + $block .= $this->getSegment( $segIndex ); + } + + $block = substr( $block, + $start - $startSeg * self::SEGSIZE, + $length ); + + if ( strlen( $block ) < $length ) { + $this->error( 'zip-bad', 'getBlock() returned an unexpectedly small amount of data' ); + } + + return $block; + } + + /** + * Get a section of the file starting at position $segIndex * self::SEGSIZE, + * of length self::SEGSIZE. The result is cached. This is a helper function + * for getBlock(). + * + * If there are not enough bytes in the file to satsify the request, the + * return value will be truncated. If a request is made for a segment beyond + * the end of the file, an empty string will be returned. + */ + function getSegment( $segIndex ) { + if ( !isset( $this->buffer[$segIndex] ) ) { + $bytePos = $segIndex * self::SEGSIZE; + if ( $bytePos >= $this->getFileLength() ) { + $this->buffer[$segIndex] = ''; + return ''; + } + if ( fseek( $this->file, $bytePos ) ) { + $this->error( 'zip-bad', "seek to $bytePos failed" ); + } + $seg = fread( $this->file, self::SEGSIZE ); + if ( $seg === false ) { + $this->error( 'zip-bad', "read from $bytePos failed" ); + } + $this->buffer[$segIndex] = $seg; + } + return $this->buffer[$segIndex]; + } + + /** + * Get the size of a structure in bytes. See unpack() for the format of $struct. + */ + function getStructSize( $struct ) { + $size = 0; + foreach ( $struct as $key => $type ) { + if ( is_array( $type ) ) { + list( $typeName, $fieldSize ) = $type; + $size += $fieldSize; + } else { + $size += $type; + } + } + return $size; + } + + /** + * Unpack a binary structure. This is like the built-in unpack() function + * except nicer. + * + * @param $string The binary data input + * + * @param $struct An associative array giving structure members and their + * types. In the key is the field name. The value may be either an + * integer, in which case the field is a little-endian unsigned integer + * encoded in the given number of bytes, or an array, in which case the + * first element of the array is the type name, and the subsequent + * elements are type-dependent parameters. Only one such type is defined: + * - "string": The second array element gives the length of string. + * Not null terminated. + * + * @param $offset The offset into the string at which to start unpacking. + * + * @return Unpacked associative array. Note that large integers in the input + * may be represented as floating point numbers in the return value, so + * the use of weak comparison is advised. + */ + function unpack( $string, $struct, $offset = 0 ) { + $size = $this->getStructSize( $struct ); + if ( $offset + $size > strlen( $string ) ) { + $this->error( 'zip-bad', 'unpack() would run past the end of the supplied string' ); + } + + $data = array(); + $pos = $offset; + foreach ( $struct as $key => $type ) { + if ( is_array( $type ) ) { + list( $typeName, $fieldSize ) = $type; + switch ( $typeName ) { + case 'string': + $data[$key] = substr( $string, $pos, $fieldSize ); + $pos += $fieldSize; + break; + default: + throw new MWException( __METHOD__.": invalid type \"$typeName\"" ); + } + } else { + // Unsigned little-endian integer + $length = intval( $type ); + $bytes = substr( $string, $pos, $length ); + + // Calculate the value. Use an algorithm which automatically + // upgrades the value to floating point if necessary. + $value = 0; + for ( $i = $length - 1; $i >= 0; $i-- ) { + $value *= 256; + $value += ord( $string[$pos + $i] ); + } + + // Throw an exception if there was loss of precision + if ( $value > pow( 2, 52 ) ) { + $this->error( 'zip-unsupported', 'number too large to be stored in a double. ' . + 'This could happen if we tried to unpack a 64-bit structure ' . + 'at an invalid location.' ); + } + $data[$key] = $value; + $pos += $length; + } + } + + return $data; + } + + /** + * Returns a bit from a given position in an integer value, converted to + * boolean. + * + * @param $value integer + * @param $bitIndex The index of the bit, where 0 is the LSB. + */ + function testBit( $value, $bitIndex ) { + return (bool)( ( $value >> $bitIndex ) & 1 ); + } + + /** + * Debugging helper function which dumps a string in hexdump -C format. + */ + function hexDump( $s ) { + $n = strlen( $s ); + for ( $i = 0; $i < $n; $i += 16 ) { + printf( "%08X ", $i ); + for ( $j = 0; $j < 16; $j++ ) { + print " "; + if ( $j == 8 ) { + print " "; + } + if ( $i + $j >= $n ) { + print " "; + } else { + printf( "%02X", ord( $s[$i + $j] ) ); + } + } + + print " |"; + for ( $j = 0; $j < 16; $j++ ) { + if ( $i + $j >= $n ) { + print " "; + } elseif ( ctype_print( $s[$i + $j] ) ) { + print $s[$i + $j]; + } else { + print '.'; + } + } + print "|\n"; + } + } +} + +/** + * Internal exception class. Will be caught by private code. + */ +class ZipDirectoryReaderError extends Exception { + var $code; + + function __construct( $code ) { + $this->code = $code; + parent::__construct( "ZipDirectoryReader error: $code" ); + } + + function getErrorCode() { + return $this->code; + } +} diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index da2bc97258..037aa8353b 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -20,6 +20,7 @@ abstract class UploadBase { protected $mFilteredName, $mFinalExtension; protected $mLocalFile, $mFileSize, $mFileProps; protected $mBlackListedExtensions; + protected $mJavaDetected; const SUCCESS = 0; const OK = 0; @@ -347,6 +348,7 @@ abstract class UploadBase { * @return mixed true of the file is verified, array otherwise. */ protected function verifyFile() { + global $wgAllowJavaUploads; # get the title, even though we are doing nothing with it, because # we need to populate mFinalExtension $this->getTitle(); @@ -371,9 +373,25 @@ abstract class UploadBase { } } - /** - * Scan the uploaded file for viruses - */ + # Check for Java applets, which if uploaded can bypass cross-site + # restrictions. + if ( !$wgAllowJavaUploads ) { + $this->mJavaDetected = false; + $zipStatus = ZipDirectoryReader::read( $this->mTempPath, + array( $this, 'zipEntryCallback' ) ); + if ( !$zipStatus->isOK() ) { + $errors = $zipStatus->getErrorsArray(); + $error = reset( $errors ); + if ( $error[0] !== 'zip-wrong-format' ) { + return $error; + } + } + if ( $this->mJavaDetected ) { + return array( 'uploadjava' ); + } + } + + # Scan the uploaded file for viruses $virus = $this->detectVirus( $this->mTempPath ); if ( $virus ) { return array( 'uploadvirus', $virus ); @@ -397,6 +415,29 @@ abstract class UploadBase { return true; } + /** + * Callback for ZipDirectoryReader to detect Java class files. + */ + function zipEntryCallback( $entry ) { + $names = array( $entry['name'] ); + + // If there is a null character, cut off the name at it, because JDK's + // ZIP_GetEntry() uses strcmp() if the name hashes match. If a file name + // were constructed which had ".class\0" followed by a string chosen to + // make the hash collide with the truncated name, that file could be + // returned in response to a request for the .class file. + $nullPos = strpos( $entry['name'], "\000" ); + if ( $nullPos !== false ) { + $names[] = substr( $entry['name'], 0, $nullPos ); + } + + // If there is a trailing slash in the file name, we have to strip it, + // because that's what ZIP_GetEntry() does. + if ( preg_grep( '!\.class/?$!', $names ) ) { + $this->mJavaDetected = true; + } + } + /** * Check whether the user can edit, upload and create the image. This * checks only against the current title; if it returns errors, it may diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index 72950ed3b0..40a3f4a401 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -2185,6 +2185,8 @@ You should check that file's deletion history before proceeding to re-upload it. 'php-uploaddisabledtext' => 'File uploads are disabled in PHP. Please check the file_uploads setting.', 'uploadscripted' => 'This file contains HTML or script code that may be erroneously interpreted by a web browser.', +'uploadjava' => 'The file is a ZIP file which contains a Java .class file. +Uploading Java files is not allowed, because they can cause security restrictions to be bypassed.', 'uploadvirus' => 'The file contains a virus! Details: $1', 'upload-source' => 'Source file', @@ -2239,6 +2241,14 @@ If the problem persists, contact an [[Special:ListUsers/sysop|administrator]].', 'upload-unknown-size' => 'Unknown size', 'upload-http-error' => 'An HTTP error occured: $1', +# ZipDirectoryReader +'zip-file-open-error' => 'An error was encountered when opening the file for ZIP checks.', +'zip-wrong-format' => 'The specified file was not a ZIP file.', +'zip-bad' => 'The file is a corrupt or otherwise unreadable ZIP file. +It cannot be properly checked for security.', +'zip-unsupported' => 'The file is a ZIP file which uses ZIP features not supported by MediaWiki. +It cannot be properly checked for security.', + # Special:UploadStash 'uploadstash' => 'Upload stash', 'uploadstash-summary' => 'This page provides access to files which are uploaded (or in the process of uploading) but are not yet published to the wiki. These files are not visible to anyone but the user who uploaded them.', diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc index 034cce0cdf..2bf4ad55bd 100644 --- a/maintenance/language/messages.inc +++ b/maintenance/language/messages.inc @@ -1319,6 +1319,7 @@ $wgMessageStructure = array( 'php-uploaddisabledtext', 'uploadscripted', 'uploadvirus', + 'uploadjava', 'upload-source', 'sourcefilename', 'sourceurl', @@ -1350,6 +1351,13 @@ $wgMessageStructure = array( 'upload-http-error', ), + 'zip' => array( + 'zip-file-open-error', + 'zip-wrong-format', + 'zip-bad', + 'zip-unsupported' + ), + 'uploadstash' => array( 'uploadstash', 'uploadstash-summary', @@ -3362,6 +3370,7 @@ XHTML id names.", 'recentchanges' => 'Recent changes', 'recentchangeslinked' => 'Recent changes linked', 'upload' => 'Upload', + 'zip' => 'ZipDirectoryReader', 'upload-errors' => '', 'uploadstash' => 'Special:UploadStash', 'img-auth' => 'img_auth script messages', diff --git a/tests/phpunit/data/zip/cd-gap.zip b/tests/phpunit/data/zip/cd-gap.zip new file mode 100644 index 0000000000000000000000000000000000000000..b5ae6ccd363ea5bb9941140e020208a5c8eab560 GIT binary patch literal 182 zcmWIWW@h1HfB=21$!yn+7=auR=4Oy#aL!3AF4jv1k)a`+49pi6C0c=SX$3a}Bg+eB z1_m&}72wUtB*%=)90{=TATyUVf|w|#vO-KnGtoV<0D=O%S=m5(7=bVlNV|bJ3;+oJ BAs_$% literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/cd-truncated.zip b/tests/phpunit/data/zip/cd-truncated.zip new file mode 100644 index 0000000000000000000000000000000000000000..4d40d7d4147d53d1af267efa149878030e9b3616 GIT binary patch literal 171 zcmWIWW@h1HfB=21$!yn+7=auR=4Oy#aL!3AF4jv1k)a`+49pi6C0c=SX$3a}Bg+eB z1_m&}72wUtB*%=)90{=TATyUVf|zhqLqk|0W}=xF;LXYgQpgB|{y^Fd#9;scSY;gW literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/class-trailing-null.zip b/tests/phpunit/data/zip/class-trailing-null.zip new file mode 100644 index 0000000000000000000000000000000000000000..31dcf3d8d8b75c70bf64d0568a04be30685f829a GIT binary patch literal 173 zcmWIWW@h1HfB=21$!yn+7=auR=4Oy#aCCBZ(M!%rEG}jU4dG;9zOX3K3WQ55xEUB( zUNAE-fC;VuZ$>6LW?be-fQ<*4xug-qL@|{WVk(-60p6@^Af=2z7zm`@KpX}D0SFub literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/class-trailing-slash.zip b/tests/phpunit/data/zip/class-trailing-slash.zip new file mode 100644 index 0000000000000000000000000000000000000000..9eb1f0376bc04d91b106b539e594839b41ee50ed GIT binary patch literal 173 zcmWIWW@h1HfB=21$!yn+7=auR=4Oy#aCCBZ(M!%rEH2g$4dG;9zOX3K3WQ55xEUB( zUNAE-fC;VuZ$>6LW?be-fQ<*4xug-qL@|{WVk(-60p6@^Af=2z7zm`@KpX}DA`~26 literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/class.zip b/tests/phpunit/data/zip/class.zip new file mode 100644 index 0000000000000000000000000000000000000000..98a625b7517f87907b2dd2e213d42387d5c4e57a GIT binary patch literal 173 zcmWIWW@h1HfB=21$!yn+7=auR=4Oy#aL!3AF4jv1k)a`+49pi6C0c=SX$3a}Bg+eB x1_m&}72wUtB*%=)90{=TATyUVf|w|#vO-KnGcmxMl?|kn5eNf;v>S-S006II9oPT> literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/empty.zip b/tests/phpunit/data/zip/empty.zip new file mode 100644 index 0000000000000000000000000000000000000000..15cb0ecb3e219d1701294bfdf0fe3f5cb5d208e7 GIT binary patch literal 22 NcmWIWW@Tf*000g10H*)| literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/looks-like-zip64.zip b/tests/phpunit/data/zip/looks-like-zip64.zip new file mode 100644 index 0000000000000000000000000000000000000000..7428cdddc2a3b78a354e16991552ab5bca42186b GIT binary patch literal 173 zcmWIWW@h1HfB=21$!yn+7=auR=4Oy#aL!3AF4jv1k)a`+49pi6C0c=SX$3a}Bg+eB z1_m&}72wUtB*%=)90{=TATyUVf|w|#vO-KnGcmxMl?|knk>UUUKp^b~;xGUJ#y=m` literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/nosig.zip b/tests/phpunit/data/zip/nosig.zip new file mode 100644 index 0000000000000000000000000000000000000000..a22c73a475c971c32f97e29da7f3c61a8e8221c8 GIT binary patch literal 173 zcmb=ZU}oWBfB=21$!yn+7=auR=4Oy#aL!3AF4jv1k)a`+49pi6C0c=SX$3a}Bg+eB x1_m&}RZ+pnB*%=)90{=TATyUVf|w|#vO-KnGqIwAl?|kn5eNf;v>S-S000r(9@hW> literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/split.zip b/tests/phpunit/data/zip/split.zip new file mode 100644 index 0000000000000000000000000000000000000000..6984ae6d9f19d04e8d9dacce28768b65a440cb44 GIT binary patch literal 196 zcmbRBRw2%MW~Zd5eQUw{Zryr6<{w835+@{|DHZ6k`gbLNtI~=icYlj238u+aDNm1? zWmQQWd?p5N1_>Yy0ZSTLfQ+KVyp;T0T|*;_?9dQa2IduF i>As~E+zgB?FPIq^z(jyID;pC~Gb0cN0=2|~^#cF~1Ut0= literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/trail.zip b/tests/phpunit/data/zip/trail.zip new file mode 100644 index 0000000000000000000000000000000000000000..50bcea128a46a40417700a9594c9cdbecb38a55e GIT binary patch literal 181 zcmWIWW@h1HfB=21$!yn+7=auR=4Oy#aL!3AF4jv1k)a`+49pi6C0c=SX$3a}Bg+eB z1_m&}72wUtB*%=)90{=TATyUVf|w|#vO-KnGcmxMl?|kn5eNf;v>S-S5K@$wnWLwt F2LP)EAYK3f literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/wrong-cd-start-disk.zip b/tests/phpunit/data/zip/wrong-cd-start-disk.zip new file mode 100644 index 0000000000000000000000000000000000000000..59b45938e47cf0c4b0f6658b63fa2294e26032e1 GIT binary patch literal 173 zcmWIWW@h1HfB=21$!yn+7=auR=4Oy#aL!3AF4jv1k)a`+49pi6C0c=SX$3a}Bg+eB x1_m&}72wUtB*%=)90{=TATyUVf|w|#vO-KnGcmxMl?~(!5DWxraRcdN006IY9oYZ? literal 0 HcmV?d00001 diff --git a/tests/phpunit/data/zip/wrong-central-entry-sig.zip b/tests/phpunit/data/zip/wrong-central-entry-sig.zip new file mode 100644 index 0000000000000000000000000000000000000000..05329b4333b73e7a9b7cecb29b3432d747e44c37 GIT binary patch literal 173 zcmWIWW@h1HfB=21$!yn+7=auR=4Oy#aL!3AF4jv1k)a`+49pi6C0c=SX$3a}Bg+eB v1_m&}RRIKY%(%>v02>c7b4eqJiDD`%#8fmB1H4(;KuQ^bFc3()fjA5RE`lE# literal 0 HcmV?d00001 diff --git a/tests/phpunit/includes/ZipDirectoryReaderTest.php b/tests/phpunit/includes/ZipDirectoryReaderTest.php new file mode 100644 index 0000000000..f7ca59e241 --- /dev/null +++ b/tests/phpunit/includes/ZipDirectoryReaderTest.php @@ -0,0 +1,79 @@ +zipDir = dirname( __FILE__ ) . '/../data/zip'; + } + + function zipCallback( $entry ) { + $this->entries[] = $entry; + } + + function readZipAssertError( $file, $error, $assertMessage ) { + $this->entries = array(); + $status = ZipDirectoryReader::read( "{$this->zipDir}/$file", array( $this, 'zipCallback' ) ); + $this->assertTrue( $status->hasMessage( $error ), $assertMessage ); + } + + function readZipAssertSuccess( $file, $assertMessage ) { + $this->entries = array(); + $status = ZipDirectoryReader::read( "{$this->zipDir}/$file", array( $this, 'zipCallback' ) ); + $this->assertTrue( $status->isOK(), $assertMessage ); + } + + function testEmpty() { + $this->readZipAssertSuccess( 'empty.zip', 'Empty zip' ); + } + + function testMultiDisk0() { + $this->readZipAssertError( 'split.zip', 'zip-unsupported', + 'Split zip error' ); + } + + function testNoSignature() { + $this->readZipAssertError( 'nosig.zip', 'zip-wrong-format', + 'No signature should give "wrong format" error' ); + } + + function testSimple() { + $this->readZipAssertSuccess( 'class.zip', 'Simple ZIP' ); + $this->assertEquals( $this->entries, array( array( + 'name' => 'Class.class', + 'mtime' => '20010115000000', + 'size' => 1, + ) ) ); + } + + function testBadCentralEntrySignature() { + $this->readZipAssertError( 'wrong-central-entry-sig.zip', 'zip-bad', + 'Bad central entry error' ); + } + + function testTrailingBytes() { + $this->readZipAssertError( 'trail.zip', 'zip-bad', + 'Trailing bytes error' ); + } + + function testWrongCDStart() { + $this->readZipAssertError( 'wrong-cd-start-disk.zip', 'zip-unsupported', + 'Wrong CD start disk error' ); + } + + + function testCentralDirectoryGap() { + $this->readZipAssertError( 'cd-gap.zip', 'zip-bad', + 'CD gap error' ); + } + + function testCentralDirectoryTruncated() { + $this->readZipAssertError( 'cd-truncated.zip', 'zip-bad', + 'CD truncated error (should hit unpack() overrun)' ); + } + + function testLooksLikeZip64() { + $this->readZipAssertError( 'looks-like-zip64.zip', 'zip-unsupported', + 'A file which looks like ZIP64 but isn\'t, should give error' ); + } +} -- 2.20.1