SECURITY: Hide 'logs' link if page is a hidden user in 'Get actions'

Why:
* Special:CheckUser 'Get actions' has links shown before a row in
  the results which provide links to the diff for an edit and the
  logs page for a log action.
* When a user is blocked with 'hideuser' enabled, Special:CheckUser
  still adds the 'logs' link for actions which are associated with
  this user.
* This association is partly done through the username being used
  as the 'page' query paramter value, so that a user can filter for
  logs performed by this user.
* However, in the case of a user which the current authority cannot
  see this causes the username to be leaked via the URL of that
  'logs' link.
* Removing the 'logs' link if the 'page' query parameter is a
  hidden user should fix this issue.

What:
* Update CheckUserGetActionsPager::getLinksFromRow to check if the
  'page' title is a username (NS_USER) and if so, whether the
  username is hidden from the current authority. If it is hidden,
  then don't add the 'logs' link to the links shown at the start
  of the result line.
* Don't display the links at the start of the row if there are no
  links to display, including the separator which is usually added
  after the links.
* Add tests to verify that the security fix has worked.

Bug: T361479
Change-Id: I67d76232b131054713534d619d04972f4d84e232
这个提交包含在:
Dreamy Jazz 2024-04-02 00:03:08 +01:00
父节点 28bc28b664
当前提交 99c6128bb5
共有 3 个文件被更改,包括 89 次插入19 次删除

查看文件

@ -4,6 +4,7 @@ namespace MediaWiki\CheckUser\CheckUser\Pagers;
use HtmlArmor;
use IContextSource;
use LogEventsList;
use LogFormatter;
use LogicException;
use LogPage;
@ -166,8 +167,6 @@ class CheckUserGetActionsPager extends AbstractCheckUserPager {
*/
public function formatRow( $row ): string {
$templateParams = [];
// Create diff/hist/page links
$templateParams['links'] = $this->getLinksFromRow( $row );
// Show date
$templateParams['timestamp'] =
$this->getLanguage()->userTime( wfTimestamp( TS_MW, $row->timestamp ), $this->getUser() );
@ -189,6 +188,9 @@ class CheckUserGetActionsPager extends AbstractCheckUserPager {
$hidden = $this->userFactory->newFromUserIdentity( $user )->isHidden()
&& !$this->getAuthority()->isAllowed( 'hideuser' );
}
// Create diff/hist/page links
$templateParams['links'] = $this->getLinksFromRow( $row, $user );
$templateParams['showLinks'] = $templateParams['links'] !== '';
if ( $hidden ) {
$templateParams['userLink'] = Html::element(
'span',
@ -299,16 +301,17 @@ class CheckUserGetActionsPager extends AbstractCheckUserPager {
/**
* @param stdClass $row
* @param UserIdentity $performer The user that performed the action represented by this row.
* @return string diff, hist and page other links related to the change
*/
protected function getLinksFromRow( stdClass $row ): string {
protected function getLinksFromRow( stdClass $row, UserIdentity $performer ): string {
$links = [];
// Log items
// Due to T315224 triple equals for type does not work for sqlite.
if ( $row->type == RC_LOG ) {
$title = Title::makeTitle( $row->namespace, $row->title );
$links['log'] = '';
if ( $this->eventTableReadNew && isset( $row->log_id ) ) {
if ( $this->eventTableReadNew && isset( $row->log_id ) && $row->log_id ) {
$links['log'] = Html::rawElement( 'span', [],
$this->getLinkRenderer()->makeKnownLink(
SpecialPage::getTitleFor( 'Log' ),
@ -316,21 +319,49 @@ class CheckUserGetActionsPager extends AbstractCheckUserPager {
[],
[ 'logid' => $row->log_id ]
)
) . ' ';
);
}
$links['log'] .= Html::rawElement( 'span', [],
// Hide the 'logs' link if the page is a username and the current authority does not have permission to see
// the username in question (T361479).
$hidden = false;
if ( $title->getNamespace() === NS_USER ) {
$user = $this->userFactory->newFromName( $title->getBaseText() );
if ( isset( $row->log_deleted ) && $performer->getName() === $title->getText() ) {
// If the username of the performer is the same as the title, we can also check whether the
// performer of the log entry is hidden.
$hidden = !LogEventsList::userCanBitfield(
$row->log_deleted,
LogPage::DELETED_USER,
$this->getContext()->getAuthority()
);
}
if ( $user !== null && !$hidden ) {
// If LogEventsList::userCanBitfield said the log entry isn't hidden, then also check if the user
// is hidden in general (via a block with 'hideuser' set).
// LogEventsList::userCanBitfield can return false while this is true for events from
// cu_private_event, as log_deleted is always 0 for those rows (as they cannot be revision deleted).
$hidden = $user->isHidden() && !$this->getAuthority()->isAllowed( 'hideuser' );
}
}
if ( !$hidden ) {
$links['log'] .= Html::rawElement( 'span', [],
$this->getLinkRenderer()->makeKnownLink(
SpecialPage::getTitleFor( 'Log' ),
new HtmlArmor( $this->message['checkuser-logs-link-text'] ),
[],
[ 'page' => $title->getPrefixedText() ]
)
);
$links['log'] = Html::rawElement(
'span',
[ 'class' => 'mw-changeslist-links' ],
$links['log']
);
SpecialPage::getTitleFor( 'Log' ),
new HtmlArmor( $this->message['checkuser-logs-link-text'] ),
[],
[ 'page' => $title->getPrefixedText() ]
)
);
}
// Only add the log related links if we have any to add. There may be none for cu_private_event rows when
// the username listed as the title is blocked with 'hideuser' enabled.
if ( $links['log'] !== '' ) {
$links['log'] = Html::rawElement(
'span',
[ 'class' => 'mw-changeslist-links' ],
$links['log']
);
}
} else {
$title = Title::makeTitle( $row->namespace, $row->title );
// New pages

查看文件

@ -1,6 +1,8 @@
<li>
{{{links}}}
<span class="mw-changeslist-separator"></span>
{{#showLinks}}
{{{links}}}
<span class="mw-changeslist-separator"></span>
{{/showLinks}}
{{timestamp}}
<span class="mw-changeslist-separator"></span>
<span class="mw-checkuser-user-link{{#userLinkClass}} {{userLinkClass}}{{/userLinkClass}}">

查看文件

@ -260,6 +260,43 @@ class CheckUserGetActionsPagerTest extends CheckUserPagerTestBase {
];
}
public function testFormatRowWhenTitleIsHiddenUser() {
// Get a user which has been blocked with the 'hideuser' enabled.
$hiddenUser = $this->getTestUser()->getUser();
$blockStatus = $this->getServiceContainer()->getBlockUserFactory()
->newBlockUser(
$hiddenUser,
$this->getTestUser( [ 'suppress', 'sysop' ] )->getAuthority(),
'infinity',
'block to hide the test user',
[ 'isHideUser' => true ]
)->placeBlock();
$this->assertStatusGood( $blockStatus );
// Test that when the title is the username of a hidden user, the 'logs' link is not set (as this uses the
// the title for the row).
$this->testFormatRow(
[
'log_type' => 'test',
'log_action' => 'phpunit',
'log_deleted' => 0,
'namespace' => NS_USER,
'title' => $hiddenUser->getUserPage()->getText(),
'user_text' => $hiddenUser->getName(),
'user' => $hiddenUser->getId(),
'log_params' => LogEntryBase::makeParamBlob( [] ),
'client_hints_reference_id' => 1,
'client_hints_reference_type' => UserAgentClientHintsManager::IDENTIFIER_CU_CHANGES,
],
[ $hiddenUser->getName() => '' ],
[ $hiddenUser->getId() => true ],
[],
new ClientHintsBatchFormatterResults( [], [] ),
[ 'showLinks' => false ],
SCHEMA_COMPAT_NEW,
true,
);
}
public static function provideFormatRow() {
// @todo test the rest of the template parameters.
return [