* (bug 24230) Added JAR detection. ZIP archives containing a .class file will be...
authorTim Starling <tstarling@users.mediawiki.org>
Fri, 25 Feb 2011 04:51:17 +0000 (04:51 +0000)
committerTim Starling <tstarling@users.mediawiki.org>
Fri, 25 Feb 2011 04:51:17 +0000 (04:51 +0000)
* 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().

21 files changed:
RELEASE-NOTES
includes/AutoLoader.php
includes/DefaultSettings.php
includes/Status.php
includes/ZipDirectoryReader.php [new file with mode: 0644]
includes/upload/UploadBase.php
languages/messages/MessagesEn.php
maintenance/language/messages.inc
tests/phpunit/data/zip/cd-gap.zip [new file with mode: 0644]
tests/phpunit/data/zip/cd-truncated.zip [new file with mode: 0644]
tests/phpunit/data/zip/class-trailing-null.zip [new file with mode: 0644]
tests/phpunit/data/zip/class-trailing-slash.zip [new file with mode: 0644]
tests/phpunit/data/zip/class.zip [new file with mode: 0644]
tests/phpunit/data/zip/empty.zip [new file with mode: 0644]
tests/phpunit/data/zip/looks-like-zip64.zip [new file with mode: 0644]
tests/phpunit/data/zip/nosig.zip [new file with mode: 0644]
tests/phpunit/data/zip/split.zip [new file with mode: 0644]
tests/phpunit/data/zip/trail.zip [new file with mode: 0644]
tests/phpunit/data/zip/wrong-cd-start-disk.zip [new file with mode: 0644]
tests/phpunit/data/zip/wrong-central-entry-sig.zip [new file with mode: 0644]
tests/phpunit/includes/ZipDirectoryReaderTest.php [new file with mode: 0644]

index 52cc00a..5ebd6df 100644 (file)
@@ -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
index 4b7c346..c43240b 100644 (file)
@@ -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',
index 3b2cf21..bdee3f9 100644 (file)
@@ -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.
  *
index f285cf7..30cbb85 100644 (file)
@@ -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 (file)
index 0000000..85e5347
--- /dev/null
@@ -0,0 +1,683 @@
+<?php
+
+/**
+ * A class for reading ZIP file directories, for the purposes of upload 
+ * verification. 
+ *
+ * Only a functional interface is provided: ZipFileReader::read(). No access is
+ * given to object instances.
+ *
+ */
+class ZipDirectoryReader {
+       /**
+        * Read a ZIP file and call a function for each file discovered in it.
+        *
+        * Because this class is aimed at verification, an error is raised on 
+        * suspicious or ambiguous input, instead of emulating some standard
+        * behaviour.
+        *
+        * @param $fileName The archive file name
+        * @param $callback The callback function. It will be called for each file 
+        *   with a single associative array each time, with members:
+        *
+        *      - name: The file name. Directories conventionally have a trailing 
+        *        slash.
+        *
+        *      - mtime: The file modification time, in MediaWiki 14-char format
+        *
+        *      - size: The uncompressed file size
+        *
+        * @param $options An associative array of read options, with the option
+        *    name in the key. This may currently contain:
+        *
+        *      - zip64: If this is set to true, then we will emulate a 
+        *        library with ZIP64 support, like OpenJDK 7. If it is set to 
+        *        false, then we will emulate a library with no knowledge of 
+        *        ZIP64.
+        *
+        *        NOTE: The ZIP64 code is untested and probably doesn't work. It 
+        *        turned out to be easier to just reject ZIP64 archive uploads, 
+        *        since they are likely to be very rare. Confirming safety of a 
+        *        ZIP64 file is fairly complex. What do you do with a file that is 
+        *        ambiguous and broken when read with a non-ZIP64 reader, but valid 
+        *        when read with a ZIP64 reader? This situation is normal for a 
+        *        valid ZIP64 file, and working out what non-ZIP64 readers will make 
+        *        of such a file is not trivial.
+        *
+        * @return A Status object. The following fatal errors are defined:
+        *
+        *      - zip-file-open-error: The file could not be opened.
+        *
+        *      - zip-wrong-format: The file does not appear to be a ZIP file.
+        *
+        *      - zip-bad: There was something wrong or ambiguous about the file 
+        *        data.
+        *
+        *      - zip-unsupported: The ZIP file uses features which 
+        *        ZipDirectoryReader does not support.
+        *
+        * The default messages for those fatal errors are written in a way that 
+        * makes sense for upload verification.
+        *
+        * If a fatal error is returned, more information about the error will be 
+        * available in the debug log.
+        *
+        * Note that the callback function may be called any number of times before
+        * a fatal error is returned. If this occurs, the data sent to the callback 
+        * function should be discarded.
+        */
+       public static function read( $fileName, $callback, $options = array() ) {
+               $zdr = new self( $fileName, $callback, $options );
+               return $zdr->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;
+       }
+}
index da2bc97..037aa83 100644 (file)
@@ -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
index 72950ed..40a3f4a 100644 (file)
@@ -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.',
index 034cce0..2bf4ad5 100644 (file)
@@ -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 (file)
index 0000000..b5ae6cc
Binary files /dev/null and b/tests/phpunit/data/zip/cd-gap.zip differ
diff --git a/tests/phpunit/data/zip/cd-truncated.zip b/tests/phpunit/data/zip/cd-truncated.zip
new file mode 100644 (file)
index 0000000..4d40d7d
Binary files /dev/null and b/tests/phpunit/data/zip/cd-truncated.zip differ
diff --git a/tests/phpunit/data/zip/class-trailing-null.zip b/tests/phpunit/data/zip/class-trailing-null.zip
new file mode 100644 (file)
index 0000000..31dcf3d
Binary files /dev/null and b/tests/phpunit/data/zip/class-trailing-null.zip differ
diff --git a/tests/phpunit/data/zip/class-trailing-slash.zip b/tests/phpunit/data/zip/class-trailing-slash.zip
new file mode 100644 (file)
index 0000000..9eb1f03
Binary files /dev/null and b/tests/phpunit/data/zip/class-trailing-slash.zip differ
diff --git a/tests/phpunit/data/zip/class.zip b/tests/phpunit/data/zip/class.zip
new file mode 100644 (file)
index 0000000..98a625b
Binary files /dev/null and b/tests/phpunit/data/zip/class.zip differ
diff --git a/tests/phpunit/data/zip/empty.zip b/tests/phpunit/data/zip/empty.zip
new file mode 100644 (file)
index 0000000..15cb0ec
Binary files /dev/null and b/tests/phpunit/data/zip/empty.zip differ
diff --git a/tests/phpunit/data/zip/looks-like-zip64.zip b/tests/phpunit/data/zip/looks-like-zip64.zip
new file mode 100644 (file)
index 0000000..7428cdd
Binary files /dev/null and b/tests/phpunit/data/zip/looks-like-zip64.zip differ
diff --git a/tests/phpunit/data/zip/nosig.zip b/tests/phpunit/data/zip/nosig.zip
new file mode 100644 (file)
index 0000000..a22c73a
Binary files /dev/null and b/tests/phpunit/data/zip/nosig.zip differ
diff --git a/tests/phpunit/data/zip/split.zip b/tests/phpunit/data/zip/split.zip
new file mode 100644 (file)
index 0000000..6984ae6
Binary files /dev/null and b/tests/phpunit/data/zip/split.zip differ
diff --git a/tests/phpunit/data/zip/trail.zip b/tests/phpunit/data/zip/trail.zip
new file mode 100644 (file)
index 0000000..50bcea1
Binary files /dev/null and b/tests/phpunit/data/zip/trail.zip differ
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 (file)
index 0000000..59b4593
Binary files /dev/null and b/tests/phpunit/data/zip/wrong-cd-start-disk.zip differ
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 (file)
index 0000000..05329b4
Binary files /dev/null and b/tests/phpunit/data/zip/wrong-central-entry-sig.zip differ
diff --git a/tests/phpunit/includes/ZipDirectoryReaderTest.php b/tests/phpunit/includes/ZipDirectoryReaderTest.php
new file mode 100644 (file)
index 0000000..f7ca59e
--- /dev/null
@@ -0,0 +1,79 @@
+<?php
+
+class ZipDirectoryReaderTest extends MediaWikiTestCase {
+       var $zipDir, $entries;
+
+       function setUp() {
+               $this->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' );
+       }
+}