* (bug 34212) ApiBlock/ApiUnblock allow action to take place without a token parameter present1.18.2

* (bug 35317) CSRF in Special:Upload
Revert r56793, which removed the CSRF check for Special:Upload for normal file
uploads. Cross-site posting of file uploads without user interaction has been
possible since at least as early as Chrome 8 (late 2010) and Firefox 6 (mid
2011).

Commonist has used api.php since version 0.4.0 (April 2010), and the API
already requires an edit token, so Commonist 0.4.0+ is not affected by this
change.

* (bug 34907) Fix for CSRF vulnerability due to mw.user.tokens. Patch by Roan
Kattouw and Tim Starling.
* Filter out private modules early in ResourceLoader::makeResponse() and just
pretend they weren't specified. This means these modules cannot be loaded
through load.php . This filtering must not happen in makeModuleResponse(),
because that would break inlining.
* Force inlining of private modules in OutputPage::makeResourceLoaderLink(),
disregarding $wgResourceLoaderInlinePrivateModules
* Remove $wgResourceLoaderInlinePrivateModules
* Remove special treatment of private modules ($private) in
ResourceLoader::makeResponse() and sendResponseHeaders(), because we're not
allowing private modules to be loaded through here any more
* Remove identity checks in ResourceLoaderUserOptionsModule and
ResourceLoaderUserCSSPrefsModule, they didn't make a lot of sense before but
they're certainly useless now.
* Factored out error comment construction in ResourceLoader.php and stripped
comment terminations from exception messages. I didn't find an XSS
vulnerability but it looked scary.

Change-Id: I0e632722a0d9f4fc6d6bb355a986f498017c815d

7 files changed, 43 insertions, 69 deletions

diff --git a/RELEASE-NOTES-1.18 b/RELEASE-NOTES-1.18
index 5b9133a564e5..9af5a1a5c0ad 100644
--- a/RELEASE-NOTES-1.18
+++ b/RELEASE-NOTES-1.18
@@ -4,9 +4,9 @@ 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.18.2 ==
-XXXX-XX-XX
+2012-03-21
 
-This is a maintenance release of the MediaWiki 1.18 branch.
+This is a maintenance and security release of the MediaWiki 1.18 branch.
 
 == Changes since 1.18.1 ==
 
@@ -22,6 +22,11 @@ This is a maintenance release of the MediaWiki 1.18 branch.
   Special:Whatlinkshere
 * (bug 22555) Remove or skip strip markers from tag hooks like <nowiki> in
   core parser functions which operate on strings, such as formatnum.
+* (bug 34212) ApiBlock/ApiUnblock allow action to take place without a token
+  parameter present.
+* (bug 34907) Fixed exposure of tokens through load.php that could have facilitated
+  CSRF attacks.
+* (bug 35317) CSRF in Special:Upload.
 
 === Changes since 1.18.0 ===
 
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 9d387fb5c971..7cea30f692b2 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -33,7 +33,7 @@ $wgConf = new SiteConfiguration;
 /** @endcond */
 
 /** MediaWiki version number */
-$wgVersion = '1.18.1';
+$wgVersion = '1.18.2';
 
 /** Name of the site. It must be changed in LocalSettings.php */
 $wgSitename         = 'MediaWiki';
@@ -2494,13 +2494,6 @@ $wgResourceLoaderMaxage = array(
 );
 
 /**
- * Whether to embed private modules inline with HTML output or to bypass
- * caching and check the user parameter against $wgUser to prevent
- * unauthorized access to private modules.
- */
-$wgResourceLoaderInlinePrivateModules = true;
-
-/**
  * The default debug mode (on/off) for of ResourceLoader requests. This will still
  * be overridden when the debug URL parameter is used.
  */
diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index a3a9592dea93..68c771bce832 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2359,8 +2359,7 @@ $templates
 	 * @return string html <script> and <style> tags
 	 */
 	protected function makeResourceLoaderLink( Skin $skin, $modules, $only, $useESI = false ) {
-		global $wgLoadScript, $wgResourceLoaderUseESI,
-			$wgResourceLoaderInlinePrivateModules;
+		global $wgLoadScript, $wgResourceLoaderUseESI;
 		// Lazy-load ResourceLoader
 		// TODO: Should this be a static function of ResourceLoader instead?
 		$baseQuery = array(
@@ -2444,8 +2443,11 @@ $templates
 
 			$query['modules'] = ResourceLoader::makePackedModulesString( array_keys( $modules ) );
 
-			// Support inlining of private modules if configured as such
-			if ( $group === 'private' && $wgResourceLoaderInlinePrivateModules ) {
+			// Inline private modules. These can't be loaded through load.php for security
+			// reasons, see bug 34907. Note that these modules should be loaded from
+			// getHeadScripts() before the first loader call. Otherwise other modules can't
+			// properly use them as dependencies (bug 30914)
+			if ( $group === 'private' ) {
 				if ( $only == ResourceLoaderModule::TYPE_STYLES ) {
 					$links .= Html::inlineStyle(
 						$resourceLoader->makeModuleResponse( $context, $modules )
diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 9a3c82993c15..dd4e1ffb9c2d 100644
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -581,7 +581,7 @@ class ApiMain extends ApiBase {
 
 		// Die if token required, but not provided (unless there is a gettoken parameter)
 		$salt = $module->getTokenSalt();
-		if ( $salt !== false && !isset( $moduleParams['gettoken'] ) ) {
+		if ( $salt !== false && !$moduleParams['gettoken'] ) {
 			if ( !isset( $moduleParams['token'] ) ) {
 				$this->dieUsageMsg( array( 'missingparam', 'token' ) );
 			} else {
diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 2a2e298150f1..01b70e8eaaf9 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -164,7 +164,7 @@ class ResourceLoader {
 			$cache->set( $key, $result );
 		} catch ( Exception $exception ) {
 			// Return exception as a comment
-			$result = "/*\n{$exception->__toString()}\n*/\n";
+			$result = $this->makeComment( $exception->__toString() );
 		}
 
 		wfProfileOut( __METHOD__ );
@@ -309,13 +309,20 @@ class ResourceLoader {
 		ob_start();
 
 		wfProfileIn( __METHOD__ );
-		$exceptions = '';
+		$errors = '';
 
 		// Split requested modules into two groups, modules and missing
 		$modules = array();
 		$missing = array();
 		foreach ( $context->getModules() as $name ) {
 			if ( isset( $this->moduleInfos[$name] ) ) {
+				$module = $this->getModule( $name );
+				// Do not allow private modules to be loaded from the web.
+				// This is a security issue, see bug 34907.
+				if ( $module->getGroup() === 'private' ) {
+					$errors .= $this->makeComment( "Cannot show private module \"$name\"" );
+					continue;
+				}
 				$modules[$name] = $this->getModule( $name );
 			} else {
 				$missing[] = $name;
@@ -340,26 +347,21 @@ class ResourceLoader {
 			$this->preloadModuleInfo( array_keys( $modules ), $context );
 		} catch( Exception $e ) {
 			// Add exception to the output as a comment
-			$exceptions .= "/*\n{$e->__toString()}\n*/\n";
+			$errors .= $this->makeComment( $e->__toString() );
 		}
 
 		wfProfileIn( __METHOD__.'-getModifiedTime' );
 
-		$private = false;
 		// To send Last-Modified and support If-Modified-Since, we need to detect
 		// the last modified time
 		$mtime = wfTimestamp( TS_UNIX, $wgCacheEpoch );
 		foreach ( $modules as $module ) {
 			try {
-				// Bypass Squid and other shared caches if the request includes any private modules
-				if ( $module->getGroup() === 'private' ) {
-					$private = true;
-				}
 				// Calculate maximum modified time
 				$mtime = max( $mtime, $module->getModifiedTime( $context ) );
 			} catch ( Exception $e ) {
 				// Add exception to the output as a comment
-				$exceptions .= "/*\n{$e->__toString()}\n*/\n";
+				$errors .= $this->makeComment( $e->__toString() );
 			}
 		}
 
@@ -376,13 +378,8 @@ class ResourceLoader {
 			header( 'Cache-Control: private, no-cache, must-revalidate' );
 			header( 'Pragma: no-cache' );
 		} else {
-			if ( $private ) {
-				header( "Cache-Control: private, max-age=$maxage" );
-				$exp = $maxage;
-			} else {
-				header( "Cache-Control: public, max-age=$maxage, s-maxage=$smaxage" );
-				$exp = min( $maxage, $smaxage );
-			}
+			header( "Cache-Control: public, max-age=$maxage, s-maxage=$smaxage" );
+			$exp = min( $maxage, $smaxage );
 			header( 'Expires: ' . wfTimestamp( TS_RFC2822, $exp + time() ) );
 		}
 
@@ -422,12 +419,12 @@ class ResourceLoader {
 		$response = $this->makeModuleResponse( $context, $modules, $missing );
 
 		// Prepend comments indicating exceptions
-		$response = $exceptions . $response;
+		$response = $errors . $response;
 
 		// Capture any PHP warnings from the output buffer and append them to the
 		// response in a comment if we're in debug mode.
 		if ( $context->getDebug() && strlen( $warnings = ob_get_contents() ) ) {
-			$response = "/*\n$warnings\n*/\n" . $response;
+			$response = $this->makeComment( $warnings ) . $response;
 		}
 
 		// Remove the output buffer and output the response
@@ -437,6 +434,11 @@ class ResourceLoader {
 		wfProfileOut( __METHOD__ );
 	}
 
+	protected function makeComment( $text ) {
+		$encText = str_replace( '*/', '* /', $text );
+		return "/*\n$encText\n*/\n";
+	}
+
 	/**
 	 * Generates code for a response
 	 *
@@ -461,7 +463,7 @@ class ResourceLoader {
 				$blobs = MessageBlobStore::get( $this, $modules, $context->getLanguage() );
 			} catch ( Exception $e ) {
 				// Add exception to the output as a comment
-				$exceptions .= "/*\n{$e->__toString()}\n*/\n";
+				$exceptions .= $this->makeComment( $e->__toString() );
 			}
 		} else {
 			$blobs = array();
@@ -534,7 +536,7 @@ class ResourceLoader {
 				}
 			} catch ( Exception $e ) {
 				// Add exception to the output as a comment
-				$exceptions .= "/*\n{$e->__toString()}\n*/\n";
+				$exceptions .= $this->makeComment( $e->__toString() );
 
 				// Register module as missing
 				$missing[] = $name;
diff --git a/includes/resourceloader/ResourceLoaderUserOptionsModule.php b/includes/resourceloader/ResourceLoaderUserOptionsModule.php
index 8f28eb8d0d8a..78548416bfeb 100644
--- a/includes/resourceloader/ResourceLoaderUserOptionsModule.php
+++ b/includes/resourceloader/ResourceLoaderUserOptionsModule.php
@@ -45,29 +45,7 @@ class ResourceLoaderUserOptionsModule extends ResourceLoaderModule {
 
 		global $wgUser;
 
-		if ( $context->getUser() === $wgUser->getName() ) {
-			return $this->modifiedTime[$hash] = wfTimestamp( TS_UNIX, $wgUser->getTouched() );
-		} else {
-			return 1;
-		}
-	}
-
-	/**
-	 * Fetch the context's user options, or if it doesn't match current user,
-	 * the default options.
-	 * 
-	 * @param $context ResourceLoaderContext: Context object
-	 * @return Array: List of user options keyed by option name
-	 */
-	protected function contextUserOptions( ResourceLoaderContext $context ) {
-		global $wgUser;
-
-		// Verify identity -- this is a private module
-		if ( $context->getUser() === $wgUser->getName() ) {
-			return $wgUser->getOptions();
-		} else {
-			return User::getDefaultOptions();
-		}
+		return $this->modifiedTime[$hash] = wfTimestamp( TS_UNIX, $wgUser->getTouched() );
 	}
 
 	/**
@@ -75,8 +53,9 @@ class ResourceLoaderUserOptionsModule extends ResourceLoaderModule {
 	 * @return string
 	 */
 	public function getScript( ResourceLoaderContext $context ) {
+		global $wgUser;
 		return Xml::encodeJsCall( 'mw.user.options.set', 
-			array( $this->contextUserOptions( $context ) ) );
+			array( $wgUser->getOptions() ) );
 	}
 
 	/**
@@ -84,10 +63,10 @@ class ResourceLoaderUserOptionsModule extends ResourceLoaderModule {
 	 * @return array
 	 */
 	public function getStyles( ResourceLoaderContext $context ) {
-		global $wgAllowUserCssPrefs;
+		global $wgAllowUserCssPrefs, $wgUser;
 
 		if ( $wgAllowUserCssPrefs ) {
-			$options = $this->contextUserOptions( $context );
+			$options = $wgUser->getOptions();
 
 			// Build CSS rules
 			$rules = array();
diff --git a/includes/specials/SpecialUpload.php b/includes/specials/SpecialUpload.php
index 8eeca5d5a7f7..33013e089936 100644
--- a/includes/specials/SpecialUpload.php
+++ b/includes/specials/SpecialUpload.php
@@ -119,14 +119,7 @@ class SpecialUpload extends SpecialPage {
 
 		// If it was posted check for the token (no remote POST'ing with user credentials)
 		$token = $request->getVal( 'wpEditToken' );
-		if( $this->mSourceType == 'file' && $token == null ) {
-			// Skip token check for file uploads as that can't be faked via JS...
-			// Some client-side tools don't expect to need to send wpEditToken
-			// with their submissions, as that's new in 1.16.
-			$this->mTokenOk = true;
-		} else {
-			$this->mTokenOk = $wgUser->matchEditToken( $token );
-		}
+		$this->mTokenOk = $wgUser->matchEditToken( $token );
 
 		$this->uploadFormTextTop = '';
 		$this->uploadFormTextAfterSummary = '';