From ddabd50c0effb46d27ca7ab8066b7d95a92fd722 Mon Sep 17 00:00:00 2001 From: Dreamy Jazz Date: Tue, 2 Apr 2024 11:42:33 +0100 Subject: [PATCH] SECURITY: Hide hidden usernames in the CheckUser API Why: * The CheckUser API displays usernames in the results of the 'actions' and 'ipusers'. There is code that hides the usernames in the 'actions' response if the username is hidden from the edit, but the code does not look for whether the username is hidden via a block that applies 'hideuser'. * Furthermore, for some log events, the title of the action can be a username. This username may also need to be hidden from the current user, and so the deleted status for that username should also be checked. * These usernames need to be hidden as it represents a information leak. If a user needs to see these usernames they can either be granted the rights to see suppressed content or ask another checkuser with these rights to inspect the data. What: * Add code to ApiQueryCheckUserIpUsersResponse to replace the 'name' in the results with the 'rev-deleted-user' message if the username is hidden from the current authority. * Add code to ApiQueryCheckUserActionsResponse to replace the 'user' with the 'rev-deleted-user' message if the performer of the row is hidden with a 'hideuser' block. * If the title of the row is a username and that username is hidden from the current user, then replace the 'title' with the 'rev-deleted-user' message. * Add tests to verify that the security patch worked. Bug: T361295 Change-Id: I7a797d509e86916b8cc8a5b6519c42e6aa355ea9 --- .../ApiQueryCheckUserActionsResponse.php | 25 ++++++- .../ApiQueryCheckUserIpUsersResponse.php | 37 +++++++++++ .../ApiQueryCheckUserResponseFactory.php | 3 +- .../integration/Api/ApiQueryCheckUserTest.php | 66 +++++++++++++++++++ 4 files changed, 129 insertions(+), 2 deletions(-) diff --git a/src/Api/CheckUser/ApiQueryCheckUserActionsResponse.php b/src/Api/CheckUser/ApiQueryCheckUserActionsResponse.php index eb8b08fb..40036cc5 100644 --- a/src/Api/CheckUser/ApiQueryCheckUserActionsResponse.php +++ b/src/Api/CheckUser/ApiQueryCheckUserActionsResponse.php @@ -104,7 +104,30 @@ class ApiQueryCheckUserActionsResponse extends ApiQueryCheckUserAbstractResponse 'agent' => $row->agent, ]; - $summary = $this->getSummary( $row, new UserIdentityValue( $row->user ?? 0, $row->user_text ) ); + $user = $this->userFactory->newFromUserIdentity( + new UserIdentityValue( $row->user ?? 0, $row->user_text ) + ); + // If the 'user' key is a username which the current authority cannot see, then replace it with the + // 'rev-deleted-user' message. + if ( $user->isHidden() && !$this->module->getUser()->isAllowed( 'hideuser' ) ) { + $action['user'] = $this->messageLocalizer->msg( 'rev-deleted-user' )->text(); + } + + // If the title is a user page and the username in this user page link is hidden from the current authority, + // then replace the title with the 'rev-deleted-user' message. + $title = Title::makeTitle( $row->namespace, $row->title ); + if ( $title->getNamespace() === NS_USER ) { + $titleUser = $this->userFactory->newFromName( $title->getBaseText() ); + if ( + $titleUser && + $titleUser->isHidden() && + !$this->module->getUser()->isAllowed( 'hideuser' ) + ) { + $action['title'] = $this->messageLocalizer->msg( 'rev-deleted-user' )->text(); + } + } + + $summary = $this->getSummary( $row, $user ); if ( $summary !== null ) { $action['summary'] = $summary; } diff --git a/src/Api/CheckUser/ApiQueryCheckUserIpUsersResponse.php b/src/Api/CheckUser/ApiQueryCheckUserIpUsersResponse.php index 613a8b42..3655f455 100644 --- a/src/Api/CheckUser/ApiQueryCheckUserIpUsersResponse.php +++ b/src/Api/CheckUser/ApiQueryCheckUserIpUsersResponse.php @@ -3,12 +3,41 @@ namespace MediaWiki\CheckUser\Api\CheckUser; use ApiResult; +use MediaWiki\CheckUser\Api\ApiQueryCheckUser; +use MediaWiki\CheckUser\Services\CheckUserLogService; +use MediaWiki\CheckUser\Services\CheckUserLookupUtils; +use MediaWiki\Config\Config; +use MediaWiki\User\UserFactory; +use MediaWiki\User\UserNameUtils; +use MessageLocalizer; +use Wikimedia\Rdbms\IConnectionProvider; use Wikimedia\Rdbms\IExpression; use Wikimedia\Rdbms\SelectQueryBuilder; use Wikimedia\Timestamp\ConvertibleTimestamp; class ApiQueryCheckUserIpUsersResponse extends ApiQueryCheckUserAbstractResponse { + private UserFactory $userFactory; + private MessageLocalizer $messageLocalizer; + + public function __construct( + ApiQueryCheckUser $module, + IConnectionProvider $dbProvider, + Config $config, + MessageLocalizer $messageLocalizer, + CheckUserLogService $checkUserLogService, + UserNameUtils $userNameUtils, + CheckUserLookupUtils $checkUserLookupUtils, + UserFactory $userFactory + ) { + parent::__construct( + $module, $dbProvider, $config, $messageLocalizer, $checkUserLogService, + $userNameUtils, $checkUserLookupUtils + ); + $this->userFactory = $userFactory; + $this->messageLocalizer = $messageLocalizer; + } + /** @inheritDoc */ public function getRequestType(): string { return 'ipusers'; @@ -45,6 +74,14 @@ class ApiQueryCheckUserIpUsersResponse extends ApiQueryCheckUserAbstractResponse $resultUsers = []; foreach ( $users as $userName => $userData ) { + // Hide the user name if it is hidden from the current authority. + $user = $this->userFactory->newFromName( $userName ); + if ( $user !== null && $user->isHidden() && !$this->module->getUser()->isAllowed( 'hideuser' ) ) { + // If the username is hidden from the current user, then hide the username in the results using the + // 'rev-deleted-user' message. + $userName = $this->messageLocalizer->msg( 'rev-deleted-user' )->text(); + } + $userData['name'] = $userName; ApiResult::setIndexedTagName( $userData['ips'], 'ip' ); ApiResult::setIndexedTagName( $userData['agents'], 'agent' ); diff --git a/src/Services/ApiQueryCheckUserResponseFactory.php b/src/Services/ApiQueryCheckUserResponseFactory.php index 904c1a6f..0b38999c 100644 --- a/src/Services/ApiQueryCheckUserResponseFactory.php +++ b/src/Services/ApiQueryCheckUserResponseFactory.php @@ -107,7 +107,8 @@ class ApiQueryCheckUserResponseFactory { $this->messageLocalizer, $this->checkUserLogService, $this->userNameUtils, - $this->checkUserLookupUtils + $this->checkUserLookupUtils, + $this->userFactory ); default: $module->dieWithError( 'apierror-checkuser-invalidmode', 'invalidmode' ); diff --git a/tests/phpunit/integration/Api/ApiQueryCheckUserTest.php b/tests/phpunit/integration/Api/ApiQueryCheckUserTest.php index 5df1381d..5bd17626 100644 --- a/tests/phpunit/integration/Api/ApiQueryCheckUserTest.php +++ b/tests/phpunit/integration/Api/ApiQueryCheckUserTest.php @@ -373,6 +373,72 @@ class ApiQueryCheckUserTest extends ApiTestCase { ); } + public function testActionsForHiddenUser() { + // Block CheckUserAPITestUser1 with 'hideuser' enabled. + $blockStatus = $this->getServiceContainer()->getBlockUserFactory() + ->newBlockUser( + $this->getServiceContainer()->getUserIdentityLookup()->getUserIdentityByName( 'CheckUserAPITestUser1' ), + $this->getTestUser( [ 'sysop', 'suppress' ] )->getUser(), + 'infinity', + 'block to hide the test user', + [ 'isHideUser' => true ] + )->placeBlock(); + $this->assertStatusGood( $blockStatus ); + // Perform an 'actions' request and verify that the hidden user is not shown in the response. + $this->testResponseFromApi( + 'actions', 'edits', '127.2.3.4', '-3 months', true, + [ + [ + 'timestamp' => '2023-04-05T06:07:12Z', + 'ns' => 2, + 'title' => wfMessage( 'rev-deleted-user' )->text(), + 'user' => wfMessage( 'rev-deleted-user' )->text(), + 'ip' => '1.2.3.4', + 'agent' => 'user-agent-for-logout', + 'summary' => wfMessage( 'checkuser-logout' )->text(), + 'xff' => '127.2.3.4', + ], + [ + 'timestamp' => '2023-04-05T06:07:11Z', + 'ns' => 0, + 'title' => 'CheckUserTestPage', + 'user' => wfMessage( 'rev-deleted-user' )->text(), + 'ip' => '1.2.3.4', + 'agent' => 'user-agent-for-edits', + 'summary' => 'Test1233', + 'xff' => '127.2.3.4', + ], + ] + ); + } + + public function testIpUsersForHiddenUser() { + // Block CheckUserAPITestUser1 with 'hideuser' enabled. + $blockStatus = $this->getServiceContainer()->getBlockUserFactory() + ->newBlockUser( + $this->getServiceContainer()->getUserIdentityLookup()->getUserIdentityByName( 'CheckUserAPITestUser1' ), + $this->getTestUser( [ 'sysop', 'suppress' ] )->getUser(), + 'infinity', + 'block to hide the test user', + [ 'isHideUser' => true ] + )->placeBlock(); + $this->assertStatusGood( $blockStatus ); + // Perform an 'ipusers' request and verify that the hidden user is not shown in the response. + $this->testResponseFromApi( + 'ipusers', 'ipusers', '127.2.3.4', '-3 months', true, + [ + [ + 'name' => wfMessage( 'rev-deleted-user' )->text(), + 'end' => '2023-04-05T06:07:12Z', + 'editcount' => 2, + 'agents' => [ 'user-agent-for-logout', 'user-agent-for-edits' ], + 'ips' => [ '1.2.3.4' ], + 'start' => '2023-04-05T06:07:11Z' + ], + ] + ); + } + /** @dataProvider provideCuRequestTypesThatAcceptAUsernameTarget */ public function testApiForNonExistentUserAsTarget( $requestType ) { $this->expectApiErrorCode( 'nosuchuser' ); -- 2.34.1