SECURITY: Do checks on all upload types

[lhc/web/wiklou.git] / includes / upload / UploadBase.php

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 17da80e..2ed20c5 100644 (file)
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -356,8 +356,10 @@ abstract class UploadBase {
        }
 
        /**
-        * Verify the mime type
+        * 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 string $mime representing the mime
         * @return mixed true if the file is verified, an array otherwise
         */
@@ -372,12 +374,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 );
@@ -398,17 +394,68 @@ abstract class UploadBase {
                return true;
        }
 
+
        /**
         * Verifies that it's ok to include the uploaded file
         *
         * @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__ );
 
-               # 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 );
@@ -462,23 +509,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;
        }
@@ -677,7 +707,6 @@ abstract class UploadBase {
                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.
                 */