4 files changed, 104 insertions, 32 deletions

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 3a5733ca..0848780f 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -345,6 +345,8 @@ abstract class UploadBase {
 	/**
 	 * Verify the mime type
 	 *
+	 * @note Only checks that it is not an evil mime. The does it have
+	 *  correct extension given its mime type check is in verifyFile.
 	 * @param $mime string representing the mime
 	 * @return mixed true if the file is verified, an array otherwise
 	 */
@@ -359,12 +361,6 @@ abstract class UploadBase {
 				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
 			$fp = fopen( $this->mTempPath, 'rb' );
 			$chunk = fread( $fp, 256 );
@@ -391,6 +387,56 @@ abstract class UploadBase {
 	 * @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.
+	 *
+	 * @return Mixed true for valid or array with error message key.
+	 */
+	protected function verifyPartialFile() {
 		global $wgAllowJavaUploads, $wgDisableUploadScriptChecks;
 		wfProfileIn( __METHOD__ );
 
@@ -449,23 +495,6 @@ abstract class UploadBase {
 			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 0542bba5..531f7be4 100644
--- a/includes/upload/UploadFromChunks.php
+++ b/includes/upload/UploadFromChunks.php
@@ -69,6 +69,8 @@ class UploadFromChunks extends UploadFromFile {
 		// 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 )
@@ -127,9 +129,18 @@ class UploadFromChunks extends UploadFromFile {
 		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;
@@ -181,6 +192,15 @@ class UploadFromChunks extends UploadFromFile {
 			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:
@@ -300,7 +320,26 @@ class UploadFromChunks extends UploadFromFile {
 		}
 		return $this->mFileKey . '.' . $index ;
 	}
+
+	/**
+	 * Verify that the chunk isn't really an evil html file
+	 *
+	 * @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 607965f3..d79641ce 100644
--- a/includes/upload/UploadFromStash.php
+++ b/includes/upload/UploadFromStash.php
@@ -129,14 +129,9 @@ class UploadFromStash extends UploadBase {
 		return $this->mSourceType;
 	}
 
-	/**
-	 * File has been previously verified so no need to do so again.
-	 *
-	 * @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 c7fd23a9..53a90582 100644
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
@@ -422,6 +422,7 @@ class UploadStash {
 	 * @return string
 	 */
 	public static function getExtensionForPath( $path ) {
+		global $wgFileBlacklist;
 		// Does this have an extension?
 		$n = strrpos( $path, '.' );
 		$extension = null;
@@ -441,7 +442,15 @@ class UploadStash {
 			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;
 	}
 
 	/**