* Enhance upload extension blacklist to protect against vulnerable Apache configurations1.3.9

2 files changed, 68 insertions, 13 deletions

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 09e07f999543..3477e666aca4 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -3,7 +3,23 @@
 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.
 
-== Version 1.3.9, ****-**-** ==
+== Version 1.3.9, 2004-12-12 ==
+
+MediaWiki 1.3.9 is a security and bug fix release.
+
+A flaw in upload handling has been found which may allow upload and 
+execution of arbitrary scripts with the permissions of the web server.
+Only wikis that have enabled uploads and have a vulnerable Apache 
+configuration will be affected, but to be safe all wikis should upgrade.
+
+Wikis with uploads available should either disable uploads or upgrade to
+1.3.9 immediately; if other files are customized and require merging 
+changes, includes/SpecialUpload.php may be replaced individually to add
+the fix.
+
+(It is also recommended to configure your web server to disable script
+execution in the 'images' subdirectory where uploads are placed, which
+prevents most attacks even if the wiki fails.)
 
 Changes from 1.3.8:
 * Backported "Templates used in this page"-feature of EditPage
@@ -12,6 +28,9 @@ Changes from 1.3.8:
 * (bug 1010) fix broken Commons image link on Classic & Cologne Blue
 * (bug 1004) Norsk language names for interwiki links changed,
   Nauruan language name changed
+* Enhance upload extension blacklist to protect against vulnerable
+  Apache configurations
+
 
 == Version 1.3.8, 2004-11-15 ==
 
diff --git a/includes/SpecialUpload.php b/includes/SpecialUpload.php
index 9b66d8016dc2..f0e9e16ab0e1 100644
--- a/includes/SpecialUpload.php
+++ b/includes/SpecialUpload.php
@@ -156,14 +156,18 @@ class UploadForm {
 		# Chop off any directories in the given filename
 		$basename = basename( $this->mOname );
 
-		if( preg_match( '/^(.*)\.([^.]*)$/', $basename, $matches ) ) {
-			$partname = $matches[1];
-			$ext      = $matches[2];
+		/**
+		 * We'll want to blacklist against *any* 'extension', and use
+		 * only the final one for the whitelist.
+		 */
+		list( $partname, $ext ) = $this->splitExtensions( $basename );
+		if( count( $ext ) ) {
+			$finalExt = $ext[count( $ext ) - 1];
 		} else {
-		    $partname = $basename;
-		    $ext = '';
+			$finalExt = '';
 		}
-
+		$fullExt = implode( '.', $ext );
+		
 		if ( strlen( $partname ) < 3 ) {
 			$this->mainUploadForm( wfMsg( 'minlength' ) );
 			return;
@@ -192,9 +196,10 @@ class UploadForm {
 		/* Don't allow users to override the blacklist */
 		global $wgStrictFileExtensions;
 		global $wgFileExtensions, $wgFileBlacklist;
-		if( $this->checkFileExtension( $ext, $wgFileBlacklist ) ||
-			($wgStrictFileExtensions && !$this->checkFileExtension( $ext, $wgFileExtensions ) ) ) {
-			return $this->uploadError( wfMsg( 'badfiletype', htmlspecialchars( $ext ) ) );
+		if( $this->checkFileExtensionList( $ext, $wgFileBlacklist ) ||
+			($wgStrictFileExtensions &&
+				!$this->checkFileExtension( $finalExt, $wgFileExtensions ) ) ) {
+			return $this->uploadError( wfMsg( 'badfiletype', htmlspecialchars( $fullExt ) ) );
 		}
 		
 		/**
@@ -202,7 +207,7 @@ class UploadForm {
 		 * type but it's corrupt or data of the wrong type, we should
 		 * probably not accept it.
 		 */
-		if( !$this->verify( $this->mUploadTempName, $ext ) ) {
+		if( !$this->verify( $this->mUploadTempName, $finalExt ) ) {
 			return $this->uploadError( wfMsg( 'uploadcorrupt' ) );
 		}
 		
@@ -217,8 +222,8 @@ class UploadForm {
 	
 			global $wgCheckFileExtensions;
 			if ( $wgCheckFileExtensions ) {
-				if ( ! $this->checkFileExtension( $ext, $wgFileExtensions ) ) {
-					$warning .= '<li>'.wfMsg( 'badfiletype', htmlspecialchars( $ext ) ).'</li>';
+				if ( ! $this->checkFileExtension( $finalExt, $wgFileExtensions ) ) {
+					$warning .= '<li>'.wfMsg( 'badfiletype', htmlspecialchars( $fullExt ) ).'</li>';
 				}
 			}
 	
@@ -535,6 +540,20 @@ class UploadForm {
 	/* -------------------------------------------------------------- */
 
 	/**
+	 * Split a file into a base name and all dot-delimited 'extensions'
+	 * on the end. Some web server configurations will fall back to
+	 * earlier pseudo-'extensions' to determine type and execute
+	 * scripts, so the blacklist needs to check them all.
+	 *
+	 * @return array
+	 */
+	function splitExtensions( $filename ) {
+		$bits = explode( '.', $filename );
+		$basename = array_shift( $bits );
+		return array( $basename, $bits );
+	}
+	
+	/**
 	 * Perform case-insensitive match against a list of file extensions.
 	 * Returns true if the extension is in the list.
 	 *
@@ -547,6 +566,23 @@ class UploadForm {
 	}
 
 	/**
+	 * Perform case-insensitive match against a list of file extensions.
+	 * Returns true if any of the extensions are in the list.
+	 *
+	 * @param array $ext
+	 * @param array $list
+	 * @return bool
+	 */
+	function checkFileExtensionList( $ext, $list ) {
+		foreach( $ext as $e ) {
+			if( in_array( strtolower( $e ), $list ) ) {
+				return true;
+			}
+		}
+		return false;
+	}
+	
+	/**
 	 * Returns false if the file is of a known type but can't be recognized,
 	 * indicating a corrupt file.
 	 * Returns true otherwise; unknown file types are not checked if given