gmane.org.wikimedia.mediawiki.cvs

[MediaWiki-commits] [Gerrit] Updating MobileFrontend to productiontip - change (mediawiki/core)

awjrichards (Code Review — 2013-05-21T20:51:05

awjrichards has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64856


Change subject: Updating MobileFrontend to production tip
......................................................................

Updating MobileFrontend to production tip

Change-Id: Idd3da6688736fa23d5c4d9cf31d18872712cd65e
---
M extensions/MobileFrontend
1 file changed, 0 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/56/64856/1

diff --git a/extensions/MobileFrontend b/extensions/MobileFrontend
index 05d3a18..8ee5fdf 160000
--- a/extensions/MobileFrontend
+++ b/extensions/MobileFrontend
-Subproject commit 05d3a1821cf77bda90f7bbab9eb6be7c5cdd9b22
+Subproject commit 8ee5fdf6954e2feaad42b35012e5230e92ad9bae

[MediaWiki-commits] [Gerrit] Updated release notes and versionnumber - change (mediawiki/core)

jenkins-bot (Code Review — 2013-05-21T20:50:27

jenkins-bot has submitted this change and it was merged.

Change subject: Updated release notes and version number
......................................................................


Updated release notes and version number

Change-Id: I1fb122b2e946df330578474ace5c1da7e163f5b1
---
M RELEASE-NOTES-1.19
M includes/DefaultSettings.php
2 files changed, 9 insertions(+), 1 deletion(-)

Approvals:
  Krinkle: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/RELEASE-NOTES-1.19 b/RELEASE-NOTES-1.19
index a61c237..560ee3b 100644
--- a/RELEASE-NOTES-1.19
+++ b/RELEASE-NOTES-1.19
< at >< at > -3,6 +3,14 < at >< at >
 Security reminder: MediaWiki does not require PHP's register_globals
 setting since version 1.2.0. If you have it on, turn it '''off''' if you can.
 
+== MediaWiki 1.19.7 ==
+
+This is a security and maintenance release of the MediaWiki 1.19 branch
+
+=== Changes since 1.19.6 ===
+* (bug 48306) SECURITY: Run file validation checks on  chunked uploads, and chunks
+  of upload, during the upload process.
+
 == MediaWiki 1.19.6 ==
 
 This is a security and maintenance release of the MediaWiki 1.19 branch
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 6aa5c9e..84cc9c2 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
< at >< at > -33,7 +33,7 < at >< at >
 /** < at >endcond */
 
 /** MediaWiki version number */
-$wgVersion = '1.19.6';
+$wgVersion = '1.19.7';
 
 /** Name of the site. It must be changed in LocalSettings.php */
 $wgSitename = 'MediaWiki';

[MediaWiki-commits] [Gerrit] Updated release notes and versionnumber - change (mediawiki/core)

jenkins-bot (Code Review — 2013-05-21T20:43:30

jenkins-bot has submitted this change and it was merged.

Change subject: Updated release notes and version number
......................................................................


Updated release notes and version number

Change-Id: I630010e6371ac4ba9ea24e5f2f48e891d3e41c2f
---
M RELEASE-NOTES-1.20
M includes/DefaultSettings.php
2 files changed, 6 insertions(+), 3 deletions(-)

Approvals:
  CSteipp: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/RELEASE-NOTES-1.20 b/RELEASE-NOTES-1.20
index 2da9e35..d4f399a 100644
--- a/RELEASE-NOTES-1.20
+++ b/RELEASE-NOTES-1.20
< at >< at > -6,11 +6,14 < at >< at >
 
 == MediaWiki 1.20.6 ==
 
-This is a maintenance release of the MediaWiki 1.20 branch.
+This is a security and maintenance release of the MediaWiki 1.20 branch.
 
 === Changes since 1.20.5 ===
+* (bug 48306) SECURITY: Run file validation checks on chunked uploads, and chunks
+  of upload, during the upload process.
+* (bug 44327) mediawiki.user: Use session ID instead of 1-year cross-session cookies
 * (bug 47202) wikibits: FF2Fixes.css should not be loaded in Firefox 20.
-
+* (bug 31044) Make ResourceLoader behave in read-only mode
 
 == MediaWiki 1.20.5 ==
 
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 6abc2b1..710605a 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
< at >< at > -59,7 +59,7 < at >< at >
 $wgConf = new SiteConfiguration;
 
 /** MediaWiki version number */
-$wgVersion = '1.20.5';
+$wgVersion = '1.20.6';
 
 /** Name of the site. It must be changed in LocalSettings.php */
 $wgSitename = 'MediaWiki';

[MediaWiki-commits] [Gerrit] SECURITY: Do checks on all uploadtypes - change (mediawiki/core)

jenkins-bot (Code Review — 2013-05-21T20:38:25

jenkins-bot has submitted this change and it was merged.

Change subject: SECURITY: Do checks on all upload types
......................................................................


SECURITY: Do checks on all upload types

Also, verify file before stashing it

Based on change Ib2474cb778d53959a4f479e53d0392f916b18d83

Change-Id: I2b230af2f23c4303acdb695468608df19a88d16d
---
M includes/AutoLoader.php
M includes/api/ApiUpload.php
M includes/upload/UploadBase.php
M includes/upload/UploadFromChunks.php
M includes/upload/UploadFromStash.php
M includes/upload/UploadStash.php
6 files changed, 111 insertions(+), 30 deletions(-)

Approvals:
  CSteipp: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 7611ed9..2bf134b 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
< at >< at > -883,6 +883,7 < at >< at >
 
 # includes/upload
 'UploadBase' => 'includes/upload/UploadBase.php',
+'UploadChunkVerificationException' => 'includes/upload/UploadFromChunks.php',
 'UploadFromFile' => 'includes/upload/UploadFromFile.php',
 'UploadFromChunks' => 'includes/upload/UploadFromChunks.php',
 'UploadFromStash' => 'includes/upload/UploadFromStash.php',
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index fdc1eff..922b89d 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
< at >< at > -175,7 +175,12 < at >< at >
 $chunkPath = $request->getFileTempname( 'chunk' );
 $chunkSize = $request->getUpload( 'chunk' )->getSize();
 if ($this->mParams['offset'] == 0) {
-$result['filekey'] = $this->performStash();
+try {
+$result['filekey'] = $this->performStash();
+} catch ( MWException $e ) {
+// FIXME: Error handling here is wrong/different from rest of this
+$this->dieUsage( $e->getMessage(), 'stashfailed' );
+}
 } else {
 $status = $this->mUpload->addChunk($chunkPath, $chunkSize,
 $this->mParams['offset']);
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 3995519..5f09c04 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
< at >< at > -301,6 +301,8 < at >< at >
 /**
  * Verify the mime type
  *
+ * < at >note Only checks that it is not an evil mime. The does it have
+ *  correct extension given its mime type check is in verifyFile.
  * < at >param $mime string representing the mime
  * < at >return mixed true if the file is verified, an array otherwise
  */
< at >< at > -311,11 +313,6 < at >< at >
 global $wgMimeTypeBlacklist;
 if ( $this->checkFileExtension( $mime, $wgMimeTypeBlacklist ) ) {
 return array( 'filetype-badmime', $mime );
-}
-
-# XXX: Missing extension will be caught by validateName() via getTitle()
-if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
-return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
 }
 
 # Check IE type
< at >< at > -342,6 +339,56 < at >< at >
  * < at >return mixed true of the file is verified, array otherwise.
  */
 protected function verifyFile() {
+global $wgVerifyMimeType;
+wfProfileIn( __METHOD__ );
+
+$status = $this->verifyPartialFile();
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+if ( $wgVerifyMimeType ) {
+$this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
+$mime = $this->mFileProps['file-mime'];
+
+# XXX: Missing extension will be caught by validateName() via getTitle()
+if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
+wfProfileOut( __METHOD__ );
+return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
+}
+}
+
+$handler = MediaHandler::getHandler( $mime );
+if ( $handler ) {
+$handlerStatus = $handler->verifyUpload( $this->mTempPath );
+if ( !$handlerStatus->isOK() ) {
+$errors = $handlerStatus->getErrorsArray();
+wfProfileOut( __METHOD__ );
+return reset( $errors );
+}
+}
+
+wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+wfDebug( __METHOD__ . ": all clear; passing.\n" );
+wfProfileOut( __METHOD__ );
+return true;
+}
+
+/**
+ * A verification routine suitable for partial files
+ *
+ * Runs the blacklist checks, but not any checks that may
+ * assume the entire file is present.
+ *
+ * < at >return Mixed true for valid or array with error message key.
+ */
+protected function verifyPartialFile() {
 global $wgAllowJavaUploads, $wgDisableUploadScriptChecks;
 # get the title, even though we are doing nothing with it, because
 # we need to populate mFinalExtension
< at >< at > -392,21 +439,6 < at >< at >
 return array( 'uploadvirus', $virus );
 }
 
-$handler = MediaHandler::getHandler( $mime );
-if ( $handler ) {
-$handlerStatus = $handler->verifyUpload( $this->mTempPath );
-if ( !$handlerStatus->isOK() ) {
-$errors = $handlerStatus->getErrorsArray();
-return reset( $errors );
-}
-}
-
-wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
-if ( $status !== true ) {
-return $status;
-}
-
-wfDebug( __METHOD__ . ": all clear; passing.\n" );
 return true;
 }
 
diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php
index ec83f7d..a76f03d 100644
--- a/includes/upload/UploadFromChunks.php
+++ b/includes/upload/UploadFromChunks.php
< at >< at > -47,6 +47,8 < at >< at >
 // Stash file is the called on creating a new chunk session: 
 $this->mChunkIndex = 0;
 $this->mOffset = 0;
+
+$this->verifyChunk();
 // Create a local stash target
 $this->mLocalFile = parent::stashFile();
 // Update the initial file offset ( based on file size ) 
< at >< at > -105,9 +107,18 < at >< at >
 if( !$status->isOk() ){
 return $status; 
 }
+
+$this->mTempPath = $tmpPath; // file system path
+$this->mFileSize = filesize( $this->mTempPath ); //Since this was set for the last chunk previously
+$ret = $this->verifyUpload();
+if ( $ret['status'] !== UploadBase::OK ) {
+wfDebugLog( 'fileconcatenate', "Verification failed for chunked upload" );
+$status->fatal( $this->getVerificationErrorCode( $ret['status'] ) );
+return $status;
+}
+
 // Update the mTempPath and mLocalFile
 // ( for FileUpload or normal Stash to take over )  
-$this->mTempPath = $tmpPath; // file system path
 $this->mLocalFile = parent::stashFile();
 
 return $status;
< at >< at > -157,6 +168,15 < at >< at >
 if ( $preAppendOffset == $offset ) {
 // Update local chunk index for the current chunk   
 $this->mChunkIndex++;
+try {
+# For some reason mTempPath is set to first part
+$oldTemp = $this->mTempPath;
+$this->mTempPath = $chunkPath;
+$this->verifyChunk();
+$this->mTempPath = $oldTemp;
+} catch ( UploadChunkVerificationException $e ) {
+return Status::newFatal( $e->getMessage() );
+}
 $status = $this->outputChunk( $chunkPath );
 if( $status->isGood() ){
 // Update local offset: 
< at >< at > -270,7 +290,26 < at >< at >
 }
 return $this->mFileKey . '.' . $index ;
 }
+
+/**
+ * Verify that the chunk isn't really an evil html file
+ *
+ * < at >throws UploadChunkVerificationException
+ */
+private function verifyChunk() {
+// Rest mDesiredDestName here so we verify the name as if it were mFileKey
+$oldDesiredDestName = $this->mDesiredDestName;
+$this->mDesiredDestName = $this->mFileKey;
+$this->mTitle = false;
+$res = $this->verifyPartialFile();
+$this->mDesiredDestName = $oldDesiredDestName;
+$this->mTitle = false;
+if( is_array( $res ) ) {
+throw new UploadChunkVerificationException( $res[0] );
+}
+}
 }
 
 class UploadChunkZeroLengthFileException extends MWException {};
 class UploadChunkFileException extends MWException {};
+class UploadChunkVerificationException extends MWException {};
diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php
index f7589bd..014aa53 100644
--- a/includes/upload/UploadFromStash.php
+++ b/includes/upload/UploadFromStash.php
< at >< at > -109,14 +109,9 < at >< at >
 return $this->mSourceType;
 }
 
-/**
- * File has been previously verified so no need to do so again.
- *
- * < at >return bool
+/*
+ * protected function verifyFile() inherited
  */
-protected function verifyFile() {
-return true;
-}
 
 /**
  * Stash the file.
diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php
index ad153d2..73e3811 100644
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
< at >< at > -393,6 +393,7 < at >< at >
  * uploads versus the desired filename. Maybe we can get that passed to us...
  */
 public static function getExtensionForPath( $path ) {
+global $wgFileBlacklist;
 // Does this have an extension?
 $n = strrpos( $path, '.' );
 $extension = null;
< at >< at > -412,7 +413,15 < at >< at >
 throw new UploadStashFileException( "extension is null" );
 }
 
-return File::normalizeExtension( $extension );
+$extension = File::normalizeExtension( $extension );
+if ( in_array( $extension, $wgFileBlacklist ) ) {
+// The file should already be checked for being evil.
+// However, if somehow we got here, we definitely
+// don't want to give it an extension of .php and
+// put it in a web accesible directory.
+return '';
+}
+return $extension;
 }
 
 /**

[MediaWiki-commits] [Gerrit] hide flickr upload if multiple uploadis disabled - change (mediawiki...UploadWizard)

Nischayn22 (Code Review — 2013-05-21T20:37:24

Nischayn22 has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64855


Change subject: hide flickr upload if multiple upload is disabled
......................................................................

hide flickr upload if multiple upload is disabled

Hides flickr upload button if enableMultipleFiles is false and one file is
already uploading.

Change-Id: Ifcf4c016be25be2edebc20b6d575de1a914e33b8
---
M resources/mw.UploadWizard.js
1 file changed, 1 insertion(+), 0 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/UploadWizard refs/changes/55/64855/1

diff --git a/resources/mw.UploadWizard.js b/resources/mw.UploadWizard.js
index e6005d8..b61bb87 100644
--- a/resources/mw.UploadWizard.js
+++ b/resources/mw.UploadWizard.js
< at >< at > -952,6 +952,7 < at >< at >
 _this.$addFile.hide();
 _this.$fileInput = this.$fileInput || $j( '.mwe-upwiz-file-input' );
 _this.$fileInput.hide();
+$j( '#mwe-upwiz-upload-ctrl-flickr-container, #mwe-upwiz-flickr-select-list-container' ).hide();
 }
 
 // add the styling to the filelist, so it has rounded corners and is visible and all.

[MediaWiki-commits] [Gerrit] SECURITY: Do checks on all uploadtypes - change (mediawiki/core)

jenkins-bot (Code Review — 2013-05-21T20:35:28

jenkins-bot has submitted this change and it was merged.

Change subject: SECURITY: Do checks on all upload types
......................................................................


SECURITY: Do checks on all upload types

Also, verify file before stashing it

Change-Id: I5dd027acc994cf7308c9e4d85a61237d802ccee8
---
M includes/AutoLoader.php
M includes/api/ApiUpload.php
M includes/upload/UploadBase.php
M includes/upload/UploadFromChunks.php
M includes/upload/UploadFromStash.php
M includes/upload/UploadStash.php
6 files changed, 111 insertions(+), 33 deletions(-)

Approvals:
  CSteipp: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 9bdfcd2..a8b2202 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
< at >< at > -965,6 +965,7 < at >< at >
 'UnwatchedpagesPage' => 'includes/specials/SpecialUnwatchedpages.php',
 'UploadChunkFileException' => 'includes/upload/UploadFromChunks.php',
 'UploadChunkZeroLengthFileException' => 'includes/upload/UploadFromChunks.php',
+'UploadChunkVerificationException' => 'includes/upload/UploadFromChunks.php',
 'UploadForm' => 'includes/specials/SpecialUpload.php',
 'UploadSourceField' => 'includes/specials/SpecialUpload.php',
 'UserrightsPage' => 'includes/specials/SpecialUserrights.php',
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 3a9b5c5..e7a7849 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
< at >< at > -187,7 +187,12 < at >< at >
 $chunkPath = $request->getFileTempname( 'chunk' );
 $chunkSize = $request->getUpload( 'chunk' )->getSize();
 if ($this->mParams['offset'] == 0) {
-$result['filekey'] = $this->performStash();
+try {
+$result['filekey'] = $this->performStash();
+} catch ( MWException $e ) {
+// FIXME: Error handling here is wrong/different from rest of this
+$this->dieUsage( $e->getMessage(), 'stashfailed' );
+}
 } else {
 $status = $this->mUpload->addChunk($chunkPath, $chunkSize,
 $this->mParams['offset']);
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 3a5733c..0848780 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
< at >< at > -345,6 +345,8 < at >< at >
 /**
  * Verify the mime type
  *
+ * < at >note Only checks that it is not an evil mime. The does it have
+ *  correct extension given its mime type check is in verifyFile.
  * < at >param $mime string representing the mime
  * < at >return mixed true if the file is verified, an array otherwise
  */
< at >< at > -357,12 +359,6 < at >< at >
 if ( $this->checkFileExtension( $mime, $wgMimeTypeBlacklist ) ) {
 wfProfileOut( __METHOD__ );
 return array( 'filetype-badmime', $mime );
-}
-
-# XXX: Missing extension will be caught by validateName() via getTitle()
-if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
-wfProfileOut( __METHOD__ );
-return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
 }
 
 # Check IE type
< at >< at > -391,6 +387,56 < at >< at >
  * < at >return mixed true of the file is verified, array otherwise.
  */
 protected function verifyFile() {
+global $wgVerifyMimeType;
+wfProfileIn( __METHOD__ );
+
+$status = $this->verifyPartialFile();
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+if ( $wgVerifyMimeType ) {
+$this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
+$mime = $this->mFileProps['file-mime'];
+
+# XXX: Missing extension will be caught by validateName() via getTitle()
+if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
+wfProfileOut( __METHOD__ );
+return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
+}
+}
+
+$handler = MediaHandler::getHandler( $mime );
+if ( $handler ) {
+$handlerStatus = $handler->verifyUpload( $this->mTempPath );
+if ( !$handlerStatus->isOK() ) {
+$errors = $handlerStatus->getErrorsArray();
+wfProfileOut( __METHOD__ );
+return reset( $errors );
+}
+}
+
+wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+wfDebug( __METHOD__ . ": all clear; passing.\n" );
+wfProfileOut( __METHOD__ );
+return true;
+}
+
+/**
+ * A verification routine suitable for partial files
+ *
+ * Runs the blacklist checks, but not any checks that may
+ * assume the entire file is present.
+ *
+ * < at >return Mixed true for valid or array with error message key.
+ */
+protected function verifyPartialFile() {
 global $wgAllowJavaUploads, $wgDisableUploadScriptChecks;
 wfProfileIn( __METHOD__ );
 
< at >< at > -449,23 +495,6 < at >< at >
 return array( 'uploadvirus', $virus );
 }
 
-$handler = MediaHandler::getHandler( $mime );
-if ( $handler ) {
-$handlerStatus = $handler->verifyUpload( $this->mTempPath );
-if ( !$handlerStatus->isOK() ) {
-$errors = $handlerStatus->getErrorsArray();
-wfProfileOut( __METHOD__ );
-return reset( $errors );
-}
-}
-
-wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
-if ( $status !== true ) {
-wfProfileOut( __METHOD__ );
-return $status;
-}
-
-wfDebug( __METHOD__ . ": all clear; passing.\n" );
 wfProfileOut( __METHOD__ );
 return true;
 }
diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php
index 0542bba..531f7be 100644
--- a/includes/upload/UploadFromChunks.php
+++ b/includes/upload/UploadFromChunks.php
< at >< at > -69,6 +69,8 < at >< at >
 // Stash file is the called on creating a new chunk session:
 $this->mChunkIndex = 0;
 $this->mOffset = 0;
+
+$this->verifyChunk();
 // Create a local stash target
 $this->mLocalFile = parent::stashFile();
 // Update the initial file offset ( based on file size )
< at >< at > -127,9 +129,18 < at >< at >
 if( !$status->isOk() ){
 return $status;
 }
+
+$this->mTempPath = $tmpPath; // file system path
+$this->mFileSize = filesize( $this->mTempPath ); //Since this was set for the last chunk previously
+$ret = $this->verifyUpload();
+if ( $ret['status'] !== UploadBase::OK ) {
+wfDebugLog( 'fileconcatenate', "Verification failed for chunked upload" );
+$status->fatal( $this->getVerificationErrorCode( $ret['status'] ) );
+return $status;
+}
+
 // Update the mTempPath and mLocalFile
 // ( for FileUpload or normal Stash to take over )
-$this->mTempPath = $tmpPath; // file system path
 $this->mLocalFile = parent::stashFile();
 
 return $status;
< at >< at > -181,6 +192,15 < at >< at >
 if ( $preAppendOffset == $offset ) {
 // Update local chunk index for the current chunk
 $this->mChunkIndex++;
+try {
+# For some reason mTempPath is set to first part
+$oldTemp = $this->mTempPath;
+$this->mTempPath = $chunkPath;
+$this->verifyChunk();
+$this->mTempPath = $oldTemp;
+} catch ( UploadChunkVerificationException $e ) {
+return Status::newFatal( $e->getMessage() );
+}
 $status = $this->outputChunk( $chunkPath );
 if( $status->isGood() ){
 // Update local offset:
< at >< at > -300,7 +320,26 < at >< at >
 }
 return $this->mFileKey . '.' . $index ;
 }
+
+/**
+ * Verify that the chunk isn't really an evil html file
+ *
+ * < at >throws UploadChunkVerificationException
+ */
+private function verifyChunk() {
+// Rest mDesiredDestName here so we verify the name as if it were mFileKey
+$oldDesiredDestName = $this->mDesiredDestName;
+$this->mDesiredDestName = $this->mFileKey;
+$this->mTitle = false;
+$res = $this->verifyPartialFile();
+$this->mDesiredDestName = $oldDesiredDestName;
+$this->mTitle = false;
+if( is_array( $res ) ) {
+throw new UploadChunkVerificationException( $res[0] );
+}
+}
 }
 
 class UploadChunkZeroLengthFileException extends MWException {};
 class UploadChunkFileException extends MWException {};
+class UploadChunkVerificationException extends MWException {};
diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php
index 607965f..d79641c 100644
--- a/includes/upload/UploadFromStash.php
+++ b/includes/upload/UploadFromStash.php
< at >< at > -129,14 +129,9 < at >< at >
 return $this->mSourceType;
 }
 
-/**
- * File has been previously verified so no need to do so again.
- *
- * < at >return bool
+/*
+ * protected function verifyFile() inherited
  */
-protected function verifyFile() {
-return true;
-}
 
 /**
  * Stash the file.
diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php
index c7fd23a..53a9058 100644
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
< at >< at > -422,6 +422,7 < at >< at >
  * < at >return string
  */
 public static function getExtensionForPath( $path ) {
+global $wgFileBlacklist;
 // Does this have an extension?
 $n = strrpos( $path, '.' );
 $extension = null;
< at >< at > -441,7 +442,15 < at >< at >
 throw new UploadStashFileException( "extension is null" );
 }
 
-return File::normalizeExtension( $extension );
+$extension = File::normalizeExtension( $extension );
+if ( in_array( $extension, $wgFileBlacklist ) ) {
+// The file should already be checked for being evil.
+// However, if somehow we got here, we definitely
+// don't want to give it an extension of .php and
+// put it in a web accesible directory.
+return '';
+}
+return $extension;
 }
 
 /**

[MediaWiki-commits] [Gerrit] jquery.byteLimit: Improve unit tests -change (mediawiki/core)

Krinkle (Code Review — 2013-05-21T20:33:50

Krinkle has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64854


Change subject: jquery.byteLimit: Improve unit tests
......................................................................

jquery.byteLimit: Improve unit tests

* Update oudated documentation
* Remove hasLimit and limit properties of the test helper
  function. These just repeated data already provided
  (hasLimit if input != output, limit == expected.length)
* Add tests for a filter that increases the length
  (currently only a decreasing filter was tested).

Change-Id: Ib4c19f4d49ed9d19117bbbaccedb0fccaeb34719
---
M tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js
1 file changed, 43 insertions(+), 48 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/54/64854/1

diff --git a/tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js b/tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js
index c21844e..96186ff 100644
--- a/tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js
+++ b/tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js
< at >< at > -31,22 +31,22 < at >< at >
 /**
  * Test factory for $.fn.byteLimit
  *
- * < at >param $input {jQuery} jQuery object in an input element
- * < at >param hasLimit {Boolean} Wether a limit should apply at all
- * < at >param limit {Number} Limit (if used) otherwise undefined
- * The limit should be less than 20 (the sample data's length)
+ * < at >param {Object} options
+ * < at >param {string} options.description Test name
+ * < at >param {jQuery} options.$input jQuery object in an input element
+ * < at >param {string} options.sample Sequence of characters to simulate being
+ *  added one by one
+ * < at >param {string} options.expected Expected final value of `$input`
  */
 function byteLimitTest( options ) {
 var opt = $.extend( {
 description: '',
 $input: null,
 sample: '',
-hasLimit: false,
-expected: '',
-limit: null
+expected: ''
 }, options );
 
-QUnit.asyncTest( opt.description, opt.hasLimit ? 3 : 2, function ( assert ) {
+QUnit.asyncTest( opt.description, 1, function ( assert ) {
 setTimeout( function () {
 var rawVal, fn, effectiveVal;
 
< at >< at > -55,31 +55,12 < at >< at >
 // Simulate pressing keys for each of the sample characters
 addChars( opt.$input, opt.sample );
 
-rawVal = opt.$input.val();
-fn = opt.$input.data( 'byteLimit.callback' );
-effectiveVal = fn ? fn( rawVal ) : rawVal;
+assert.equal(
+opt.$input.val(),
+opt.expected,
+'New value matches the expected string'
+);
 
-if ( opt.hasLimit ) {
-assert.ltOrEq(
-$.byteLength( effectiveVal ),
-opt.limit,
-'Prevent keypresses after byteLimit was reached, length never exceeded the limit'
-);
-assert.equal(
-$.byteLength( rawVal ),
-$.byteLength( opt.expected ),
-'Not preventing keypresses too early, length has reached the expected length'
-);
-assert.equal( rawVal, opt.expected, 'New value matches the expected string' );
-
-} else {
-assert.equal(
-$.byteLength( effectiveVal ),
-$.byteLength( opt.expected ),
-'Unlimited scenarios are not affected, expected length reached'
-);
-assert.equal( rawVal, opt.expected, 'New value matches the expected string' );
-}
 QUnit.start();
 }, 10 );
 } );
< at >< at > -89,7 +70,6 < at >< at >
 description: 'Plain text input',
 $input: $( '<input type="text"/>' ),
 sample: simpleSample,
-hasLimit: false,
 expected: simpleSample
 } );
 
< at >< at > -98,7 +78,6 < at >< at >
 $input: $( '<input type="text"/>' )
 .byteLimit(),
 sample: simpleSample,
-hasLimit: false,
 expected: simpleSample
 } );
 
< at >< at > -108,8 +87,6 < at >< at >
 .attr( 'maxlength', '10' )
 .byteLimit(),
 sample: simpleSample,
-hasLimit: true,
-limit: 10,
 expected: '1234567890'
 } );
 
< at >< at > -118,8 +95,6 < at >< at >
 $input: $( '<input type="text"/>' )
 .byteLimit( 10 ),
 sample: simpleSample,
-hasLimit: true,
-limit: 10,
 expected: '1234567890'
 } );
 
< at >< at > -129,8 +104,6 < at >< at >
 .attr( 'maxlength', '10' )
 .byteLimit( 15 ),
 sample: simpleSample,
-hasLimit: true,
-limit: 15,
 expected: '123456789012345'
 } );
 
< at >< at > -139,8 +112,6 < at >< at >
 $input: $( '<input type="text"/>' )
 .byteLimit( 14 ),
 sample: mbSample,
-hasLimit: true,
-limit: 14,
 expected: '1234567890' + U_20AC + '1'
 } );
 
< at >< at > -149,8 +120,6 < at >< at >
 $input: $( '<input type="text"/>' )
 .byteLimit( 12 ),
 sample: mbSample,
-hasLimit: true,
-limit: 12,
 expected: '1234567890' + '12'
 } );
 
< at >< at > -167,8 +136,6 < at >< at >
 return new mw.Title( String( val ) ).getMain();
 } ),
 sample: 'User:Sample',
-hasLimit: true,
-limit: 6, // 'Sample' length
 expected: 'User:Sample'
 } );
 
< at >< at > -186,11 +153,39 < at >< at >
 return new mw.Title( String( val ) ).getMain();
 } ),
 sample: 'User:Sample',
-hasLimit: true,
-limit: 6, // 'Sample' length
 expected: 'User:Sample'
 } );
 
+byteLimitTest( {
+description: 'Pass the limit and a callback as input filter',
+$input: $( '<input type="text"/>' )
+.byteLimit( 6, function ( val ) {
+// Invalid title
+if ( val === '' ) {
+return '';
+}
+
+// Return without namespace prefix
+return new mw.Title( String( val ) ).getMain();
+} ),
+sample: 'User:Example',
+// The callback alters the value to be used to calculeate
+// the length. The altered value is "Exampl" which has
+// a length of 6, the "e" would exceed the limit.
+expected: 'User:Exampl'
+} );
+
+byteLimitTest( {
+description: 'Input filter that increases the length',
+$input: $( '<input type="text"/>' )
+.byteLimit( 10, function ( text ) {
+return 'prefix' + text;
+} ),
+sample: simpleSample,
+// Prefix adds 6 characters, limit is reached after 4
+expected: '1234'
+} );
+
 QUnit.test( 'Confirm properties and attributes set', 4, function ( assert ) {
 var $el, $elA, $elB;

[MediaWiki-commits] [Gerrit] SECURITY: Do checks on all uploadtypes - change (mediawiki/core)

jenkins-bot (Code Review — 2013-05-21T20:33:38

jenkins-bot has submitted this change and it was merged.

Change subject: SECURITY: Do checks on all upload types
......................................................................


SECURITY: Do checks on all upload types

Also, verify file before stashing it

Change-Id: Ib2474cb778d53959a4f479e53d0392f916b18d83
---
M includes/AutoLoader.php
M includes/api/ApiUpload.php
M includes/upload/UploadBase.php
M includes/upload/UploadFromChunks.php
M includes/upload/UploadFromStash.php
M includes/upload/UploadStash.php
6 files changed, 117 insertions(+), 38 deletions(-)

Approvals:
  CSteipp: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 326fb5c..3e08e74 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
< at >< at > -996,6 +996,7 < at >< at >
 'UnwatchedpagesPage' => 'includes/specials/SpecialUnwatchedpages.php',
 'UploadChunkFileException' => 'includes/upload/UploadFromChunks.php',
 'UploadChunkZeroLengthFileException' => 'includes/upload/UploadFromChunks.php',
+'UploadChunkVerificationException' => 'includes/upload/UploadFromChunks.php',
 'UploadForm' => 'includes/specials/SpecialUpload.php',
 'UploadSourceField' => 'includes/specials/SpecialUpload.php',
 'UserrightsPage' => 'includes/specials/SpecialUserrights.php',
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 5563087..e04c762 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
< at >< at > -88,9 +88,10 < at >< at >
 if ( !$this->mUpload->getTitle() ) {
 $this->dieUsage( 'Invalid file title supplied', 'internal-error' );
 }
-} elseif ( $this->mParams['async'] ) {
+} elseif ( $this->mParams['async'] && $this->mParams['filekey'] ) {
 // defer verification to background process
 } else {
+wfDebug( __METHOD__ . 'about to verify' );
 $this->verifyUpload();
 }
 
< at >< at > -195,7 +196,12 < at >< at >
 $chunkPath = $request->getFileTempname( 'chunk' );
 $chunkSize = $request->getUpload( 'chunk' )->getSize();
 if ( $this->mParams['offset'] == 0 ) {
-$filekey = $this->performStash();
+try {
+$filekey = $this->performStash();
+} catch ( MWException $e ) {
+// FIXME: Error handling here is wrong/different from rest of this
+$this->dieUsage( $e->getMessage(), 'stashfailed' );
+}
 } else {
 $filekey = $this->mParams['filekey'];
 /** < at >var $status Status */
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 17da80e..2ed20c5 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
< at >< at > -356,8 +356,10 < at >< at >
 }
 
 /**
- * Verify the mime type
+ * Verify the mime type.
  *
+ * < at >note Only checks that it is not an evil mime. The does it have
+ *  correct extension given its mime type check is in verifyFile.
  * < at >param string $mime representing the mime
  * < at >return mixed true if the file is verified, an array otherwise
  */
< at >< at > -370,12 +372,6 < at >< at >
 if ( $this->checkFileExtension( $mime, $wgMimeTypeBlacklist ) ) {
 wfProfileOut( __METHOD__ );
 return array( 'filetype-badmime', $mime );
-}
-
-# XXX: Missing extension will be caught by validateName() via getTitle()
-if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
-wfProfileOut( __METHOD__ );
-return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
 }
 
 # Check IE type
< at >< at > -398,17 +394,68 < at >< at >
 return true;
 }
 
+
 /**
  * Verifies that it's ok to include the uploaded file
  *
  * < at >return mixed true of the file is verified, array otherwise.
  */
 protected function verifyFile() {
+global $wgVerifyMimeType;
+wfProfileIn( __METHOD__ );
+
+$status = $this->verifyPartialFile();
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+if ( $wgVerifyMimeType ) {
+$this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
+$mime = $this->mFileProps['file-mime'];
+
+# XXX: Missing extension will be caught by validateName() via getTitle()
+if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
+wfProfileOut( __METHOD__ );
+return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
+}
+}
+
+
+$handler = MediaHandler::getHandler( $mime );
+if ( $handler ) {
+$handlerStatus = $handler->verifyUpload( $this->mTempPath );
+if ( !$handlerStatus->isOK() ) {
+$errors = $handlerStatus->getErrorsArray();
+wfProfileOut( __METHOD__ );
+return reset( $errors );
+}
+}
+
+wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+wfDebug( __METHOD__ . ": all clear; passing.\n" );
+wfProfileOut( __METHOD__ );
+return true;
+}
+
+/**
+ * A verification routine suitable for partial files
+ *
+ * Runs the blacklist checks, but not any checks that may
+ * assume the entire file is present.
+ *
+ * < at >return Mixed true for valid or array with error message key.
+ */
+protected function verifyPartialFile() {
 global $wgAllowJavaUploads, $wgDisableUploadScriptChecks;
 wfProfileIn( __METHOD__ );
 
-# get the title, even though we are doing nothing with it, because
-# we need to populate mFinalExtension
+# getTitle() sets some internal parameters like $this->mFinalExtension
 $this->getTitle();
 
 $this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
< at >< at > -462,23 +509,6 < at >< at >
 return array( 'uploadvirus', $virus );
 }
 
-$handler = MediaHandler::getHandler( $mime );
-if ( $handler ) {
-$handlerStatus = $handler->verifyUpload( $this->mTempPath );
-if ( !$handlerStatus->isOK() ) {
-$errors = $handlerStatus->getErrorsArray();
-wfProfileOut( __METHOD__ );
-return reset( $errors );
-}
-}
-
-wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
-if ( $status !== true ) {
-wfProfileOut( __METHOD__ );
-return $status;
-}
-
-wfDebug( __METHOD__ . ": all clear; passing.\n" );
 wfProfileOut( __METHOD__ );
 return true;
 }
< at >< at > -677,7 +707,6 < at >< at >
 if ( $this->mTitle !== false ) {
 return $this->mTitle;
 }
-
 /* Assume that if a user specified File:Something.jpg, this is an error
  * and that the namespace prefix needs to be stripped of.
  */
diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php
index 37db688..1746206 100644
--- a/includes/upload/UploadFromChunks.php
+++ b/includes/upload/UploadFromChunks.php
< at >< at > -70,6 +70,8 < at >< at >
 // Stash file is the called on creating a new chunk session:
 $this->mChunkIndex = 0;
 $this->mOffset = 0;
+
+$this->verifyChunk();
 // Create a local stash target
 $this->mLocalFile = parent::stashFile();
 // Update the initial file offset (based on file size)
< at >< at > -131,9 +133,18 < at >< at >
 return $status;
 }
 wfDebugLog( 'fileconcatenate', "Combined $i chunks in $tAmount seconds.\n" );
+
+$this->mTempPath = $tmpPath; // file system path
+$this->mFileSize = filesize( $this->mTempPath ); //Since this was set for the last chunk previously
+$ret = $this->verifyUpload();
+if ( $ret['status'] !== UploadBase::OK ) {
+wfDebugLog( 'fileconcatenate', "Verification failed for chunked upload" );
+$status->fatal( $this->getVerificationErrorCode( $ret['status'] ) );
+return $status;
+}
+
 // Update the mTempPath and mLocalFile
 // (for FileUpload or normal Stash to take over)
-$this->mTempPath = $tmpPath; // file system path
 $tStart = microtime( true );
 $this->mLocalFile = parent::stashFile( $this->user );
 $tAmount = microtime( true ) - $tStart;
< at >< at > -189,6 +200,15 < at >< at >
 if ( $preAppendOffset == $offset ) {
 // Update local chunk index for the current chunk
 $this->mChunkIndex++;
+try {
+# For some reason mTempPath is set to first part
+$oldTemp = $this->mTempPath;
+$this->mTempPath = $chunkPath;
+$this->verifyChunk();
+$this->mTempPath = $oldTemp;
+} catch ( UploadChunkVerificationException $e ) {
+return Status::newFatal( $e->getMessage() );
+}
 $status = $this->outputChunk( $chunkPath );
 if ( $status->isGood() ) {
 // Update local offset:
< at >< at > -312,7 +332,26 < at >< at >
 }
 return $this->mFileKey . '.' . $index;
 }
+
+/**
+ * Verify that the chunk isn't really an evil html file
+ *
+ * < at >throws UploadChunkVerificationException
+ */
+private function verifyChunk() {
+// Rest mDesiredDestName here so we verify the name as if it were mFileKey
+$oldDesiredDestName = $this->mDesiredDestName;
+$this->mDesiredDestName = $this->mFileKey;
+$this->mTitle = false;
+$res = $this->verifyPartialFile();
+$this->mDesiredDestName = $oldDesiredDestName;
+$this->mTitle = false;
+if( is_array( $res ) ) {
+throw new UploadChunkVerificationException( $res[0] );
+}
+}
 }
 
 class UploadChunkZeroLengthFileException extends MWException {};
 class UploadChunkFileException extends MWException {};
+class UploadChunkVerificationException extends MWException {};
diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php
index 9276b53..cb85fc6 100644
--- a/includes/upload/UploadFromStash.php
+++ b/includes/upload/UploadFromStash.php
< at >< at > -137,14 +137,9 < at >< at >
 return $this->mFileProps['sha1'];
 }
 
-/**
- * File has been previously verified so no need to do so again.
- *
- * < at >return bool
+/*
+ * protected function verifyFile() inherited
  */
-protected function verifyFile() {
-return true;
-}
 
 /**
  * Stash the file.
diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php
index 1ee4627..8a6d766 100644
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
< at >< at > -422,6 +422,7 < at >< at >
  * < at >return string
  */
 public static function getExtensionForPath( $path ) {
+global $wgFileBlacklist;
 // Does this have an extension?
 $n = strrpos( $path, '.' );
 $extension = null;
< at >< at > -441,7 +442,15 < at >< at >
 throw new UploadStashFileException( "extension is null" );
 }
 
-return File::normalizeExtension( $extension );
+$extension = File::normalizeExtension( $extension );
+if ( in_array( $extension, $wgFileBlacklist ) ) {
+// The file should already be checked for being evil.
+// However, if somehow we got here, we definitely
+// don't want to give it an extension of .php and
+// put it in a web accesible directory.
+return '';
+}
+return $extension;
 }
 
 /**

[MediaWiki-commits] [Gerrit] Updated release notes and versionnumber - change (mediawiki/core)

CSteipp (Code Review — 2013-05-21T20:25:43

CSteipp has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64853


Change subject: Updated release notes and version number
......................................................................

Updated release notes and version number

Change-Id: I1fb122b2e946df330578474ace5c1da7e163f5b1
---
M RELEASE-NOTES-1.19
M includes/DefaultSettings.php
2 files changed, 9 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/53/64853/1

diff --git a/RELEASE-NOTES-1.19 b/RELEASE-NOTES-1.19
index a61c237..560ee3b 100644
--- a/RELEASE-NOTES-1.19
+++ b/RELEASE-NOTES-1.19
< at >< at > -3,6 +3,14 < at >< at >
 Security reminder: MediaWiki does not require PHP's register_globals
 setting since version 1.2.0. If you have it on, turn it '''off''' if you can.
 
+== MediaWiki 1.19.7 ==
+
+This is a security and maintenance release of the MediaWiki 1.19 branch
+
+=== Changes since 1.19.6 ===
+* (bug 48306) SECURITY: Run file validation checks on  chunked uploads, and chunks
+  of upload, during the upload process.
+
 == MediaWiki 1.19.6 ==
 
 This is a security and maintenance release of the MediaWiki 1.19 branch
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 6aa5c9e..84cc9c2 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
< at >< at > -33,7 +33,7 < at >< at >
 /** < at >endcond */
 
 /** MediaWiki version number */
-$wgVersion = '1.19.6';
+$wgVersion = '1.19.7';
 
 /** Name of the site. It must be changed in LocalSettings.php */
 $wgSitename = 'MediaWiki';

[MediaWiki-commits] [Gerrit] SECURITY: Do checks on all uploadtypes - change (mediawiki/core)

CSteipp (Code Review — 2013-05-21T20:25:42

CSteipp has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64852


Change subject: SECURITY: Do checks on all upload types
......................................................................

SECURITY: Do checks on all upload types

Also, verify file before stashing it

Change-Id: I2b230af2f23c4303acdb695468608df19a88d16d
---
M includes/AutoLoader.php
M includes/api/ApiUpload.php
M includes/upload/UploadBase.php
M includes/upload/UploadFromChunks.php
M includes/upload/UploadFromStash.php
M includes/upload/UploadStash.php
6 files changed, 111 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/52/64852/1

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 7611ed9..2bf134b 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
< at >< at > -883,6 +883,7 < at >< at >
 
 # includes/upload
 'UploadBase' => 'includes/upload/UploadBase.php',
+'UploadChunkVerificationException' => 'includes/upload/UploadFromChunks.php',
 'UploadFromFile' => 'includes/upload/UploadFromFile.php',
 'UploadFromChunks' => 'includes/upload/UploadFromChunks.php',
 'UploadFromStash' => 'includes/upload/UploadFromStash.php',
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index fdc1eff..922b89d 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
< at >< at > -175,7 +175,12 < at >< at >
 $chunkPath = $request->getFileTempname( 'chunk' );
 $chunkSize = $request->getUpload( 'chunk' )->getSize();
 if ($this->mParams['offset'] == 0) {
-$result['filekey'] = $this->performStash();
+try {
+$result['filekey'] = $this->performStash();
+} catch ( MWException $e ) {
+// FIXME: Error handling here is wrong/different from rest of this
+$this->dieUsage( $e->getMessage(), 'stashfailed' );
+}
 } else {
 $status = $this->mUpload->addChunk($chunkPath, $chunkSize,
 $this->mParams['offset']);
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 3995519..5f09c04 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
< at >< at > -301,6 +301,8 < at >< at >
 /**
  * Verify the mime type
  *
+ * < at >note Only checks that it is not an evil mime. The does it have
+ *  correct extension given its mime type check is in verifyFile.
  * < at >param $mime string representing the mime
  * < at >return mixed true if the file is verified, an array otherwise
  */
< at >< at > -311,11 +313,6 < at >< at >
 global $wgMimeTypeBlacklist;
 if ( $this->checkFileExtension( $mime, $wgMimeTypeBlacklist ) ) {
 return array( 'filetype-badmime', $mime );
-}
-
-# XXX: Missing extension will be caught by validateName() via getTitle()
-if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
-return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
 }
 
 # Check IE type
< at >< at > -342,6 +339,56 < at >< at >
  * < at >return mixed true of the file is verified, array otherwise.
  */
 protected function verifyFile() {
+global $wgVerifyMimeType;
+wfProfileIn( __METHOD__ );
+
+$status = $this->verifyPartialFile();
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+if ( $wgVerifyMimeType ) {
+$this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
+$mime = $this->mFileProps['file-mime'];
+
+# XXX: Missing extension will be caught by validateName() via getTitle()
+if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
+wfProfileOut( __METHOD__ );
+return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
+}
+}
+
+$handler = MediaHandler::getHandler( $mime );
+if ( $handler ) {
+$handlerStatus = $handler->verifyUpload( $this->mTempPath );
+if ( !$handlerStatus->isOK() ) {
+$errors = $handlerStatus->getErrorsArray();
+wfProfileOut( __METHOD__ );
+return reset( $errors );
+}
+}
+
+wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+wfDebug( __METHOD__ . ": all clear; passing.\n" );
+wfProfileOut( __METHOD__ );
+return true;
+}
+
+/**
+ * A verification routine suitable for partial files
+ *
+ * Runs the blacklist checks, but not any checks that may
+ * assume the entire file is present.
+ *
+ * < at >return Mixed true for valid or array with error message key.
+ */
+protected function verifyPartialFile() {
 global $wgAllowJavaUploads, $wgDisableUploadScriptChecks;
 # get the title, even though we are doing nothing with it, because
 # we need to populate mFinalExtension
< at >< at > -392,21 +439,6 < at >< at >
 return array( 'uploadvirus', $virus );
 }
 
-$handler = MediaHandler::getHandler( $mime );
-if ( $handler ) {
-$handlerStatus = $handler->verifyUpload( $this->mTempPath );
-if ( !$handlerStatus->isOK() ) {
-$errors = $handlerStatus->getErrorsArray();
-return reset( $errors );
-}
-}
-
-wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
-if ( $status !== true ) {
-return $status;
-}
-
-wfDebug( __METHOD__ . ": all clear; passing.\n" );
 return true;
 }
 
diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php
index ec83f7d..a76f03d 100644
--- a/includes/upload/UploadFromChunks.php
+++ b/includes/upload/UploadFromChunks.php
< at >< at > -47,6 +47,8 < at >< at >
 // Stash file is the called on creating a new chunk session: 
 $this->mChunkIndex = 0;
 $this->mOffset = 0;
+
+$this->verifyChunk();
 // Create a local stash target
 $this->mLocalFile = parent::stashFile();
 // Update the initial file offset ( based on file size ) 
< at >< at > -105,9 +107,18 < at >< at >
 if( !$status->isOk() ){
 return $status; 
 }
+
+$this->mTempPath = $tmpPath; // file system path
+$this->mFileSize = filesize( $this->mTempPath ); //Since this was set for the last chunk previously
+$ret = $this->verifyUpload();
+if ( $ret['status'] !== UploadBase::OK ) {
+wfDebugLog( 'fileconcatenate', "Verification failed for chunked upload" );
+$status->fatal( $this->getVerificationErrorCode( $ret['status'] ) );
+return $status;
+}
+
 // Update the mTempPath and mLocalFile
 // ( for FileUpload or normal Stash to take over )  
-$this->mTempPath = $tmpPath; // file system path
 $this->mLocalFile = parent::stashFile();
 
 return $status;
< at >< at > -157,6 +168,15 < at >< at >
 if ( $preAppendOffset == $offset ) {
 // Update local chunk index for the current chunk   
 $this->mChunkIndex++;
+try {
+# For some reason mTempPath is set to first part
+$oldTemp = $this->mTempPath;
+$this->mTempPath = $chunkPath;
+$this->verifyChunk();
+$this->mTempPath = $oldTemp;
+} catch ( UploadChunkVerificationException $e ) {
+return Status::newFatal( $e->getMessage() );
+}
 $status = $this->outputChunk( $chunkPath );
 if( $status->isGood() ){
 // Update local offset: 
< at >< at > -270,7 +290,26 < at >< at >
 }
 return $this->mFileKey . '.' . $index ;
 }
+
+/**
+ * Verify that the chunk isn't really an evil html file
+ *
+ * < at >throws UploadChunkVerificationException
+ */
+private function verifyChunk() {
+// Rest mDesiredDestName here so we verify the name as if it were mFileKey
+$oldDesiredDestName = $this->mDesiredDestName;
+$this->mDesiredDestName = $this->mFileKey;
+$this->mTitle = false;
+$res = $this->verifyPartialFile();
+$this->mDesiredDestName = $oldDesiredDestName;
+$this->mTitle = false;
+if( is_array( $res ) ) {
+throw new UploadChunkVerificationException( $res[0] );
+}
+}
 }
 
 class UploadChunkZeroLengthFileException extends MWException {};
 class UploadChunkFileException extends MWException {};
+class UploadChunkVerificationException extends MWException {};
diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php
index f7589bd..014aa53 100644
--- a/includes/upload/UploadFromStash.php
+++ b/includes/upload/UploadFromStash.php
< at >< at > -109,14 +109,9 < at >< at >
 return $this->mSourceType;
 }
 
-/**
- * File has been previously verified so no need to do so again.
- *
- * < at >return bool
+/*
+ * protected function verifyFile() inherited
  */
-protected function verifyFile() {
-return true;
-}
 
 /**
  * Stash the file.
diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php
index ad153d2..73e3811 100644
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
< at >< at > -393,6 +393,7 < at >< at >
  * uploads versus the desired filename. Maybe we can get that passed to us...
  */
 public static function getExtensionForPath( $path ) {
+global $wgFileBlacklist;
 // Does this have an extension?
 $n = strrpos( $path, '.' );
 $extension = null;
< at >< at > -412,7 +413,15 < at >< at >
 throw new UploadStashFileException( "extension is null" );
 }
 
-return File::normalizeExtension( $extension );
+$extension = File::normalizeExtension( $extension );
+if ( in_array( $extension, $wgFileBlacklist ) ) {
+// The file should already be checked for being evil.
+// However, if somehow we got here, we definitely
+// don't want to give it an extension of .php and
+// put it in a web accesible directory.
+return '';
+}
+return $extension;
 }
 
 /**

[MediaWiki-commits] [Gerrit] disable multiple file upload on Safari- change (mediawiki...UploadWizard)

Nischayn22 (Code Review — 2013-05-21T20:25:03

Nischayn22 has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64851


Change subject: disable multiple file upload on Safari
......................................................................

disable multiple file upload on Safari

Bug: 44772
Change-Id: Ib60dd151965310d0a567c7d888b4638cc48a696d
---
M UploadWizardPage.js
1 file changed, 5 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/UploadWizard refs/changes/51/64851/1

diff --git a/UploadWizardPage.js b/UploadWizardPage.js
index 85d51a1..4d1792b 100644
--- a/UploadWizardPage.js
+++ b/UploadWizardPage.js
< at >< at > -16,6 +16,11 < at >< at >
 mw.log.level = mw.log.NONE;
 }
 
+// hack for Safari bug, see https://bugzilla.wikimedia.org/show_bug.cgi?id=44772
+if ( navigator.userAgent.indexOf( "Safari" ) > -1 ) {
+config.enableMultipleFiles = false;
+}
+
 var uploadWizard = new mw.UploadWizard( config );
 uploadWizard.createInterface( '#upload-wizard' );

[MediaWiki-commits] [Gerrit] Updated release notes and versionnumber - change (mediawiki/core)

CSteipp (Code Review — 2013-05-21T20:24:41

CSteipp has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64850


Change subject: Updated release notes and version number
......................................................................

Updated release notes and version number

Change-Id: I630010e6371ac4ba9ea24e5f2f48e891d3e41c2f
---
M RELEASE-NOTES-1.20
M includes/DefaultSettings.php
2 files changed, 6 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/50/64850/1

diff --git a/RELEASE-NOTES-1.20 b/RELEASE-NOTES-1.20
index 2da9e35..d4f399a 100644
--- a/RELEASE-NOTES-1.20
+++ b/RELEASE-NOTES-1.20
< at >< at > -6,11 +6,14 < at >< at >
 
 == MediaWiki 1.20.6 ==
 
-This is a maintenance release of the MediaWiki 1.20 branch.
+This is a security and maintenance release of the MediaWiki 1.20 branch.
 
 === Changes since 1.20.5 ===
+* (bug 48306) SECURITY: Run file validation checks on chunked uploads, and chunks
+  of upload, during the upload process.
+* (bug 44327) mediawiki.user: Use session ID instead of 1-year cross-session cookies
 * (bug 47202) wikibits: FF2Fixes.css should not be loaded in Firefox 20.
-
+* (bug 31044) Make ResourceLoader behave in read-only mode
 
 == MediaWiki 1.20.5 ==
 
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 6abc2b1..710605a 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
< at >< at > -59,7 +59,7 < at >< at >
 $wgConf = new SiteConfiguration;
 
 /** MediaWiki version number */
-$wgVersion = '1.20.5';
+$wgVersion = '1.20.6';
 
 /** Name of the site. It must be changed in LocalSettings.php */
 $wgSitename = 'MediaWiki';

[MediaWiki-commits] [Gerrit] SECURITY: Do checks on all uploadtypes - change (mediawiki/core)

CSteipp (Code Review — 2013-05-21T20:24:40

CSteipp has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64849


Change subject: SECURITY: Do checks on all upload types
......................................................................

SECURITY: Do checks on all upload types

Also, verify file before stashing it

Change-Id: I5dd027acc994cf7308c9e4d85a61237d802ccee8
---
M includes/AutoLoader.php
M includes/api/ApiUpload.php
M includes/upload/UploadBase.php
M includes/upload/UploadFromChunks.php
M includes/upload/UploadFromStash.php
M includes/upload/UploadStash.php
6 files changed, 111 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/49/64849/1

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 9bdfcd2..a8b2202 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
< at >< at > -965,6 +965,7 < at >< at >
 'UnwatchedpagesPage' => 'includes/specials/SpecialUnwatchedpages.php',
 'UploadChunkFileException' => 'includes/upload/UploadFromChunks.php',
 'UploadChunkZeroLengthFileException' => 'includes/upload/UploadFromChunks.php',
+'UploadChunkVerificationException' => 'includes/upload/UploadFromChunks.php',
 'UploadForm' => 'includes/specials/SpecialUpload.php',
 'UploadSourceField' => 'includes/specials/SpecialUpload.php',
 'UserrightsPage' => 'includes/specials/SpecialUserrights.php',
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 3a9b5c5..e7a7849 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
< at >< at > -187,7 +187,12 < at >< at >
 $chunkPath = $request->getFileTempname( 'chunk' );
 $chunkSize = $request->getUpload( 'chunk' )->getSize();
 if ($this->mParams['offset'] == 0) {
-$result['filekey'] = $this->performStash();
+try {
+$result['filekey'] = $this->performStash();
+} catch ( MWException $e ) {
+// FIXME: Error handling here is wrong/different from rest of this
+$this->dieUsage( $e->getMessage(), 'stashfailed' );
+}
 } else {
 $status = $this->mUpload->addChunk($chunkPath, $chunkSize,
 $this->mParams['offset']);
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 3a5733c..0848780 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
< at >< at > -345,6 +345,8 < at >< at >
 /**
  * Verify the mime type
  *
+ * < at >note Only checks that it is not an evil mime. The does it have
+ *  correct extension given its mime type check is in verifyFile.
  * < at >param $mime string representing the mime
  * < at >return mixed true if the file is verified, an array otherwise
  */
< at >< at > -357,12 +359,6 < at >< at >
 if ( $this->checkFileExtension( $mime, $wgMimeTypeBlacklist ) ) {
 wfProfileOut( __METHOD__ );
 return array( 'filetype-badmime', $mime );
-}
-
-# XXX: Missing extension will be caught by validateName() via getTitle()
-if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
-wfProfileOut( __METHOD__ );
-return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
 }
 
 # Check IE type
< at >< at > -391,6 +387,56 < at >< at >
  * < at >return mixed true of the file is verified, array otherwise.
  */
 protected function verifyFile() {
+global $wgVerifyMimeType;
+wfProfileIn( __METHOD__ );
+
+$status = $this->verifyPartialFile();
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+if ( $wgVerifyMimeType ) {
+$this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
+$mime = $this->mFileProps['file-mime'];
+
+# XXX: Missing extension will be caught by validateName() via getTitle()
+if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
+wfProfileOut( __METHOD__ );
+return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
+}
+}
+
+$handler = MediaHandler::getHandler( $mime );
+if ( $handler ) {
+$handlerStatus = $handler->verifyUpload( $this->mTempPath );
+if ( !$handlerStatus->isOK() ) {
+$errors = $handlerStatus->getErrorsArray();
+wfProfileOut( __METHOD__ );
+return reset( $errors );
+}
+}
+
+wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+wfDebug( __METHOD__ . ": all clear; passing.\n" );
+wfProfileOut( __METHOD__ );
+return true;
+}
+
+/**
+ * A verification routine suitable for partial files
+ *
+ * Runs the blacklist checks, but not any checks that may
+ * assume the entire file is present.
+ *
+ * < at >return Mixed true for valid or array with error message key.
+ */
+protected function verifyPartialFile() {
 global $wgAllowJavaUploads, $wgDisableUploadScriptChecks;
 wfProfileIn( __METHOD__ );
 
< at >< at > -449,23 +495,6 < at >< at >
 return array( 'uploadvirus', $virus );
 }
 
-$handler = MediaHandler::getHandler( $mime );
-if ( $handler ) {
-$handlerStatus = $handler->verifyUpload( $this->mTempPath );
-if ( !$handlerStatus->isOK() ) {
-$errors = $handlerStatus->getErrorsArray();
-wfProfileOut( __METHOD__ );
-return reset( $errors );
-}
-}
-
-wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
-if ( $status !== true ) {
-wfProfileOut( __METHOD__ );
-return $status;
-}
-
-wfDebug( __METHOD__ . ": all clear; passing.\n" );
 wfProfileOut( __METHOD__ );
 return true;
 }
diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php
index 0542bba..531f7be 100644
--- a/includes/upload/UploadFromChunks.php
+++ b/includes/upload/UploadFromChunks.php
< at >< at > -69,6 +69,8 < at >< at >
 // Stash file is the called on creating a new chunk session:
 $this->mChunkIndex = 0;
 $this->mOffset = 0;
+
+$this->verifyChunk();
 // Create a local stash target
 $this->mLocalFile = parent::stashFile();
 // Update the initial file offset ( based on file size )
< at >< at > -127,9 +129,18 < at >< at >
 if( !$status->isOk() ){
 return $status;
 }
+
+$this->mTempPath = $tmpPath; // file system path
+$this->mFileSize = filesize( $this->mTempPath ); //Since this was set for the last chunk previously
+$ret = $this->verifyUpload();
+if ( $ret['status'] !== UploadBase::OK ) {
+wfDebugLog( 'fileconcatenate', "Verification failed for chunked upload" );
+$status->fatal( $this->getVerificationErrorCode( $ret['status'] ) );
+return $status;
+}
+
 // Update the mTempPath and mLocalFile
 // ( for FileUpload or normal Stash to take over )
-$this->mTempPath = $tmpPath; // file system path
 $this->mLocalFile = parent::stashFile();
 
 return $status;
< at >< at > -181,6 +192,15 < at >< at >
 if ( $preAppendOffset == $offset ) {
 // Update local chunk index for the current chunk
 $this->mChunkIndex++;
+try {
+# For some reason mTempPath is set to first part
+$oldTemp = $this->mTempPath;
+$this->mTempPath = $chunkPath;
+$this->verifyChunk();
+$this->mTempPath = $oldTemp;
+} catch ( UploadChunkVerificationException $e ) {
+return Status::newFatal( $e->getMessage() );
+}
 $status = $this->outputChunk( $chunkPath );
 if( $status->isGood() ){
 // Update local offset:
< at >< at > -300,7 +320,26 < at >< at >
 }
 return $this->mFileKey . '.' . $index ;
 }
+
+/**
+ * Verify that the chunk isn't really an evil html file
+ *
+ * < at >throws UploadChunkVerificationException
+ */
+private function verifyChunk() {
+// Rest mDesiredDestName here so we verify the name as if it were mFileKey
+$oldDesiredDestName = $this->mDesiredDestName;
+$this->mDesiredDestName = $this->mFileKey;
+$this->mTitle = false;
+$res = $this->verifyPartialFile();
+$this->mDesiredDestName = $oldDesiredDestName;
+$this->mTitle = false;
+if( is_array( $res ) ) {
+throw new UploadChunkVerificationException( $res[0] );
+}
+}
 }
 
 class UploadChunkZeroLengthFileException extends MWException {};
 class UploadChunkFileException extends MWException {};
+class UploadChunkVerificationException extends MWException {};
diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php
index 607965f..d79641c 100644
--- a/includes/upload/UploadFromStash.php
+++ b/includes/upload/UploadFromStash.php
< at >< at > -129,14 +129,9 < at >< at >
 return $this->mSourceType;
 }
 
-/**
- * File has been previously verified so no need to do so again.
- *
- * < at >return bool
+/*
+ * protected function verifyFile() inherited
  */
-protected function verifyFile() {
-return true;
-}
 
 /**
  * Stash the file.
diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php
index c7fd23a..53a9058 100644
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
< at >< at > -422,6 +422,7 < at >< at >
  * < at >return string
  */
 public static function getExtensionForPath( $path ) {
+global $wgFileBlacklist;
 // Does this have an extension?
 $n = strrpos( $path, '.' );
 $extension = null;
< at >< at > -441,7 +442,15 < at >< at >
 throw new UploadStashFileException( "extension is null" );
 }
 
-return File::normalizeExtension( $extension );
+$extension = File::normalizeExtension( $extension );
+if ( in_array( $extension, $wgFileBlacklist ) ) {
+// The file should already be checked for being evil.
+// However, if somehow we got here, we definitely
+// don't want to give it an extension of .php and
+// put it in a web accesible directory.
+return '';
+}
+return $extension;
 }
 
 /**

[MediaWiki-commits] [Gerrit] SECURITY: Do checks on all uploadtypes - change (mediawiki/core)

CSteipp (Code Review — 2013-05-21T20:20:50

CSteipp has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64848


Change subject: SECURITY: Do checks on all upload types
......................................................................

SECURITY: Do checks on all upload types

Also, verify file before stashing it

Change-Id: Ib2474cb778d53959a4f479e53d0392f916b18d83
---
M includes/AutoLoader.php
M includes/api/ApiUpload.php
M includes/upload/UploadBase.php
M includes/upload/UploadFromChunks.php
M includes/upload/UploadFromStash.php
M includes/upload/UploadStash.php
6 files changed, 117 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/48/64848/1

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 326fb5c..3e08e74 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
< at >< at > -996,6 +996,7 < at >< at >
 'UnwatchedpagesPage' => 'includes/specials/SpecialUnwatchedpages.php',
 'UploadChunkFileException' => 'includes/upload/UploadFromChunks.php',
 'UploadChunkZeroLengthFileException' => 'includes/upload/UploadFromChunks.php',
+'UploadChunkVerificationException' => 'includes/upload/UploadFromChunks.php',
 'UploadForm' => 'includes/specials/SpecialUpload.php',
 'UploadSourceField' => 'includes/specials/SpecialUpload.php',
 'UserrightsPage' => 'includes/specials/SpecialUserrights.php',
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 5563087..e04c762 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
< at >< at > -88,9 +88,10 < at >< at >
 if ( !$this->mUpload->getTitle() ) {
 $this->dieUsage( 'Invalid file title supplied', 'internal-error' );
 }
-} elseif ( $this->mParams['async'] ) {
+} elseif ( $this->mParams['async'] && $this->mParams['filekey'] ) {
 // defer verification to background process
 } else {
+wfDebug( __METHOD__ . 'about to verify' );
 $this->verifyUpload();
 }
 
< at >< at > -195,7 +196,12 < at >< at >
 $chunkPath = $request->getFileTempname( 'chunk' );
 $chunkSize = $request->getUpload( 'chunk' )->getSize();
 if ( $this->mParams['offset'] == 0 ) {
-$filekey = $this->performStash();
+try {
+$filekey = $this->performStash();
+} catch ( MWException $e ) {
+// FIXME: Error handling here is wrong/different from rest of this
+$this->dieUsage( $e->getMessage(), 'stashfailed' );
+}
 } else {
 $filekey = $this->mParams['filekey'];
 /** < at >var $status Status */
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 17da80e..2ed20c5 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
< at >< at > -356,8 +356,10 < at >< at >
 }
 
 /**
- * Verify the mime type
+ * Verify the mime type.
  *
+ * < at >note Only checks that it is not an evil mime. The does it have
+ *  correct extension given its mime type check is in verifyFile.
  * < at >param string $mime representing the mime
  * < at >return mixed true if the file is verified, an array otherwise
  */
< at >< at > -370,12 +372,6 < at >< at >
 if ( $this->checkFileExtension( $mime, $wgMimeTypeBlacklist ) ) {
 wfProfileOut( __METHOD__ );
 return array( 'filetype-badmime', $mime );
-}
-
-# XXX: Missing extension will be caught by validateName() via getTitle()
-if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
-wfProfileOut( __METHOD__ );
-return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
 }
 
 # Check IE type
< at >< at > -398,17 +394,68 < at >< at >
 return true;
 }
 
+
 /**
  * Verifies that it's ok to include the uploaded file
  *
  * < at >return mixed true of the file is verified, array otherwise.
  */
 protected function verifyFile() {
+global $wgVerifyMimeType;
+wfProfileIn( __METHOD__ );
+
+$status = $this->verifyPartialFile();
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+if ( $wgVerifyMimeType ) {
+$this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
+$mime = $this->mFileProps['file-mime'];
+
+# XXX: Missing extension will be caught by validateName() via getTitle()
+if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) {
+wfProfileOut( __METHOD__ );
+return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime );
+}
+}
+
+
+$handler = MediaHandler::getHandler( $mime );
+if ( $handler ) {
+$handlerStatus = $handler->verifyUpload( $this->mTempPath );
+if ( !$handlerStatus->isOK() ) {
+$errors = $handlerStatus->getErrorsArray();
+wfProfileOut( __METHOD__ );
+return reset( $errors );
+}
+}
+
+wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
+if ( $status !== true ) {
+wfProfileOut( __METHOD__ );
+return $status;
+}
+
+wfDebug( __METHOD__ . ": all clear; passing.\n" );
+wfProfileOut( __METHOD__ );
+return true;
+}
+
+/**
+ * A verification routine suitable for partial files
+ *
+ * Runs the blacklist checks, but not any checks that may
+ * assume the entire file is present.
+ *
+ * < at >return Mixed true for valid or array with error message key.
+ */
+protected function verifyPartialFile() {
 global $wgAllowJavaUploads, $wgDisableUploadScriptChecks;
 wfProfileIn( __METHOD__ );
 
-# get the title, even though we are doing nothing with it, because
-# we need to populate mFinalExtension
+# getTitle() sets some internal parameters like $this->mFinalExtension
 $this->getTitle();
 
 $this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
< at >< at > -462,23 +509,6 < at >< at >
 return array( 'uploadvirus', $virus );
 }
 
-$handler = MediaHandler::getHandler( $mime );
-if ( $handler ) {
-$handlerStatus = $handler->verifyUpload( $this->mTempPath );
-if ( !$handlerStatus->isOK() ) {
-$errors = $handlerStatus->getErrorsArray();
-wfProfileOut( __METHOD__ );
-return reset( $errors );
-}
-}
-
-wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) );
-if ( $status !== true ) {
-wfProfileOut( __METHOD__ );
-return $status;
-}
-
-wfDebug( __METHOD__ . ": all clear; passing.\n" );
 wfProfileOut( __METHOD__ );
 return true;
 }
< at >< at > -677,7 +707,6 < at >< at >
 if ( $this->mTitle !== false ) {
 return $this->mTitle;
 }
-
 /* Assume that if a user specified File:Something.jpg, this is an error
  * and that the namespace prefix needs to be stripped of.
  */
diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php
index 37db688..1746206 100644
--- a/includes/upload/UploadFromChunks.php
+++ b/includes/upload/UploadFromChunks.php
< at >< at > -70,6 +70,8 < at >< at >
 // Stash file is the called on creating a new chunk session:
 $this->mChunkIndex = 0;
 $this->mOffset = 0;
+
+$this->verifyChunk();
 // Create a local stash target
 $this->mLocalFile = parent::stashFile();
 // Update the initial file offset (based on file size)
< at >< at > -131,9 +133,18 < at >< at >
 return $status;
 }
 wfDebugLog( 'fileconcatenate', "Combined $i chunks in $tAmount seconds.\n" );
+
+$this->mTempPath = $tmpPath; // file system path
+$this->mFileSize = filesize( $this->mTempPath ); //Since this was set for the last chunk previously
+$ret = $this->verifyUpload();
+if ( $ret['status'] !== UploadBase::OK ) {
+wfDebugLog( 'fileconcatenate', "Verification failed for chunked upload" );
+$status->fatal( $this->getVerificationErrorCode( $ret['status'] ) );
+return $status;
+}
+
 // Update the mTempPath and mLocalFile
 // (for FileUpload or normal Stash to take over)
-$this->mTempPath = $tmpPath; // file system path
 $tStart = microtime( true );
 $this->mLocalFile = parent::stashFile( $this->user );
 $tAmount = microtime( true ) - $tStart;
< at >< at > -189,6 +200,15 < at >< at >
 if ( $preAppendOffset == $offset ) {
 // Update local chunk index for the current chunk
 $this->mChunkIndex++;
+try {
+# For some reason mTempPath is set to first part
+$oldTemp = $this->mTempPath;
+$this->mTempPath = $chunkPath;
+$this->verifyChunk();
+$this->mTempPath = $oldTemp;
+} catch ( UploadChunkVerificationException $e ) {
+return Status::newFatal( $e->getMessage() );
+}
 $status = $this->outputChunk( $chunkPath );
 if ( $status->isGood() ) {
 // Update local offset:
< at >< at > -312,7 +332,26 < at >< at >
 }
 return $this->mFileKey . '.' . $index;
 }
+
+/**
+ * Verify that the chunk isn't really an evil html file
+ *
+ * < at >throws UploadChunkVerificationException
+ */
+private function verifyChunk() {
+// Rest mDesiredDestName here so we verify the name as if it were mFileKey
+$oldDesiredDestName = $this->mDesiredDestName;
+$this->mDesiredDestName = $this->mFileKey;
+$this->mTitle = false;
+$res = $this->verifyPartialFile();
+$this->mDesiredDestName = $oldDesiredDestName;
+$this->mTitle = false;
+if( is_array( $res ) ) {
+throw new UploadChunkVerificationException( $res[0] );
+}
+}
 }
 
 class UploadChunkZeroLengthFileException extends MWException {};
 class UploadChunkFileException extends MWException {};
+class UploadChunkVerificationException extends MWException {};
diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php
index 9276b53..cb85fc6 100644
--- a/includes/upload/UploadFromStash.php
+++ b/includes/upload/UploadFromStash.php
< at >< at > -137,14 +137,9 < at >< at >
 return $this->mFileProps['sha1'];
 }
 
-/**
- * File has been previously verified so no need to do so again.
- *
- * < at >return bool
+/*
+ * protected function verifyFile() inherited
  */
-protected function verifyFile() {
-return true;
-}
 
 /**
  * Stash the file.
diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php
index 1ee4627..8a6d766 100644
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
< at >< at > -422,6 +422,7 < at >< at >
  * < at >return string
  */
 public static function getExtensionForPath( $path ) {
+global $wgFileBlacklist;
 // Does this have an extension?
 $n = strrpos( $path, '.' );
 $extension = null;
< at >< at > -441,7 +442,15 < at >< at >
 throw new UploadStashFileException( "extension is null" );
 }
 
-return File::normalizeExtension( $extension );
+$extension = File::normalizeExtension( $extension );
+if ( in_array( $extension, $wgFileBlacklist ) ) {
+// The file should already be checked for being evil.
+// However, if somehow we got here, we definitely
+// don't want to give it an extension of .php and
+// put it in a web accesible directory.
+return '';
+}
+return $extension;
 }
 
 /**

[MediaWiki-commits] [Gerrit] Add script sql to Tools. - change(operations/puppet)

Tim Landscheidt (Code Review — 2013-05-21T20:10:29

Tim Landscheidt has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64847


Change subject: Add script sql to Tools.
......................................................................

Add script sql to Tools.

The script allows users to just name the database they want to connect
to without having to look up the server it is stored on.

Bug: 48627
Change-Id: Ie111a5adad4c6378ed71825394548f47d4991eaf
---
A modules/toollabs/files/sql
M modules/toollabs/manifests/exec_environ.pp
2 files changed, 46 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/operations/puppet refs/changes/47/64847/1

diff --git a/modules/toollabs/files/sql b/modules/toollabs/files/sql
new file mode 100755
index 0000000..6dde329
--- /dev/null
+++ b/modules/toollabs/files/sql
< at >< at > -0,0 +1,40 < at >< at >
+#!/bin/bash
+
+function check_host () {
+    for HOST_I in enwiki bgwiki bgwiktionary cswiki enwikiquote \
+                  enwiktionary eowiki fiwiki idwiki itwiki nlwiki \
+                  nowiki plwiki ptwiki svwiki thwiki trwiki zhwiki \
+                  commonswiki dewiki wikidatawiki; do
+        if [ "$1" = "$HOST_I" ]; then
+            return 0
+        fi
+    done
+
+    return 1
+}
+
+# No database name or extra parameters given?
+if [ $# -eq 0 ] || [ $# -gt 2 ]; then
+    echo "Usage: $0 DATABASE [COMMAND]" >&2
+    exit 1
+fi
+
+# Database name doesn't end up with "_p"?
+HOST="${1%_p}"
+if [ "$1" = "$HOST" ]; then
+    echo "$0: Database name '$1' doesn't end with '_p'." >&2
+    exit 1
+fi
+
+# Host in list of replicated databases?
+if ! check_host "$HOST"; then
+    echo "$0: Database '$1' hasn't been replicated to Tools yet." >&2
+    exit 1
+fi
+
+# Call mysql.
+if [ $# -eq 1 ]; then
+    exec mysql -ch "$HOST.labsdb" "$1"
+else
+    exec mysql -ch "$HOST.labsdb" "$1" -e "$2"
+fi
diff --git a/modules/toollabs/manifests/exec_environ.pp b/modules/toollabs/manifests/exec_environ.pp
index 75461dd..f2732cf 100644
--- a/modules/toollabs/manifests/exec_environ.pp
+++ b/modules/toollabs/manifests/exec_environ.pp
< at >< at > -94,6 +94,12 < at >< at >
   sysctl { "vm.overcommit_memory": value => 2 }
   sysctl { "vm.overcommit_ratio": value => 95 }
 
+  file { "/usr/local/bin/sql":
+    ensure => file,
+    mode => "0555",
+    source => "puppet:///files/toollabs/sql";
+  }
+
   # TODO: quotas
 }

[MediaWiki-commits] [Gerrit] Update MobileFrontend to productiontip - change (mediawiki/core)

jenkins-bot (Code Review — 2013-05-21T20:10:25

jenkins-bot has submitted this change and it was merged.

Change subject: Update MobileFrontend to production tip
......................................................................


Update MobileFrontend to production tip

Change-Id: I07182a924c62fc9141a5c946b8552806ca00d0c4
---
M extensions/MobileFrontend
1 file changed, 0 insertions(+), 0 deletions(-)

Approvals:
  awjrichards: Verified; Looks good to me, approved
  jenkins-bot: Verified



diff --git a/extensions/MobileFrontend b/extensions/MobileFrontend
index 05d3a18..8ee5fdf 160000
--- a/extensions/MobileFrontend
+++ b/extensions/MobileFrontend
-Subproject commit 05d3a1821cf77bda90f7bbab9eb6be7c5cdd9b22
+Subproject commit 8ee5fdf6954e2feaad42b35012e5230e92ad9bae

[MediaWiki-commits] [Gerrit] TitleSquidURLs hook for changing theURLs to purge - change (mediawiki/core)

jenkins-bot (Code Review — 2013-05-21T20:06:12

jenkins-bot has submitted this change and it was merged.

Change subject: TitleSquidURLs hook for changing the URLs to purge
......................................................................


TitleSquidURLs hook for changing the URLs to purge

This allows extensions to purge derivative resources that need
updating when a wiki page is changed.

Change-Id: Ic28ce7f57f29376b041627288979981fcb218a44
---
M RELEASE-NOTES-1.22
M docs/hooks.txt
M includes/Title.php
3 files changed, 7 insertions(+), 0 deletions(-)

Approvals:
  Aaron Schulz: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 168f2f7..6b13e01 100644
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
< at >< at > -79,6 +79,8 < at >< at >
   which can be cascading (previously 'sysop' was hard-coded as the only one).
 * XHTML5 support has been improved. If you set $wgMimeType = 'application/xhtml+xml'
   MediaWiki will try outputting markup acording to XHTML5 rules.
+* New hook 'TitleSquidURLs' for manipulating the list of URLs to be purged from
+  HTTP caches when a page is changed.
 
 === Bug fixes in 1.22 ===
 * Disable Special:PasswordReset when $wgEnableEmail is false. Previously one
diff --git a/docs/hooks.txt b/docs/hooks.txt
index a292997..f94a1b0 100644
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
< at >< at > -2379,6 +2379,10 < at >< at >
 $user: Current user object
 &$whitelisted: Boolean value of whether this title is whitelisted
 
+'TitleSquidURLs': Called to determine which URLs to purge from HTTP caches.
+$this: Title object to purge
+&$urls: An array of URLs to purge from the caches, to be manipulated.
+
 'UndeleteForm::showHistory': Called in UndeleteForm::showHistory, after a
 PageArchive object has been created but before any further processing is done.
 &$archive: PageArchive object
diff --git a/includes/Title.php b/includes/Title.php
index 14915e5..8a37f68 100644
--- a/includes/Title.php
+++ b/includes/Title.php
< at >< at > -3448,6 +3448,7 < at >< at >
 }
 }
 
+wfRunHooks( 'TitleSquidURLs', array( $this, &$urls ) );
 return $urls;
 }

[MediaWiki-commits] [Gerrit] Update MobileFrontend to productiontip - change (mediawiki/core)

awjrichards (Code Review — 2013-05-21T20:05:48

awjrichards has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/64846


Change subject: Update MobileFrontend to production tip
......................................................................

Update MobileFrontend to production tip

Change-Id: I07182a924c62fc9141a5c946b8552806ca00d0c4
---
M extensions/MobileFrontend
1 file changed, 0 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/46/64846/1

diff --git a/extensions/MobileFrontend b/extensions/MobileFrontend
index 05d3a18..8ee5fdf 160000
--- a/extensions/MobileFrontend
+++ b/extensions/MobileFrontend
-Subproject commit 05d3a1821cf77bda90f7bbab9eb6be7c5cdd9b22
+Subproject commit 8ee5fdf6954e2feaad42b35012e5230e92ad9bae

[MediaWiki-commits] [Gerrit] Restrict Verified/Submit to JenkinsBotand l10n-bot - change (mediawiki...TranslationNotifications)

Krinkle (Code Review — 2013-05-21T20:00:36

Krinkle has submitted this change and it was merged.

Change subject: Restrict Verified/Submit to JenkinsBot and l10n-bot
......................................................................


Restrict Verified/Submit to JenkinsBot and l10n-bot

Change-Id: Ifa6c9509e6506a34da346046a5dd6005a01315ea
---
M groups
M project.config
2 files changed, 10 insertions(+), 0 deletions(-)

Approvals:
  Krinkle: Verified; Looks good to me, approved



diff --git a/groups b/groups
index c69ed7f..d5839f1 100644
--- a/groups
+++ b/groups
< at >< at > -1,3 +1,6 < at >< at >
 # UUID                                  Group Name
 #
+2bc47fcadf4e44ec9a1a73bcfa06232554f47ce2JenkinsBot
+cc37d98e3a4301744a0c0a9249173ae170696072l10n-bot
+global:Registered-Users                 Registered Users
 c626276deb0c8cac1c73820eb6e88184b55e4a9dl10n-team
diff --git a/project.config b/project.config
index 262af0b..442e947 100644
--- a/project.config
+++ b/project.config
< at >< at > -3,6 +3,13 < at >< at >
 description = MediaWiki extension TranslationNotifications.
 [access "refs/*"]
 owner = group l10n-team
+[access "refs/heads/master"]
+label-Verified = -1..+2 group JenkinsBot
+label-Verified = -1..+2 group l10n-bot
+label-Verified = -1..+0 group Registered Users
+submit = deny group Registered Users
+submit = group JenkinsBot
+submit = group l10n-bot
 [receive]
 requireChangeId = true
 [submit]

[MediaWiki-commits] [Gerrit] Restrict Verified/Submit to JenkinsBotand l10n-bot - change (mediawiki...Translate)

Krinkle (Code Review — 2013-05-21T20:00:20

Krinkle has submitted this change and it was merged.

Change subject: Restrict Verified/Submit to JenkinsBot and l10n-bot
......................................................................


Restrict Verified/Submit to JenkinsBot and l10n-bot

Change-Id: I49af1c9526655097229ed0cb154f0df1450fa8a8
---
M groups
M project.config
2 files changed, 10 insertions(+), 0 deletions(-)

Approvals:
  Krinkle: Verified; Looks good to me, approved



diff --git a/groups b/groups
index fa2a5b2..48a0455 100644
--- a/groups
+++ b/groups
< at >< at > -1,4 +1,7 < at >< at >
 # UUID                                  Group Name
 #
+2bc47fcadf4e44ec9a1a73bcfa06232554f47ce2JenkinsBot
+cc37d98e3a4301744a0c0a9249173ae170696072l10n-bot
+global:Registered-Users                 Registered Users
 39bc276be2b012b959b981bd8609aafdf450ec84extension-Translate
 c626276deb0c8cac1c73820eb6e88184b55e4a9dl10n-team
diff --git a/project.config b/project.config
index 0d92c1a..7d822f2 100644
--- a/project.config
+++ b/project.config
< at >< at > -6,6 +6,13 < at >< at >
 [access "refs/*"]
 owner = group extension-Translate
 owner = group l10n-team
+[access "refs/heads/master"]
+label-Verified = -1..+2 group JenkinsBot
+label-Verified = -1..+2 group l10n-bot
+label-Verified = -1..+0 group Registered Users
+submit = deny group Registered Users
+submit = group JenkinsBot
+submit = group l10n-bot
 [receive]
 requireChangeId = true
 [submit]

[MediaWiki-commits] [Gerrit] Restrict Verified/Submit to JenkinsBotand l10n-bot - change (mediawiki...TwnMainPage)

Krinkle (Code Review — 2013-05-21T19:59:53

Krinkle has submitted this change and it was merged.

Change subject: Restrict Verified/Submit to JenkinsBot and l10n-bot
......................................................................


Restrict Verified/Submit to JenkinsBot and l10n-bot

Change-Id: I3edbcee0852be98c041d15a25f1e657b6f352e49
---
M groups
M project.config
2 files changed, 10 insertions(+), 0 deletions(-)

Approvals:
  Krinkle: Verified; Looks good to me, approved



diff --git a/groups b/groups
index c69ed7f..d5839f1 100644
--- a/groups
+++ b/groups
< at >< at > -1,3 +1,6 < at >< at >
 # UUID                                  Group Name
 #
+2bc47fcadf4e44ec9a1a73bcfa06232554f47ce2JenkinsBot
+cc37d98e3a4301744a0c0a9249173ae170696072l10n-bot
+global:Registered-Users                 Registered Users
 c626276deb0c8cac1c73820eb6e88184b55e4a9dl10n-team
diff --git a/project.config b/project.config
index f608ee3..adb1dba 100644
--- a/project.config
+++ b/project.config
< at >< at > -1,5 +1,12 < at >< at >
 [access]
 inheritFrom = mediawiki/extensions
+[access "refs/heads/master"]
+label-Verified = -1..+2 group JenkinsBot
+label-Verified = -1..+2 group l10n-bot
+label-Verified = -1..+0 group Registered Users
+submit = deny group Registered Users
+submit = group JenkinsBot
+submit = group l10n-bot
 [project]
 state = active
 description = MediaWiki extension TwnMainPage