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

* (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.

Patchset2:
 Removes whitespace error that prevented automatic merge by Gerrit:
 includes/resourceloader/ResourceLoaderUserOptionsModule.php

Change-Id: I2dec8b8caf9db3c64919763865cc10cccdd6a1a3
This commit is contained in:
Reedy 2012-03-22 19:52:37 +00:00 committed by Antoine Musso
parent b68c5a837f
commit 13169c88c1
7 changed files with 40 additions and 97 deletions

View file

@ -2581,13 +2581,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.

View file

@ -2505,7 +2505,7 @@ $templates
* @return string html <script> and <style> tags
*/
protected function makeResourceLoaderLink( $modules, $only, $useESI = false, array $extraQuery = array(), $loadCall = false ) {
global $wgResourceLoaderUseESI, $wgResourceLoaderInlinePrivateModules;
global $wgResourceLoaderUseESI;
if ( !count( $modules ) ) {
return '';
@ -2584,10 +2584,11 @@ $templates
continue;
}
// Support inlining of private modules if configured as such. 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' && $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 )

View file

@ -595,7 +595,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 {

View file

@ -173,7 +173,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__ );
@ -431,13 +431,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;
@ -449,12 +456,11 @@ 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 );
@ -463,22 +469,18 @@ class ResourceLoader {
* @var $module ResourceLoaderModule
*/
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() );
}
}
wfProfileOut( __METHOD__.'-getModifiedTime' );
// Send content type and cache related headers
$this->sendResponseHeaders( $context, $mtime, $private );
$this->sendResponseHeaders( $context, $mtime );
// If there's an If-Modified-Since header, respond with a 304 appropriately
if ( $this->tryRespondLastModified( $context, $mtime ) ) {
@ -490,20 +492,20 @@ 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
ob_end_clean();
echo $response;
// Save response to file cache unless there are private modules or errors
if ( isset( $fileCache ) && !$private && !$exceptions && !$missing ) {
// Save response to file cache unless there are errors
if ( isset( $fileCache ) && !$errors && !$missing ) {
// Cache single modules...and other requests if there are enough hits
if ( ResourceFileCache::useFileCache( $context ) ) {
if ( $fileCache->isCacheWorthy() ) {
@ -521,10 +523,9 @@ class ResourceLoader {
* Send content type and last modified headers to the client.
* @param $context ResourceLoaderContext
* @param $mtime string TS_MW timestamp to use for last-modified
* @param $private bool True iff response contains any private modules
* @return void
*/
protected function sendResponseHeaders( ResourceLoaderContext $context, $mtime, $private ) {
protected function sendResponseHeaders( ResourceLoaderContext $context, $mtime ) {
global $wgResourceLoaderMaxage;
// If a version wasn't specified we need a shorter expiry time for updates
// to propagate to clients quickly
@ -548,13 +549,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() ) );
}
}
@ -651,6 +647,11 @@ class ResourceLoader {
return false; // cache miss
}
protected function makeComment( $text ) {
$encText = str_replace( '*/', '* /', $text );
return "/*\n$encText\n*/\n";
}
/**
* Generates code for a response
*
@ -675,7 +676,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();
@ -754,7 +755,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;

View file

@ -44,30 +44,7 @@ class ResourceLoaderUserCSSPrefsModule 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,10 +52,10 @@ class ResourceLoaderUserCSSPrefsModule 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();

View file

@ -44,30 +44,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 +52,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() ) );
}
/**

View file

@ -111,14 +111,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 = $this->getUser()->matchEditToken( $token );
}
$this->mTokenOk = $this->getUser()->matchEditToken( $token );
$this->uploadFormTextTop = '';
$this->uploadFormTextAfterSummary = '';