From: Roan Kattouw Date: Wed, 14 Jul 2010 19:00:54 +0000 (+0000) Subject: API: Make output containing private or user-specific data uncacheable for logged... X-Git-Tag: 1.31.0-rc.0~36145 X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/categories/modifier.php?a=commitdiff_plain;h=70824ccccc29d78da6b5e8bda892d18d1ab347f0;p=lhc%2Fweb%2Fwiklou.git API: Make output containing private or user-specific data uncacheable for logged-in users by setting Vary: Cookie or Cache-Control: private, whichever is appropriate. Fixes instances in core and WMF-deployed extensions only. Without this change, the output of requests like ?action=query&list=recentchanges&rcprop=patrolled&smaxage=3600 would be cached in Squid and viewable for anyone using the same URL, even if they don't have patrol rights. Other, more serious exploits are also possible. Also avoid using $wgUser in one place, kill some unused global $wgUser; instances and tweak a comment. --- diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 0d3d2614f2..330e797ec9 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -1103,9 +1103,12 @@ abstract class ApiBase { if ( $token == '' || $token != $params['token'] ) { $this->dieUsage( 'Incorrect watchlist token provided -- please set a correct token in Special:Preferences', 'bad_wltoken' ); } - } elseif ( !$wgUser->isLoggedIn() ) { - $this->dieUsage( 'You must be logged-in to have a watchlist', 'notloggedin' ); } else { + // User not determined by URL, so don't cache + $this->getMain()->setVaryCookie(); + if ( !$wgUser->isLoggedIn() ) { + $this->dieUsage( 'You must be logged-in to have a watchlist', 'notloggedin' ); + } $user = $wgUser; } return $user; diff --git a/includes/api/ApiLogout.php b/includes/api/ApiLogout.php index 7307c0eb1b..55adc3dd65 100644 --- a/includes/api/ApiLogout.php +++ b/includes/api/ApiLogout.php @@ -42,6 +42,7 @@ class ApiLogout extends ApiBase { public function execute() { global $wgUser; + $this->getMain()->setCachePrivate(); $oldName = $wgUser->getName(); $wgUser->logout(); diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index 9f161e7c1b..4e8be4d44b 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -124,7 +124,7 @@ class ApiMain extends ApiBase { private $mPrinter, $mModules, $mModuleNames, $mFormats, $mFormatNames; private $mResult, $mAction, $mShowVersions, $mEnableWrite, $mRequest; - private $mInternalMode, $mSquidMaxage, $mModule; + private $mInternalMode, $mSquidMaxage, $mModule, $mVaryCookie; private $mCacheControl = array( 'must-revalidate' => true ); @@ -169,6 +169,7 @@ class ApiMain extends ApiBase { $this->mSquidMaxage = - 1; // flag for executeActionWithErrorHandling() $this->mCommit = false; + $this->mVaryCookie = false; } /** @@ -215,6 +216,14 @@ class ApiMain extends ApiBase { 's-maxage' => $maxage ) ); } + + /** + * Make sure Cache-Control: private is set. Use this when the output of a request + * is for the current recipient only and should not be cached in any shared cache. + */ + public function setCachePrivate() { + $this->setCacheControl( array( 'private' => true ) ); + } /** * Set directives (key/value pairs) for the Cache-Control header. @@ -224,6 +233,35 @@ class ApiMain extends ApiBase { public function setCacheControl( $directives ) { $this->mCacheControl = $directives + $this->mCacheControl; } + + /** + * Make sure Vary: Cookie and friends are set. Use this when the output of a request + * may be cached for anons but may not be cached for logged-in users. + * + * WARNING: This function must be called CONSISTENTLY for a given URL. This means that a + * given URL must either always or never call this function; if it sometimes does and + * sometimes doesn't, stuff will break. + */ + public function setVaryCookie() { + $this->mVaryCookie = true; + } + + /** + * Actually output the Vary: Cookie header and its friends, if flagged with setVaryCookie(). + * Outputs the appropriate X-Vary-Options header and Cache-Control: private if needed. + */ + private function outputVaryCookieHeader() { + global $wgUseXVO, $wgOut; + if ( $this->mVaryCookie ) { + header( 'Vary: Cookie' ); + if ( $wgUseXVO ) { + header( $wgOut->getXVO() ); + if ( $wgOut->hasCacheVaryCookies() ) { + $this->setCacheControl( array( 'private' => true ) ); + } + } + } + } /** * Create an instance of an output formatter by its name @@ -276,6 +314,7 @@ class ApiMain extends ApiBase { // Error results should not be cached $this->setCacheMaxAge( 0 ); + $this->setCachePrivate(); $headerStr = 'MediaWiki-API-Error: ' . $errCode; if ( $e->getCode() === 0 ) { @@ -291,6 +330,11 @@ class ApiMain extends ApiBase { $this->mPrinter->safeProfileOut(); $this->printResult( true ); } + + // If this wiki is private, don't cache anything ever + if ( in_array( 'read', User::getGroupPermissions( array( '*' ) ), true ) ) { + $this->setCachePrivate(); + } // If nobody called setCacheMaxAge(), use the (s)maxage parameters if ( !isset( $this->mCacheControl['s-maxage'] ) ) { @@ -322,6 +366,7 @@ class ApiMain extends ApiBase { } header( "Cache-Control: $ccHeader" ); + $this->outputVaryCookieHeader(); if ( $this->mPrinter->getIsHtml() && !$this->mPrinter->isDisabled() ) { echo wfReportTime(); @@ -477,7 +522,8 @@ class ApiMain extends ApiBase { */ protected function checkExecutePermissions( $module ) { global $wgUser, $wgGroupPermissions; - if ( $module->isReadMode() && !$wgGroupPermissions['*']['read'] && !$wgUser->isAllowed( 'read' ) ) + if ( $module->isReadMode() && !in_array( 'read', User::getGroupPermissions( array( '*' ) ), true ) && + !$wgUser->isAllowed( 'read' ) ) { $this->dieUsageMsg( array( 'readrequired' ) ); } diff --git a/includes/api/ApiParse.php b/includes/api/ApiParse.php index 1396c66d4d..50b9d395ad 100644 --- a/includes/api/ApiParse.php +++ b/includes/api/ApiParse.php @@ -138,7 +138,7 @@ class ApiParse extends ApiBase { $p_result = false; $pcache = ParserCache::singleton(); if ( $wgEnableParserCache ) { - $p_result = $pcache->get( $articleObj, $wgUser ); + $p_result = $pcache->get( $articleObj, $popts ); } if ( !$p_result ) { $p_result = $wgParser->parse( $articleObj->getContent(), $titleObj, $popts ); @@ -162,6 +162,7 @@ class ApiParse extends ApiBase { if ( $params['pst'] || $params['onlypst'] ) { $text = $wgParser->preSaveTransform( $text, $titleObj, $wgUser, $popts ); + $this->getMain()->setVaryCookie(); } if ( $params['onlypst'] ) { // Build a result and bail out diff --git a/includes/api/ApiPatrol.php b/includes/api/ApiPatrol.php index c0450abbe6..361fe408d7 100644 --- a/includes/api/ApiPatrol.php +++ b/includes/api/ApiPatrol.php @@ -41,6 +41,7 @@ class ApiPatrol extends ApiBase { * Patrols the article or provides the reason the patrol failed. */ public function execute() { + $this->getMain()->setCachePrivate(); $params = $this->extractRequestParams(); if ( !isset( $params['rcid'] ) ) { diff --git a/includes/api/ApiPurge.php b/includes/api/ApiPurge.php index aee27e7a8e..4936a60752 100644 --- a/includes/api/ApiPurge.php +++ b/includes/api/ApiPurge.php @@ -42,6 +42,7 @@ class ApiPurge extends ApiBase { */ public function execute() { global $wgUser; + $this->getMain()->setCachePrivate(); $params = $this->extractRequestParams(); if ( !$wgUser->isAllowed( 'purge' ) ) { $this->dieUsageMsg( array( 'cantpurge' ) ); diff --git a/includes/api/ApiQueryAllmessages.php b/includes/api/ApiQueryAllmessages.php index 364c3ac8af..5184ecf669 100644 --- a/includes/api/ApiQueryAllmessages.php +++ b/includes/api/ApiQueryAllmessages.php @@ -48,6 +48,9 @@ class ApiQueryAllmessages extends ApiQueryBase { if ( !is_null( $params['lang'] ) && $params['lang'] != $wgLang->getCode() ) { $oldLang = $wgLang; // Keep $wgLang for restore later $wgLang = Language::factory( $params['lang'] ); + } else if ( is_null( $params['lang'] ) ) { + // Language not determined by URL but by user preferences, so don't cache + $this->getMain()->setVaryCookie(); } $prop = array_flip( (array)$params['prop'] ); diff --git a/includes/api/ApiQueryBlocks.php b/includes/api/ApiQueryBlocks.php index ca4f500954..fc5e3b73f1 100644 --- a/includes/api/ApiQueryBlocks.php +++ b/includes/api/ApiQueryBlocks.php @@ -127,6 +127,9 @@ class ApiQueryBlocks extends ApiQueryBase { 'ipb_auto' => 0 ) ); } + + // Make sure private data (deleted blocks) isn't cached + $this->getMain()->setVaryCookie(); if ( !$wgUser->isAllowed( 'hideuser' ) ) { $this->addWhereFld( 'ipb_deleted', 0 ); } diff --git a/includes/api/ApiQueryDeletedrevs.php b/includes/api/ApiQueryDeletedrevs.php index 1bc55174b3..9f28192f88 100644 --- a/includes/api/ApiQueryDeletedrevs.php +++ b/includes/api/ApiQueryDeletedrevs.php @@ -41,6 +41,7 @@ class ApiQueryDeletedrevs extends ApiQueryBase { public function execute() { global $wgUser; + $this->getMain()->setVaryCookie(); // Before doing anything at all, let's check permissions if ( !$wgUser->isAllowed( 'deletedhistory' ) ) { $this->dieUsage( 'You don\'t have permission to view deleted revision information', 'permissiondenied' ); diff --git a/includes/api/ApiQueryFilearchive.php b/includes/api/ApiQueryFilearchive.php index 1d4590e349..d23c91144e 100644 --- a/includes/api/ApiQueryFilearchive.php +++ b/includes/api/ApiQueryFilearchive.php @@ -43,6 +43,7 @@ class ApiQueryFilearchive extends ApiQueryBase { public function execute() { global $wgUser; + $this->getMain()->setVaryCookie(); // Before doing anything at all, let's check permissions if ( !$wgUser->isAllowed( 'deletedhistory' ) ) { $this->dieUsage( 'You don\'t have permission to view deleted file information', 'permissiondenied' ); diff --git a/includes/api/ApiQueryInfo.php b/includes/api/ApiQueryInfo.php index 4bde932c80..7a32691c41 100644 --- a/includes/api/ApiQueryInfo.php +++ b/includes/api/ApiQueryInfo.php @@ -253,6 +253,7 @@ class ApiQueryInfo extends ApiQueryBase { } if ( $this->fld_watched ) { + $this->getMain()->setVaryCookie(); $this->getWatchedInfo(); } @@ -298,6 +299,9 @@ class ApiQueryInfo extends ApiQueryBase { } if ( !is_null( $this->params['token'] ) ) { + // Don't cache tokens + $this->getMain()->setCachePrivate(); + $tokenFunctions = $this->getTokenFunctions(); $pageInfo['starttimestamp'] = wfTimestamp( TS_ISO_8601, time() ); foreach ( $this->params['token'] as $t ) { @@ -534,7 +538,7 @@ class ApiQueryInfo extends ApiQueryBase { } /** - * Get information about watched status and put it in $watched + * Get information about watched status and put it in $this->watched */ private function getWatchedInfo() { global $wgUser; diff --git a/includes/api/ApiQueryRecentChanges.php b/includes/api/ApiQueryRecentChanges.php index d2388824ab..a9f7794859 100644 --- a/includes/api/ApiQueryRecentChanges.php +++ b/includes/api/ApiQueryRecentChanges.php @@ -143,9 +143,11 @@ class ApiQueryRecentChanges extends ApiQueryBase { // Check permissions global $wgUser; - if ( ( isset( $show['patrolled'] ) || isset( $show['!patrolled'] ) ) && !$wgUser->useRCPatrol() && !$wgUser->useNPPatrol() ) - { - $this->dieUsage( 'You need the patrol right to request the patrolled flag', 'permissiondenied' ); + if ( isset( $show['patrolled'] ) || isset( $show['!patrolled'] ) ) { + $this->getMain()->setVaryCookie(); + if ( !$wgUser->useRCPatrol() && !$wgUser->useNPPatrol() ) { + $this->dieUsage( 'You need the patrol right to request the patrolled flag', 'permissiondenied' ); + } } /* Add additional conditions to query depending upon parameters. */ @@ -412,6 +414,9 @@ class ApiQueryRecentChanges extends ApiQueryBase { } if ( !is_null( $this->token ) ) { + // Don't cache tokens + $this->getMain()->setCachePrivate(); + $tokenFunctions = $this->getTokenFunctions(); foreach ( $this->token as $t ) { $val = call_user_func( $tokenFunctions[$t], $row->rc_cur_id, diff --git a/includes/api/ApiQueryRevisions.php b/includes/api/ApiQueryRevisions.php index 7cf1cb3c31..f80b6855e5 100644 --- a/includes/api/ApiQueryRevisions.php +++ b/includes/api/ApiQueryRevisions.php @@ -411,6 +411,9 @@ class ApiQueryRevisions extends ApiQueryBase { } if ( !is_null( $this->token ) ) { + // Don't cache tokens + $this->getMain()->setCachePrivate(); + $tokenFunctions = $this->getTokenFunctions(); foreach ( $this->token as $t ) { $val = call_user_func( $tokenFunctions[$t], $title->getArticleID(), $title, $revision ); diff --git a/includes/api/ApiQueryUserContributions.php b/includes/api/ApiQueryUserContributions.php index a974a31cb2..310ca768eb 100644 --- a/includes/api/ApiQueryUserContributions.php +++ b/includes/api/ApiQueryUserContributions.php @@ -164,6 +164,8 @@ class ApiQueryContributions extends ApiQueryBase { ); } + // Make sure private data (deleted revisions) isn't cached + $this->getMain()->setVaryCookie(); if ( !$wgUser->isAllowed( 'hideuser' ) ) { $this->addWhere( $this->getDB()->bitAnd( 'rev_deleted', Revision::DELETED_USER ) . ' = 0' ); } @@ -215,9 +217,12 @@ class ApiQueryContributions extends ApiQueryBase { $this->fld_patrolled ) { global $wgUser; + // Don't cache private data + $this->getMain()->setVaryCookie(); if ( !$wgUser->useRCPatrol() && !$wgUser->useNPPatrol() ) { $this->dieUsage( 'You need the patrol right to request the patrolled flag', 'permissiondenied' ); } + // Use a redundant join condition on both // timestamp and ID so we can use the timestamp // index diff --git a/includes/api/ApiQueryUserInfo.php b/includes/api/ApiQueryUserInfo.php index ffe1a0cf3d..8599314323 100644 --- a/includes/api/ApiQueryUserInfo.php +++ b/includes/api/ApiQueryUserInfo.php @@ -40,6 +40,7 @@ class ApiQueryUserInfo extends ApiQueryBase { } public function execute() { + $this->getMain()->setCachePrivate(); $params = $this->extractRequestParams(); $result = $this->getResult(); $r = array(); diff --git a/includes/api/ApiQueryUsers.php b/includes/api/ApiQueryUsers.php index ca54366e67..0a233a727b 100644 --- a/includes/api/ApiQueryUsers.php +++ b/includes/api/ApiQueryUsers.php @@ -162,6 +162,9 @@ if ( !defined( 'MEDIAWIKI' ) ) { } if ( !is_null( $params['token'] ) ) { + // Don't cache tokens + $this->getMain()->setCachePrivate(); + $tokenFunctions = $this->getTokenFunctions(); foreach ( $params['token'] as $t ) { $val = call_user_func( $tokenFunctions[$t], $user ); diff --git a/includes/api/ApiQueryWatchlist.php b/includes/api/ApiQueryWatchlist.php index 0064e16f01..ad51ca8f3d 100644 --- a/includes/api/ApiQueryWatchlist.php +++ b/includes/api/ApiQueryWatchlist.php @@ -74,6 +74,7 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { $this->fld_notificationtimestamp = isset( $prop['notificationtimestamp'] ); if ( $this->fld_patrol ) { + $this->getMain()->setVaryCookie(); if ( !$user->useRCPatrol() && !$user->useNPPatrol() ) { $this->dieUsage( 'patrol property is not available', 'patrol' ); } @@ -141,9 +142,11 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { } // Check permissions. FIXME: should this check $user instead of $wgUser? - if ( ( isset( $show['patrolled'] ) || isset( $show['!patrolled'] ) ) && !$wgUser->useRCPatrol() && !$wgUser->useNPPatrol() ) - { - $this->dieUsage( 'You need the patrol right to request the patrolled flag', 'permissiondenied' ); + if ( isset( $show['patrolled'] ) || isset( $show['!patrolled'] ) ) { + $this->getMain()->setVaryCookie(); + if ( !$wgUser->useRCPatrol() && !$wgUser->useNPPatrol() ) { + $this->dieUsage( 'You need the patrol right to request the patrolled flag', 'permissiondenied' ); + } } /* Add additional conditions to query depending upon parameters. */ diff --git a/includes/api/ApiWatch.php b/includes/api/ApiWatch.php index f0532f995d..6ef86917ee 100644 --- a/includes/api/ApiWatch.php +++ b/includes/api/ApiWatch.php @@ -41,6 +41,7 @@ class ApiWatch extends ApiBase { public function execute() { global $wgUser; + $this->getMain()->setCachePrivate(); if ( !$wgUser->isLoggedIn() ) { $this->dieUsage( 'You must be logged-in to have a watchlist', 'notloggedin' ); }