From 94bfc05acc553ade067892869cb41d087a38f254 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Taavi=20V=C3=A4=C3=A4n=C3=A4nen?= Date: Thu, 21 Oct 2021 14:19:39 +0300 Subject: [PATCH] SECURITY: Send original client info on x-wiki requests Bug: T285116 --- includes/EchoHooks.php | 12 +++++++----- includes/ForeignWikiRequest.php | 34 ++++++++++++++++++++++++++------- includes/NotifUser.php | 10 ++++++---- includes/api/ApiCrossWiki.php | 2 +- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/includes/EchoHooks.php b/includes/EchoHooks.php index 410a6ea2..febd036d 100644 --- a/includes/EchoHooks.php +++ b/includes/EchoHooks.php @@ -893,16 +893,18 @@ class EchoHooks implements RecentChange_saveHook { $markAsReadIds = array_map( 'intval', $markAsReadIds ); // Look up the notifications on the foreign wiki $notifUser = MWEchoNotifUser::newFromUser( $user ); - $notifInfo = $notifUser->getForeignNotificationInfo( $markAsReadIds, $markAsReadWiki ); + $notifInfo = $notifUser->getForeignNotificationInfo( $markAsReadIds, $markAsReadWiki, $request ); foreach ( $notifInfo as $id => $info ) { $subtractions[$info['section']]++; } // Schedule a deferred update to mark these notifications as read on the foreign wiki - DeferredUpdates::addCallableUpdate( static function () use ( $user, $markAsReadIds, $markAsReadWiki ) { - $notifUser = MWEchoNotifUser::newFromUser( $user ); - $notifUser->markReadForeign( $markAsReadIds, $markAsReadWiki ); - } ); + DeferredUpdates::addCallableUpdate( + static function () use ( $user, $markAsReadIds, $markAsReadWiki, $request ) { + $notifUser = MWEchoNotifUser::newFromUser( $user ); + $notifUser->markReadForeign( $markAsReadIds, $markAsReadWiki, $request ); + } + ); } } diff --git a/includes/ForeignWikiRequest.php b/includes/ForeignWikiRequest.php index cce5ff8b..475f86ff 100644 --- a/includes/ForeignWikiRequest.php +++ b/includes/ForeignWikiRequest.php @@ -49,14 +49,21 @@ class EchoForeignWikiRequest { /** * Execute the request + * @param WebRequest|null $originalRequest Original request data to be sent with these requests * @return array[] [ wiki => result ] */ - public function execute() { + public function execute( ?WebRequest $originalRequest = null ) { if ( !$this->canUseCentralAuth() ) { return []; } - $reqs = $this->getRequestParams( $this->method, [ $this, 'getQueryParams' ] ); + $reqs = $this->getRequestParams( + $this->method, + function ( string $wiki ) use ( $originalRequest ) { + return $this->getQueryParams( $wiki, $originalRequest ); + }, + $originalRequest + ); return $this->doRequests( $reqs ); } @@ -121,10 +128,11 @@ class EchoForeignWikiRequest { * This method fetches the tokens for all requested wikis at once and caches the result. * * @param string $wiki Name of the wiki to get a token for + * @param WebRequest|null $originalRequest Original request data to be sent with these requests * @suppress PhanTypeInvalidCallableArraySize getRequestParams can take an array, too (phan bug) * @return string Token, or empty string if an unable to retrieve the token. */ - protected function getCsrfToken( $wiki ) { + protected function getCsrfToken( $wiki, ?WebRequest $originalRequest ) { if ( $this->csrfTokens === null ) { $this->csrfTokens = []; $reqs = $this->getRequestParams( 'GET', [ @@ -133,7 +141,7 @@ class EchoForeignWikiRequest { 'type' => $this->tokenType, 'format' => 'json', 'centralauthtoken' => $this->getCentralAuthToken( $this->user ), - ] ); + ], $originalRequest ); $responses = $this->doRequests( $reqs ); foreach ( $responses as $w => $response ) { if ( isset( $response['query']['tokens']['csrftoken'] ) ) { @@ -156,9 +164,10 @@ class EchoForeignWikiRequest { * @param string $method 'GET' or 'POST' * @param array|callable $params Associative array of query string / POST parameters, * or a callback that takes a wiki name and returns such an array + * @param WebRequest|null $originalRequest Original request data to be sent with these requests * @return array[] Array of request parameters to pass to doRequests(), keyed by wiki name */ - protected function getRequestParams( $method, $params ) { + protected function getRequestParams( $method, $params, ?WebRequest $originalRequest ) { $apis = EchoForeignNotifications::getApiEndpoints( $this->wikis ); if ( !$apis ) { return []; @@ -172,6 +181,16 @@ class EchoForeignWikiRequest { 'url' => $api['url'], $queryKey => is_callable( $params ) ? $params( $wiki ) : $params ]; + + if ( $originalRequest ) { + $reqs[$wiki]['headers'] = [ + 'X-Forwarded-For' => $originalRequest->getIP(), + 'User-Agent' => ( + $originalRequest->getHeader( 'User-Agent' ) + . ' (via EchoForeignWikiRequest MediaWiki/' . MW_VERSION . ')' + ), + ]; + } } return $reqs; @@ -179,9 +198,10 @@ class EchoForeignWikiRequest { /** * @param string $wiki Wiki name + * @param WebRequest|null $originalRequest Original request data to be sent with these requests * @return array */ - protected function getQueryParams( $wiki ) { + protected function getQueryParams( $wiki, ?WebRequest $originalRequest ) { $extraParams = []; if ( $this->wikiParam ) { // Only request data from that specific wiki, or they'd all spawn @@ -189,7 +209,7 @@ class EchoForeignWikiRequest { $extraParams[$this->wikiParam] = $wiki; } if ( $this->method === 'POST' ) { - $extraParams['token'] = $this->getCsrfToken( $wiki ); + $extraParams['token'] = $this->getCsrfToken( $wiki, $originalRequest ); } return [ diff --git a/includes/NotifUser.php b/includes/NotifUser.php index b7d1bbda..42a8bce0 100644 --- a/includes/NotifUser.php +++ b/includes/NotifUser.php @@ -386,8 +386,9 @@ class MWEchoNotifUser { * * @param int[] $eventIds Event IDs to mark as read * @param string $wiki Wiki name + * @param WebRequest|null $originalRequest Original request data to be sent with these requests */ - public function markReadForeign( array $eventIds, $wiki ) { + public function markReadForeign( array $eventIds, $wiki, ?WebRequest $originalRequest = null ) { $foreignReq = new EchoForeignWikiRequest( $this->mUser, [ @@ -398,7 +399,7 @@ class MWEchoNotifUser { 'wikis', 'csrf' ); - $foreignReq->execute(); + $foreignReq->execute( $originalRequest ); } /** @@ -406,9 +407,10 @@ class MWEchoNotifUser { * * @param int[] $eventIds Event IDs to look up. Only unread notifications can be found. * @param string $wiki Wiki name + * @param WebRequest|null $originalRequest Original request data to be sent with these requests * @return array[] Array of notification data as returned by api.php, keyed by event ID */ - public function getForeignNotificationInfo( array $eventIds, $wiki ) { + public function getForeignNotificationInfo( array $eventIds, $wiki, ?WebRequest $originalRequest = null ) { $foreignReq = new EchoForeignWikiRequest( $this->mUser, [ @@ -421,7 +423,7 @@ class MWEchoNotifUser { [ $wiki ], 'notwikis' ); - $foreignResults = $foreignReq->execute(); + $foreignResults = $foreignReq->execute( $originalRequest ); $list = $foreignResults[$wiki]['query']['notifications']['list'] ?? []; $result = []; diff --git a/includes/api/ApiCrossWiki.php b/includes/api/ApiCrossWiki.php index b632a065..ddc7d549 100644 --- a/includes/api/ApiCrossWiki.php +++ b/includes/api/ApiCrossWiki.php @@ -36,7 +36,7 @@ trait ApiCrossWiki { $this->getModulePrefix() . 'wikis', $tokenType !== false ? $tokenType : null ); - return $foreignReq->execute(); + return $foreignReq->execute( $this->getRequest() ); } /** -- 2.34.1