r82783 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r82782‎ | r82783 | r82784 >
Date:04:51, 25 February 2011
Author:tstarling
Status:ok (Comments)
Tags:
Comment:
* (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().
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/Status.php (modified) (history)
  • /trunk/phase3/includes/ZipDirectoryReader.php (added) (history)
  • /trunk/phase3/includes/upload/UploadBase.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)
  • /trunk/phase3/maintenance/language/messages.inc (modified) (history)
  • /trunk/phase3/tests/phpunit/data (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/cd-gap.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/cd-truncated.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/class-trailing-null.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/class-trailing-slash.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/class.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/empty.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/looks-like-zip64.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/nosig.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/split.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/trail.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/wrong-cd-start-disk.zip (added) (history)
  • /trunk/phase3/tests/phpunit/data/zip/wrong-central-entry-sig.zip (added) (history)
  • /trunk/phase3/tests/phpunit/includes/ZipDirectoryReaderTest.php (added) (history)

Diff [purge]

Index: trunk/phase3/maintenance/language/messages.inc
@@ -1319,6 +1319,7 @@
13201320 'php-uploaddisabledtext',
13211321 'uploadscripted',
13221322 'uploadvirus',
 1323+ 'uploadjava',
13231324 'upload-source',
13241325 'sourcefilename',
13251326 'sourceurl',
@@ -1350,6 +1351,13 @@
13511352 'upload-http-error',
13521353 ),
13531354
 1355+ 'zip' => array(
 1356+ 'zip-file-open-error',
 1357+ 'zip-wrong-format',
 1358+ 'zip-bad',
 1359+ 'zip-unsupported'
 1360+ ),
 1361+
13541362 'uploadstash' => array(
13551363 'uploadstash',
13561364 'uploadstash-summary',
@@ -3362,6 +3370,7 @@
33633371 'recentchanges' => 'Recent changes',
33643372 'recentchangeslinked' => 'Recent changes linked',
33653373 'upload' => 'Upload',
 3374+ 'zip' => 'ZipDirectoryReader',
33663375 'upload-errors' => '',
33673376 'uploadstash' => 'Special:UploadStash',
33683377 'img-auth' => 'img_auth script messages',
Index: trunk/phase3/tests/phpunit/includes/ZipDirectoryReaderTest.php
@@ -0,0 +1,79 @@
 2+<?php
 3+
 4+class ZipDirectoryReaderTest extends MediaWikiTestCase {
 5+ var $zipDir, $entries;
 6+
 7+ function setUp() {
 8+ $this->zipDir = dirname( __FILE__ ) . '/../data/zip';
 9+ }
 10+
 11+ function zipCallback( $entry ) {
 12+ $this->entries[] = $entry;
 13+ }
 14+
 15+ function readZipAssertError( $file, $error, $assertMessage ) {
 16+ $this->entries = array();
 17+ $status = ZipDirectoryReader::read( "{$this->zipDir}/$file", array( $this, 'zipCallback' ) );
 18+ $this->assertTrue( $status->hasMessage( $error ), $assertMessage );
 19+ }
 20+
 21+ function readZipAssertSuccess( $file, $assertMessage ) {
 22+ $this->entries = array();
 23+ $status = ZipDirectoryReader::read( "{$this->zipDir}/$file", array( $this, 'zipCallback' ) );
 24+ $this->assertTrue( $status->isOK(), $assertMessage );
 25+ }
 26+
 27+ function testEmpty() {
 28+ $this->readZipAssertSuccess( 'empty.zip', 'Empty zip' );
 29+ }
 30+
 31+ function testMultiDisk0() {
 32+ $this->readZipAssertError( 'split.zip', 'zip-unsupported',
 33+ 'Split zip error' );
 34+ }
 35+
 36+ function testNoSignature() {
 37+ $this->readZipAssertError( 'nosig.zip', 'zip-wrong-format',
 38+ 'No signature should give "wrong format" error' );
 39+ }
 40+
 41+ function testSimple() {
 42+ $this->readZipAssertSuccess( 'class.zip', 'Simple ZIP' );
 43+ $this->assertEquals( $this->entries, array( array(
 44+ 'name' => 'Class.class',
 45+ 'mtime' => '20010115000000',
 46+ 'size' => 1,
 47+ ) ) );
 48+ }
 49+
 50+ function testBadCentralEntrySignature() {
 51+ $this->readZipAssertError( 'wrong-central-entry-sig.zip', 'zip-bad',
 52+ 'Bad central entry error' );
 53+ }
 54+
 55+ function testTrailingBytes() {
 56+ $this->readZipAssertError( 'trail.zip', 'zip-bad',
 57+ 'Trailing bytes error' );
 58+ }
 59+
 60+ function testWrongCDStart() {
 61+ $this->readZipAssertError( 'wrong-cd-start-disk.zip', 'zip-unsupported',
 62+ 'Wrong CD start disk error' );
 63+ }
 64+
 65+
 66+ function testCentralDirectoryGap() {
 67+ $this->readZipAssertError( 'cd-gap.zip', 'zip-bad',
 68+ 'CD gap error' );
 69+ }
 70+
 71+ function testCentralDirectoryTruncated() {
 72+ $this->readZipAssertError( 'cd-truncated.zip', 'zip-bad',
 73+ 'CD truncated error (should hit unpack() overrun)' );
 74+ }
 75+
 76+ function testLooksLikeZip64() {
 77+ $this->readZipAssertError( 'looks-like-zip64.zip', 'zip-unsupported',
 78+ 'A file which looks like ZIP64 but isn\'t, should give error' );
 79+ }
 80+}
Property changes on: trunk/phase3/tests/phpunit/includes/ZipDirectoryReaderTest.php
___________________________________________________________________
Added: svn:eol-style
181 + native
Index: trunk/phase3/tests/phpunit/data/zip/empty.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/empty.zip
___________________________________________________________________
Added: svn:mime-type
282 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/nosig.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/nosig.zip
___________________________________________________________________
Added: svn:mime-type
383 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/wrong-central-entry-sig.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/wrong-central-entry-sig.zip
___________________________________________________________________
Added: svn:mime-type
484 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/looks-like-zip64.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/looks-like-zip64.zip
___________________________________________________________________
Added: svn:mime-type
585 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/wrong-cd-start-disk.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/wrong-cd-start-disk.zip
___________________________________________________________________
Added: svn:mime-type
686 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/class-trailing-null.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/class-trailing-null.zip
___________________________________________________________________
Added: svn:mime-type
787 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/class-trailing-slash.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/class-trailing-slash.zip
___________________________________________________________________
Added: svn:mime-type
888 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/class.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/class.zip
___________________________________________________________________
Added: svn:mime-type
989 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/cd-gap.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/cd-gap.zip
___________________________________________________________________
Added: svn:mime-type
1090 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/split.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/split.zip
___________________________________________________________________
Added: svn:mime-type
1191 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/trail.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/trail.zip
___________________________________________________________________
Added: svn:mime-type
1292 + application/octet-stream
Index: trunk/phase3/tests/phpunit/data/zip/cd-truncated.zip
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/tests/phpunit/data/zip/cd-truncated.zip
___________________________________________________________________
Added: svn:mime-type
1393 + application/octet-stream
Index: trunk/phase3/includes/upload/UploadBase.php
@@ -20,6 +20,7 @@
2121 protected $mFilteredName, $mFinalExtension;
2222 protected $mLocalFile, $mFileSize, $mFileProps;
2323 protected $mBlackListedExtensions;
 24+ protected $mJavaDetected;
2425
2526 const SUCCESS = 0;
2627 const OK = 0;
@@ -347,6 +348,7 @@
348349 * @return mixed true of the file is verified, array otherwise.
349350 */
350351 protected function verifyFile() {
 352+ global $wgAllowJavaUploads;
351353 # get the title, even though we are doing nothing with it, because
352354 # we need to populate mFinalExtension
353355 $this->getTitle();
@@ -371,9 +373,25 @@
372374 }
373375 }
374376
375 - /**
376 - * Scan the uploaded file for viruses
377 - */
 377+ # Check for Java applets, which if uploaded can bypass cross-site
 378+ # restrictions.
 379+ if ( !$wgAllowJavaUploads ) {
 380+ $this->mJavaDetected = false;
 381+ $zipStatus = ZipDirectoryReader::read( $this->mTempPath,
 382+ array( $this, 'zipEntryCallback' ) );
 383+ if ( !$zipStatus->isOK() ) {
 384+ $errors = $zipStatus->getErrorsArray();
 385+ $error = reset( $errors );
 386+ if ( $error[0] !== 'zip-wrong-format' ) {
 387+ return $error;
 388+ }
 389+ }
 390+ if ( $this->mJavaDetected ) {
 391+ return array( 'uploadjava' );
 392+ }
 393+ }
 394+
 395+ # Scan the uploaded file for viruses
378396 $virus = $this->detectVirus( $this->mTempPath );
379397 if ( $virus ) {
380398 return array( 'uploadvirus', $virus );
@@ -398,6 +416,29 @@
399417 }
400418
401419 /**
 420+ * Callback for ZipDirectoryReader to detect Java class files.
 421+ */
 422+ function zipEntryCallback( $entry ) {
 423+ $names = array( $entry['name'] );
 424+
 425+ // If there is a null character, cut off the name at it, because JDK's
 426+ // ZIP_GetEntry() uses strcmp() if the name hashes match. If a file name
 427+ // were constructed which had ".class\0" followed by a string chosen to
 428+ // make the hash collide with the truncated name, that file could be
 429+ // returned in response to a request for the .class file.
 430+ $nullPos = strpos( $entry['name'], "\000" );
 431+ if ( $nullPos !== false ) {
 432+ $names[] = substr( $entry['name'], 0, $nullPos );
 433+ }
 434+
 435+ // If there is a trailing slash in the file name, we have to strip it,
 436+ // because that's what ZIP_GetEntry() does.
 437+ if ( preg_grep( '!\.class/?$!', $names ) ) {
 438+ $this->mJavaDetected = true;
 439+ }
 440+ }
 441+
 442+ /**
402443 * Check whether the user can edit, upload and create the image. This
403444 * checks only against the current title; if it returns errors, it may
404445 * very well be that another title will not give errors. Therefore
Index: trunk/phase3/includes/Status.php
@@ -281,7 +281,7 @@
282282 if( $error['params'] ) {
283283 $result[] = array_merge( array( $error['message'] ), $error['params'] );
284284 } else {
285 - $result[] = $error['message'];
 285+ $result[] = array( $error['message'] );
286286 }
287287 }
288288 }
Index: trunk/phase3/includes/AutoLoader.php
@@ -275,6 +275,7 @@
276276 'XmlSelect' => 'includes/Xml.php',
277277 'XmlTypeCheck' => 'includes/XmlTypeCheck.php',
278278 'ZhClient' => 'includes/ZhClient.php',
 279+ 'ZipDirectoryReader' => 'includes/ZipDirectoryReader.php',
279280
280281 # includes/api
281282 'ApiBase' => 'includes/api/ApiBase.php',
Index: trunk/phase3/includes/DefaultSettings.php
@@ -552,22 +552,16 @@
553553 'text/scriptlet', 'application/x-msdownload',
554554 # Windows metafile, client-side vulnerability on some systems
555555 'application/x-msmetafile',
556 - # A ZIP file may be a valid Java archive containing an applet which exploits the
557 - # same-origin policy to steal cookies
558 - 'application/zip',
559 -
560 - # MS Office OpenXML and other Open Package Conventions files are zip files
561 - # and thus blacklisted just as other zip files. If you remove these entries
562 - # from the blacklist in your local configuration, a malicious file upload
563 - # will be able to compromise the wiki's user accounts, and the user
564 - # accounts of any other website in the same cookie domain.
565 - 'application/x-opc+zip',
566 - 'application/msword',
567 - 'application/vnd.ms-powerpoint',
568 - 'application/vnd.msexcel',
569556 );
570557
571558 /**
 559+ * Allow Java archive uploads.
 560+ * This is not recommended for public wikis since a maliciously-constructed
 561+ * applet running on the same domain as the wiki can steal the user's cookies.
 562+ */
 563+$wgAllowJavaUploads = false;
 564+
 565+/**
572566 * This is a flag to determine whether or not to check file extensions on upload.
573567 *
574568 * WARNING: setting this to false is insecure for public wikis.
Index: trunk/phase3/includes/ZipDirectoryReader.php
@@ -0,0 +1,683 @@
 2+<?php
 3+
 4+/**
 5+ * A class for reading ZIP file directories, for the purposes of upload
 6+ * verification.
 7+ *
 8+ * Only a functional interface is provided: ZipFileReader::read(). No access is
 9+ * given to object instances.
 10+ *
 11+ */
 12+class ZipDirectoryReader {
 13+ /**
 14+ * Read a ZIP file and call a function for each file discovered in it.
 15+ *
 16+ * Because this class is aimed at verification, an error is raised on
 17+ * suspicious or ambiguous input, instead of emulating some standard
 18+ * behaviour.
 19+ *
 20+ * @param $fileName The archive file name
 21+ * @param $callback The callback function. It will be called for each file
 22+ * with a single associative array each time, with members:
 23+ *
 24+ * - name: The file name. Directories conventionally have a trailing
 25+ * slash.
 26+ *
 27+ * - mtime: The file modification time, in MediaWiki 14-char format
 28+ *
 29+ * - size: The uncompressed file size
 30+ *
 31+ * @param $options An associative array of read options, with the option
 32+ * name in the key. This may currently contain:
 33+ *
 34+ * - zip64: If this is set to true, then we will emulate a
 35+ * library with ZIP64 support, like OpenJDK 7. If it is set to
 36+ * false, then we will emulate a library with no knowledge of
 37+ * ZIP64.
 38+ *
 39+ * NOTE: The ZIP64 code is untested and probably doesn't work. It
 40+ * turned out to be easier to just reject ZIP64 archive uploads,
 41+ * since they are likely to be very rare. Confirming safety of a
 42+ * ZIP64 file is fairly complex. What do you do with a file that is
 43+ * ambiguous and broken when read with a non-ZIP64 reader, but valid
 44+ * when read with a ZIP64 reader? This situation is normal for a
 45+ * valid ZIP64 file, and working out what non-ZIP64 readers will make
 46+ * of such a file is not trivial.
 47+ *
 48+ * @return A Status object. The following fatal errors are defined:
 49+ *
 50+ * - zip-file-open-error: The file could not be opened.
 51+ *
 52+ * - zip-wrong-format: The file does not appear to be a ZIP file.
 53+ *
 54+ * - zip-bad: There was something wrong or ambiguous about the file
 55+ * data.
 56+ *
 57+ * - zip-unsupported: The ZIP file uses features which
 58+ * ZipDirectoryReader does not support.
 59+ *
 60+ * The default messages for those fatal errors are written in a way that
 61+ * makes sense for upload verification.
 62+ *
 63+ * If a fatal error is returned, more information about the error will be
 64+ * available in the debug log.
 65+ *
 66+ * Note that the callback function may be called any number of times before
 67+ * a fatal error is returned. If this occurs, the data sent to the callback
 68+ * function should be discarded.
 69+ */
 70+ public static function read( $fileName, $callback, $options = array() ) {
 71+ $zdr = new self( $fileName, $callback, $options );
 72+ return $zdr->execute();
 73+ }
 74+
 75+ /** The file name */
 76+ var $fileName;
 77+
 78+ /** The opened file resource */
 79+ var $file;
 80+
 81+ /** The cached length of the file, or null if it has not been loaded yet. */
 82+ var $fileLength;
 83+
 84+ /** A segmented cache of the file contents */
 85+ var $buffer;
 86+
 87+ /** The file data callback */
 88+ var $callback;
 89+
 90+ /** The ZIP64 mode */
 91+ var $zip64 = false;
 92+
 93+ /** Stored headers */
 94+ var $eocdr, $eocdr64, $eocdr64Locator;
 95+
 96+ /** The "extra field" ID for ZIP64 central directory entries */
 97+ const ZIP64_EXTRA_HEADER = 0x0001;
 98+
 99+ /** The segment size for the file contents cache */
 100+ const SEGSIZE = 16384;
 101+
 102+ /** The index of the "general field" bit for UTF-8 file names */
 103+ const GENERAL_UTF8 = 11;
 104+
 105+ /** The index of the "general field" bit for central directory encryption */
 106+ const GENERAL_CD_ENCRYPTED = 13;
 107+
 108+
 109+ /**
 110+ * Private constructor
 111+ */
 112+ protected function __construct( $fileName, $callback, $options ) {
 113+ $this->fileName = $fileName;
 114+ $this->callback = $callback;
 115+
 116+ if ( isset( $options['zip64'] ) ) {
 117+ $this->zip64 = $options['zip64'];
 118+ }
 119+ }
 120+
 121+ /**
 122+ * Read the directory according to settings in $this.
 123+ */
 124+ function execute() {
 125+ $this->file = fopen( $this->fileName, 'r' );
 126+ $this->data = array();
 127+ if ( !$this->file ) {
 128+ return Status::newFatal( 'zip-file-open-error' );
 129+ }
 130+
 131+ $status = Status::newGood();
 132+ try {
 133+ $this->readEndOfCentralDirectoryRecord();
 134+ if ( $this->zip64 ) {
 135+ list( $offset, $size ) = $this->findZip64CentralDirectory();
 136+ $this->readCentralDirectory( $offset, $size );
 137+ } else {
 138+ if ( $this->eocdr['CD size'] == 0xffffffff
 139+ || $this->eocdr['CD offset'] == 0xffffffff
 140+ || $this->eocdr['CD entries total'] == 0xffff )
 141+ {
 142+ $this->error( 'zip-unsupported', 'Central directory header indicates ZIP64, ' .
 143+ 'but we are in legacy mode. Rejecting this upload is necessary to avoid '.
 144+ 'opening vulnerabilities on clients using OpenJDK 7 or later.' );
 145+ }
 146+
 147+ list( $offset, $size ) = $this->findOldCentralDirectory();
 148+ $this->readCentralDirectory( $offset, $size );
 149+ }
 150+ } catch ( ZipDirectoryReaderError $e ) {
 151+ $status->fatal( $e->getErrorCode() );
 152+ }
 153+
 154+ fclose( $this->file );
 155+ return $status;
 156+ }
 157+
 158+ /**
 159+ * Throw an error, and log a debug message
 160+ */
 161+ function error( $code, $debugMessage ) {
 162+ wfDebug( __CLASS__.": Fatal error: $debugMessage\n" );
 163+ throw new ZipDirectoryReaderError( $code );
 164+ }
 165+
 166+ /**
 167+ * Read the header which is at the end of the central directory,
 168+ * unimaginatively called the "end of central directory record" by the ZIP
 169+ * spec.
 170+ */
 171+ function readEndOfCentralDirectoryRecord() {
 172+ $info = array(
 173+ 'signature' => 4,
 174+ 'disk' => 2,
 175+ 'CD start disk' => 2,
 176+ 'CD entries this disk' => 2,
 177+ 'CD entries total' => 2,
 178+ 'CD size' => 4,
 179+ 'CD offset' => 4,
 180+ 'file comment length' => 2,
 181+ );
 182+ $structSize = $this->getStructSize( $info );
 183+ $startPos = $this->getFileLength() - 65536 - $structSize;
 184+ if ( $startPos < 0 ) {
 185+ $startPos = 0;
 186+ }
 187+
 188+ $block = $this->getBlock( $startPos );
 189+ $sigPos = strrpos( $block, "PK\x05\x06" );
 190+ if ( $sigPos === false ) {
 191+ $this->error( 'zip-wrong-format',
 192+ "zip file lacks EOCDR signature. It probably isn't a zip file." );
 193+ }
 194+
 195+ $this->eocdr = $this->unpack( substr( $block, $sigPos ), $info );
 196+ $this->eocdr['EOCDR size'] = $structSize + $this->eocdr['file comment length'];
 197+
 198+ if ( $structSize + $this->eocdr['file comment length'] != strlen( $block ) - $sigPos ) {
 199+ $this->error( 'zip-bad', 'trailing bytes after the end of the file comment' );
 200+ }
 201+ if ( $this->eocdr['disk'] !== 0
 202+ || $this->eocdr['CD start disk'] !== 0 )
 203+ {
 204+ $this->error( 'zip-unsupported', 'more than one disk (in EOCDR)' );
 205+ }
 206+ $this->eocdr += $this->unpack(
 207+ $block,
 208+ array( 'file comment' => array( 'string', $this->eocdr['file comment length'] ) ),
 209+ $sigPos + $structSize );
 210+ $this->eocdr['position'] = $startPos + $sigPos;
 211+ }
 212+
 213+ /**
 214+ * Read the header called the "ZIP64 end of central directory locator". An
 215+ * error will be raised if it does not exist.
 216+ */
 217+ function readZip64EndOfCentralDirectoryLocator() {
 218+ $info = array(
 219+ 'signature' => array( 'string', 4 ),
 220+ 'eocdr64 start disk' => 4,
 221+ 'eocdr64 offset' => 8,
 222+ 'number of disks' => 4,
 223+ );
 224+ $structSize = $this->getStructSize( $info );
 225+
 226+ $block = $this->getBlock( $this->getFileLength() - $this->eocdr['EOCDR size']
 227+ - $structSize, $structSize );
 228+ $this->eocdr64Locator = $data = $this->unpack( $block, $info );
 229+
 230+ if ( $data['signature'] !== "PK\x06\x07" ) {
 231+ // Note: Java will allow this and continue to read the
 232+ // EOCDR64, so we have to reject the upload, we can't
 233+ // just use the EOCDR header instead.
 234+ $this->error( 'zip-bad', 'wrong signature on Zip64 end of central directory locator' );
 235+ }
 236+ }
 237+
 238+ /**
 239+ * Read the header called the "ZIP64 end of central directory record". It
 240+ * may replace the regular "end of central directory record" in ZIP64 files.
 241+ */
 242+ function readZip64EndOfCentralDirectoryRecord() {
 243+ if ( $this->eocdr64Locator['eocdr64 start disk'] != 0
 244+ || $this->eocdr64Locator['number of disks'] != 0 )
 245+ {
 246+ $this->error( 'zip-unsupported', 'more than one disk (in EOCDR64 locator)' );
 247+ }
 248+
 249+ $info = array(
 250+ 'signature' => array( 'string', 4 ),
 251+ 'EOCDR64 size' => 8,
 252+ 'version made by' => 2,
 253+ 'version needed' => 2,
 254+ 'disk' => 4,
 255+ 'CD start disk' => 4,
 256+ 'CD entries this disk' => 8,
 257+ 'CD entries total' => 8,
 258+ 'CD size' => 8,
 259+ 'CD offset' => 8
 260+ );
 261+ $structSize = $this->getStructSize( $info );
 262+ $block = $this->getBlock( $this->eocdr64Locator['eocdr64 offset'], $structSize );
 263+ $this->eocdr64 = $data = $this->unpack( $block, $info );
 264+ if ( $data['signature'] !== "PK\x06\x06" ) {
 265+ $this->error( 'zip-bad', 'wrong signature on Zip64 end of central directory record' );
 266+ }
 267+ if ( $data['disk'] !== 0
 268+ || $data['CD start disk'] !== 0 )
 269+ {
 270+ $this->error( 'zip-unsupported', 'more than one disk (in EOCDR64)' );
 271+ }
 272+ }
 273+
 274+ /**
 275+ * Find the location of the central directory, as would be seen by a
 276+ * non-ZIP64 reader.
 277+ *
 278+ * @return List containing offset, size and end position.
 279+ */
 280+ function findOldCentralDirectory() {
 281+ $size = $this->eocdr['CD size'];
 282+ $offset = $this->eocdr['CD offset'];
 283+ $endPos = $this->eocdr['position'];
 284+
 285+ // Some readers use the EOCDR position instead of the offset field
 286+ // to find the directory, so to be safe, we check if they both agree.
 287+ if ( $offset + $size != $endPos ) {
 288+ $this->error( 'zip-bad', 'the central directory does not immediately precede the end ' .
 289+ 'of central directory record' );
 290+ }
 291+ return array( $offset, $size );
 292+ }
 293+
 294+ /**
 295+ * Find the location of the central directory, as would be seen by a
 296+ * ZIP64-compliant reader.
 297+ *
 298+ * @return List containing offset, size and end position.
 299+ */
 300+ function findZip64CentralDirectory() {
 301+ // The spec is ambiguous about the exact rules of precedence between the
 302+ // ZIP64 headers and the original headers. Here we follow zip_util.c
 303+ // from OpenJDK 7.
 304+ $size = $this->eocdr['CD size'];
 305+ $offset = $this->eocdr['CD offset'];
 306+ $numEntries = $this->eocdr['CD entries total'];
 307+ $endPos = $this->eocdr['position'];
 308+ if ( $size == 0xffffffff
 309+ || $offset == 0xffffffff
 310+ || $numEntries == 0xffff )
 311+ {
 312+ $this->readZip64EndOfCentralDirectoryLocator();
 313+
 314+ if ( isset( $this->eocdr64Locator['eocdr64 offset'] ) ) {
 315+ $this->readZip64EndOfCentralDirectoryRecord();
 316+ if ( isset( $this->eocdr64['CD offset'] ) ) {
 317+ $size = $this->eocdr64['CD size'];
 318+ $offset = $this->eocdr64['CD offset'];
 319+ $endPos = $this->eocdr64Locator['eocdr64 offset'];
 320+ }
 321+ }
 322+ }
 323+ // Some readers use the EOCDR position instead of the offset field
 324+ // to find the directory, so to be safe, we check if they both agree.
 325+ if ( $offset + $size != $endPos ) {
 326+ $this->error( 'zip-bad', 'the central directory does not immediately precede the end ' .
 327+ 'of central directory record' );
 328+ }
 329+ return array( $offset, $size );
 330+ }
 331+
 332+ /**
 333+ * Read the central directory at the given location
 334+ */
 335+ function readCentralDirectory( $offset, $size ) {
 336+ $block = $this->getBlock( $offset, $size );
 337+
 338+ $fixedInfo = array(
 339+ 'signature' => array( 'string', 4 ),
 340+ 'version made by' => 2,
 341+ 'version needed' => 2,
 342+ 'general bits' => 2,
 343+ 'compression method' => 2,
 344+ 'mod time' => 2,
 345+ 'mod date' => 2,
 346+ 'crc-32' => 4,
 347+ 'compressed size' => 4,
 348+ 'uncompressed size' => 4,
 349+ 'name length' => 2,
 350+ 'extra field length' => 2,
 351+ 'comment length' => 2,
 352+ 'disk number start' => 2,
 353+ 'internal attrs' => 2,
 354+ 'external attrs' => 4,
 355+ 'local header offset' => 4,
 356+ );
 357+ $fixedSize = $this->getStructSize( $fixedInfo );
 358+
 359+ $pos = 0;
 360+ while ( $pos < $size ) {
 361+ $data = $this->unpack( $block, $fixedInfo, $pos );
 362+ $pos += $fixedSize;
 363+
 364+ if ( $data['signature'] !== "PK\x01\x02" ) {
 365+ $this->error( 'zip-bad', 'Invalid signature found in directory entry' );
 366+ }
 367+
 368+ $variableInfo = array(
 369+ 'name' => array( 'string', $data['name length'] ),
 370+ 'extra field' => array( 'string', $data['extra field length'] ),
 371+ 'comment' => array( 'string', $data['comment length'] ),
 372+ );
 373+ $data += $this->unpack( $block, $variableInfo, $pos );
 374+ $pos += $this->getStructSize( $variableInfo );
 375+
 376+ if ( $this->zip64 && (
 377+ $data['compressed size'] == 0xffffffff
 378+ || $data['uncompressed size'] == 0xffffffff
 379+ || $data['local header offset'] == 0xffffffff ) )
 380+ {
 381+ $zip64Data = $this->unpackZip64Extra( $data['extra field'] );
 382+ if ( $zip64Data ) {
 383+ $data = $zip64Data + $data;
 384+ }
 385+ }
 386+
 387+ if ( $this->testBit( $data['general bits'], self::GENERAL_CD_ENCRYPTED ) ) {
 388+ $this->error( 'zip-unsupported', 'central directory encryption is not supported' );
 389+ }
 390+
 391+ // Convert the timestamp into MediaWiki format
 392+ // For the format, please see the MS-DOS 2.0 Programmer's Reference,
 393+ // pages 3-5 and 3-6.
 394+ $time = $data['mod time'];
 395+ $date = $data['mod date'];
 396+
 397+ $year = 1980 + ( $date >> 9 );
 398+ $month = ( $date >> 5 ) & 15;
 399+ $day = $date & 31;
 400+ $hour = ( $time >> 11 ) & 31;
 401+ $minute = ( $time >> 5 ) & 63;
 402+ $second = ( $time & 31 ) * 2;
 403+ $timestamp = sprintf( "%04d%02d%02d%02d%02d%02d",
 404+ $year, $month, $day, $hour, $minute, $second );
 405+
 406+ // Convert the character set in the file name
 407+ if ( !function_exists( 'iconv' )
 408+ || $this->testBit( $data['general bits'], self::GENERAL_UTF8 ) )
 409+ {
 410+ $name = $data['name'];
 411+ } else {
 412+ $name = iconv( 'CP437', 'UTF-8', $data['name'] );
 413+ }
 414+
 415+ // Compile a data array for the user, with a sensible format
 416+ $userData = array(
 417+ 'name' => $name,
 418+ 'mtime' => $timestamp,
 419+ 'size' => $data['uncompressed size'],
 420+ );
 421+ call_user_func( $this->callback, $userData );
 422+ }
 423+ }
 424+
 425+ /**
 426+ * Interpret ZIP64 "extra field" data and return an associative array.
 427+ */
 428+ function unpackZip64Extra( $extraField ) {
 429+ $extraHeaderInfo = array(
 430+ 'id' => 2,
 431+ 'size' => 2,
 432+ );
 433+ $extraHeaderSize = $this->getStructSize( $extraHeaderInfo );
 434+
 435+ $zip64ExtraInfo = array(
 436+ 'uncompressed size' => 8,
 437+ 'compressed size' => 8,
 438+ 'local header offset' => 8,
 439+ 'disk number start' => 4,
 440+ );
 441+
 442+ $extraPos = 0;
 443+ $extraField = $data['extra field'];
 444+ while ( $extraPos < strlen( $extraField ) ) {
 445+ $extra = $this->unpack( $extraField, $extraHeaderInfo, $extraPos );
 446+ $extraPos += $extraHeaderSize;
 447+ $extra += $this->unpack( $extraField,
 448+ array( 'data' => array( 'string', $extra['size'] ) ),
 449+ $extraPos );
 450+ $extraPos += $extra['size'];
 451+
 452+ if ( $extra['id'] == self::ZIP64_EXTRA_HEADER ) {
 453+ return $this->unpack( $extra['data'], $zip64ExtraInfo );
 454+ }
 455+ }
 456+
 457+ return false;
 458+ }
 459+
 460+ /**
 461+ * Get the length of the file.
 462+ */
 463+ function getFileLength() {
 464+ if ( $this->fileLength === null ) {
 465+ $stat = fstat( $this->file );
 466+ $this->fileLength = $stat['size'];
 467+ }
 468+ return $this->fileLength;
 469+ }
 470+
 471+ /**
 472+ * Get the file contents from a given offset. If there are not enough bytes
 473+ * in the file to satisfy the request, an exception will be thrown.
 474+ *
 475+ * @param $start The byte offset of the start of the block.
 476+ * @param $length The number of bytes to return. If omitted, the remainder
 477+ * of the file will be returned.
 478+ *
 479+ * @return string
 480+ */
 481+ function getBlock( $start, $length = null ) {
 482+ $fileLength = $this->getFileLength();
 483+ if ( $start >= $fileLength ) {
 484+ $this->error( 'zip-bad', "getBlock() requested position $start, " .
 485+ "file length is $fileLength" );
 486+ }
 487+ if ( $length === null ) {
 488+ $length = $fileLength - $start;
 489+ }
 490+ $end = $start + $length;
 491+ if ( $end > $fileLength ) {
 492+ $this->error( 'zip-bad', "getBlock() requested end position $end, " .
 493+ "file length is $fileLength" );
 494+ }
 495+ $startSeg = floor( $start / self::SEGSIZE );
 496+ $endSeg = ceil( $end / self::SEGSIZE );
 497+
 498+ $block = '';
 499+ for ( $segIndex = $startSeg; $segIndex <= $endSeg; $segIndex++ ) {
 500+ $block .= $this->getSegment( $segIndex );
 501+ }
 502+
 503+ $block = substr( $block,
 504+ $start - $startSeg * self::SEGSIZE,
 505+ $length );
 506+
 507+ if ( strlen( $block ) < $length ) {
 508+ $this->error( 'zip-bad', 'getBlock() returned an unexpectedly small amount of data' );
 509+ }
 510+
 511+ return $block;
 512+ }
 513+
 514+ /**
 515+ * Get a section of the file starting at position $segIndex * self::SEGSIZE,
 516+ * of length self::SEGSIZE. The result is cached. This is a helper function
 517+ * for getBlock().
 518+ *
 519+ * If there are not enough bytes in the file to satsify the request, the
 520+ * return value will be truncated. If a request is made for a segment beyond
 521+ * the end of the file, an empty string will be returned.
 522+ */
 523+ function getSegment( $segIndex ) {
 524+ if ( !isset( $this->buffer[$segIndex] ) ) {
 525+ $bytePos = $segIndex * self::SEGSIZE;
 526+ if ( $bytePos >= $this->getFileLength() ) {
 527+ $this->buffer[$segIndex] = '';
 528+ return '';
 529+ }
 530+ if ( fseek( $this->file, $bytePos ) ) {
 531+ $this->error( 'zip-bad', "seek to $bytePos failed" );
 532+ }
 533+ $seg = fread( $this->file, self::SEGSIZE );
 534+ if ( $seg === false ) {
 535+ $this->error( 'zip-bad', "read from $bytePos failed" );
 536+ }
 537+ $this->buffer[$segIndex] = $seg;
 538+ }
 539+ return $this->buffer[$segIndex];
 540+ }
 541+
 542+ /**
 543+ * Get the size of a structure in bytes. See unpack() for the format of $struct.
 544+ */
 545+ function getStructSize( $struct ) {
 546+ $size = 0;
 547+ foreach ( $struct as $key => $type ) {
 548+ if ( is_array( $type ) ) {
 549+ list( $typeName, $fieldSize ) = $type;
 550+ $size += $fieldSize;
 551+ } else {
 552+ $size += $type;
 553+ }
 554+ }
 555+ return $size;
 556+ }
 557+
 558+ /**
 559+ * Unpack a binary structure. This is like the built-in unpack() function
 560+ * except nicer.
 561+ *
 562+ * @param $string The binary data input
 563+ *
 564+ * @param $struct An associative array giving structure members and their
 565+ * types. In the key is the field name. The value may be either an
 566+ * integer, in which case the field is a little-endian unsigned integer
 567+ * encoded in the given number of bytes, or an array, in which case the
 568+ * first element of the array is the type name, and the subsequent
 569+ * elements are type-dependent parameters. Only one such type is defined:
 570+ * - "string": The second array element gives the length of string.
 571+ * Not null terminated.
 572+ *
 573+ * @param $offset The offset into the string at which to start unpacking.
 574+ *
 575+ * @return Unpacked associative array. Note that large integers in the input
 576+ * may be represented as floating point numbers in the return value, so
 577+ * the use of weak comparison is advised.
 578+ */
 579+ function unpack( $string, $struct, $offset = 0 ) {
 580+ $size = $this->getStructSize( $struct );
 581+ if ( $offset + $size > strlen( $string ) ) {
 582+ $this->error( 'zip-bad', 'unpack() would run past the end of the supplied string' );
 583+ }
 584+
 585+ $data = array();
 586+ $pos = $offset;
 587+ foreach ( $struct as $key => $type ) {
 588+ if ( is_array( $type ) ) {
 589+ list( $typeName, $fieldSize ) = $type;
 590+ switch ( $typeName ) {
 591+ case 'string':
 592+ $data[$key] = substr( $string, $pos, $fieldSize );
 593+ $pos += $fieldSize;
 594+ break;
 595+ default:
 596+ throw new MWException( __METHOD__.": invalid type \"$typeName\"" );
 597+ }
 598+ } else {
 599+ // Unsigned little-endian integer
 600+ $length = intval( $type );
 601+ $bytes = substr( $string, $pos, $length );
 602+
 603+ // Calculate the value. Use an algorithm which automatically
 604+ // upgrades the value to floating point if necessary.
 605+ $value = 0;
 606+ for ( $i = $length - 1; $i >= 0; $i-- ) {
 607+ $value *= 256;
 608+ $value += ord( $string[$pos + $i] );
 609+ }
 610+
 611+ // Throw an exception if there was loss of precision
 612+ if ( $value > pow( 2, 52 ) ) {
 613+ $this->error( 'zip-unsupported', 'number too large to be stored in a double. ' .
 614+ 'This could happen if we tried to unpack a 64-bit structure ' .
 615+ 'at an invalid location.' );
 616+ }
 617+ $data[$key] = $value;
 618+ $pos += $length;
 619+ }
 620+ }
 621+
 622+ return $data;
 623+ }
 624+
 625+ /**
 626+ * Returns a bit from a given position in an integer value, converted to
 627+ * boolean.
 628+ *
 629+ * @param $value integer
 630+ * @param $bitIndex The index of the bit, where 0 is the LSB.
 631+ */
 632+ function testBit( $value, $bitIndex ) {
 633+ return (bool)( ( $value >> $bitIndex ) & 1 );
 634+ }
 635+
 636+ /**
 637+ * Debugging helper function which dumps a string in hexdump -C format.
 638+ */
 639+ function hexDump( $s ) {
 640+ $n = strlen( $s );
 641+ for ( $i = 0; $i < $n; $i += 16 ) {
 642+ printf( "%08X ", $i );
 643+ for ( $j = 0; $j < 16; $j++ ) {
 644+ print " ";
 645+ if ( $j == 8 ) {
 646+ print " ";
 647+ }
 648+ if ( $i + $j >= $n ) {
 649+ print " ";
 650+ } else {
 651+ printf( "%02X", ord( $s[$i + $j] ) );
 652+ }
 653+ }
 654+
 655+ print " |";
 656+ for ( $j = 0; $j < 16; $j++ ) {
 657+ if ( $i + $j >= $n ) {
 658+ print " ";
 659+ } elseif ( ctype_print( $s[$i + $j] ) ) {
 660+ print $s[$i + $j];
 661+ } else {
 662+ print '.';
 663+ }
 664+ }
 665+ print "|\n";
 666+ }
 667+ }
 668+}
 669+
 670+/**
 671+ * Internal exception class. Will be caught by private code.
 672+ */
 673+class ZipDirectoryReaderError extends Exception {
 674+ var $code;
 675+
 676+ function __construct( $code ) {
 677+ $this->code = $code;
 678+ parent::__construct( "ZipDirectoryReader error: $code" );
 679+ }
 680+
 681+ function getErrorCode() {
 682+ return $this->code;
 683+ }
 684+}
Property changes on: trunk/phase3/includes/ZipDirectoryReader.php
___________________________________________________________________
Added: svn:eol-style
1685 + native
Index: trunk/phase3/languages/messages/MessagesEn.php
@@ -2185,6 +2185,8 @@
21862186 'php-uploaddisabledtext' => 'File uploads are disabled in PHP.
21872187 Please check the file_uploads setting.',
21882188 'uploadscripted' => 'This file contains HTML or script code that may be erroneously interpreted by a web browser.',
 2189+'uploadjava' => 'The file is a ZIP file which contains a Java .class file.
 2190+Uploading Java files is not allowed, because they can cause security restrictions to be bypassed.',
21892191 'uploadvirus' => 'The file contains a virus!
21902192 Details: $1',
21912193 'upload-source' => 'Source file',
@@ -2239,6 +2241,14 @@
22402242 'upload-unknown-size' => 'Unknown size',
22412243 'upload-http-error' => 'An HTTP error occured: $1',
22422244
 2245+# ZipDirectoryReader
 2246+'zip-file-open-error' => 'An error was encountered when opening the file for ZIP checks.',
 2247+'zip-wrong-format' => 'The specified file was not a ZIP file.',
 2248+'zip-bad' => 'The file is a corrupt or otherwise unreadable ZIP file.
 2249+It cannot be properly checked for security.',
 2250+'zip-unsupported' => 'The file is a ZIP file which uses ZIP features not supported by MediaWiki.
 2251+It cannot be properly checked for security.',
 2252+
22432253 # Special:UploadStash
22442254 'uploadstash' => 'Upload stash',
22452255 '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: trunk/phase3/RELEASE-NOTES
@@ -74,6 +74,10 @@
7575 * (bug 27159) Make email confirmation code expiration time configurable
7676 * CSS/JS for each user group is imported from MediaWiki:Sysop.js,
7777 MediaWiki:Autoconfirmed.css, etc.
 78+* (bug 24230) Uploads of ZIP types, such as MS Office or OpenOffice can now be
 79+ safely enabled. A ZIP file reader was added which can scan a ZIP file for
 80+ potentially dangerous Java applets. This allows applets to be blocked
 81+ specifically, rather than all ZIP files being blocked.
7882
7983 === Bug fixes in 1.18 ===
8084 * (bug 23119) WikiError class and subclasses are now marked as deprecated

Sign-offs

UserFlagDate
Bryaninspected21:47, 14 April 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r82924Minor fix in ZIP64 code from r82783. Note that this code is, before and after...tstarling02:26, 28 February 2011

Comments

#Comment by Reedy (talk | contribs)   13:16, 25 February 2011

ZipDirectoryReader, line 442

		$extraField = $data['extra field'];

$data is undefined at this point of usage

#Comment by Reedy (talk | contribs)   13:16, 25 February 2011

Fail.

$extraField = $data['extra field'];
#Comment by Tim Starling (talk | contribs)   13:20, 25 February 2011

I said in the ZDR::read() doc comment that the ZIP64 option is untested and probably doesn't work. Are you saying I should fix it, or just adjust the comment to say that it definitely doesn't work?

#Comment by Reedy (talk | contribs)   15:55, 25 February 2011

Sure, but

	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']; //$data is undefined at this point. And it can't have come from somewhere magical like an extract
		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;
	}

The usage of the array $data on line 442 is on an undefined variable

#Comment by Tim Starling (talk | contribs)   02:22, 28 February 2011

I understand that. I'm saying that it's expected that unused, untested code has bugs in it, and that I think it would be a waste of time to fix one particular bug when it's very likely that there are several others. But I guess I can fix it if it offends you somehow.

#Comment by 😂 (talk | contribs)   13:53, 6 July 2011

Is there a reason you didn't make ZipDirectoryReaderError extend MWException and Exception instead? From the looks of it the idea was to make it reusable like some of our code in /includes/libs/. The only thing I see preventing that is the use of Status objects and the one wfDebug() call.

#Comment by 😂 (talk | contribs)   13:58, 6 July 2011

Wow that first sentence makes little sense. I meant: why make ZipDirectoryReaderError extend Exception over MWException?

#Comment by Tim Starling (talk | contribs)   23:46, 6 July 2011

Maybe at an early stage of development, it was imagined as a standalone library. But it turns out that the PEAR ZIP library was not as bad as I thought it was, so it should be sufficient for most people who need a standalone library. So the main motivation for completing ZipDirectoryReader became the fact that it could be tuned for MediaWiki and integrated with it.

#Comment by 😂 (talk | contribs)   00:07, 7 July 2011

Ah ok. Thanks for explaining. I guess those few Exceptions can become MWExceptions then.

Status & tagging log