Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F44155718
T361295.patch
Dreamy_Jazz (WBrown (WMF))
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Authored By
Dreamy_Jazz
Apr 2 2024, 11:28 AM
2024-04-02 11:28:32 (UTC+0)
Size
9 KB
Referenced Files
None
Subscribers
None
T361295.patch
View Options
From ddabd50c0effb46d27ca7ab8066b7d95a92fd722 Mon Sep 17 00:00:00 2001
From: Dreamy Jazz <wpgbrown@wikimedia.org>
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
File Metadata
Details
Attached
Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
15285734
Default Alt Text
T361295.patch (9 KB)
Attached To
Mode
T361295: CVE-2024-40610: CheckUser API for the 'ipusers' and 'actions' request type shows hidden usernames to those who cannot see them
Attached
Detach File
Event Timeline
Log In to Comment