From ed4a3287b4f860ff2a3dde6c85c40c25a89fccd4 Mon Sep 17 00:00:00 2001 From: DannyS712 Date: Wed, 16 Dec 2020 01:50:04 +0000 Subject: [PATCH] SECURITY: Act like users don't exist if hidden from viewer When viewing Special:Contributions for a hidden user and a missing user, or the user page of a hidden user and a missing user, if the viewer cannot see hidden users the output should be the same for hidden users and missing users. To that end * In OutputPage.php, only set the `wgRelevantUserName` javascript variable if the user is not hidden, or the viewer can see hidden users * In Article.php, show the `userpage-userdoesnotexist-view` on user pages of hidden users if the viewer cannot see hidden users * In SkinTemplate.php, do not add user-specific sidebar links (contributions, logs, mute, etc.) if the user is hidden and the viewer cannot see hidden users * In SpecialContributions.php, stop calling Skin::setRelevantUser for non-existing users, so that callers of Skin::getRelevantUser can ignore users that are hidden from the viewer without creating divergent behavior * In SpecialContributions.php, for users that do exist but are hidden from the viewer, don't show `sp-contributions-footer`, but do show `contributions-userdoesnotexist` Bug: T120883 Change-Id: I83b723402f315447bc4b50992e28620e3daace8f --- includes/OutputPage.php | 4 +++- includes/page/Article.php | 8 ++++++++ includes/skins/SkinTemplate.php | 10 ++++++++++ includes/specials/SpecialContributions.php | 22 ++++++++++++++++++++-- 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 99dd4a7c0e..9cae157bda 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -3162,7 +3162,9 @@ class OutputPage extends ContextSource { $vars['wgRedirectedFrom'] = $this->mRedirectedFrom->getPrefixedDBkey(); } - if ( $relevantUser ) { + if ( $relevantUser && ( !$relevantUser->isHidden() || $user->isAllowed( 'hideuser' ) ) { + // T120883 if the user is hidden and the viewer cannot see + // hidden users, pretend like it does not exist at all. $vars['wgRelevantUserName'] = $relevantUser->getName(); } diff --git a/includes/page/Article.php b/includes/page/Article.php index 8fff614782..784d8843fb 100644 --- a/includes/page/Article.php +++ b/includes/page/Article.php @@ -1164,6 +1164,14 @@ class Article implements Page { $ip = User::isIP( $rootPart ); $block = Block::newFromTarget( $user, $user ); + if ( $user && $user->isLoggedIn() && $user->isHidden() && + !$this->getContext()->getUser()->isAllowed( 'hideuser' ) + ) { + // T120883 if the user is hidden and the viewer cannot see hidden + // users, pretend like it does not exist at all. + $user = false; + } + if ( !( $user && $user->isLoggedIn() ) && !$ip ) { # User does not exist $outputPage->wrapWikiMsg( "
\n\$1\n
", [ 'userpage-userdoesnotexist-view', wfEscapeWikiText( $rootPart ) ] ); diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php index 203326f126..7753566143 100644 --- a/includes/skins/SkinTemplate.php +++ b/includes/skins/SkinTemplate.php @@ -1349,6 +1349,16 @@ class SkinTemplate extends Skin { } $user = $this->getRelevantUser(); + + // The relevant user should only be set if it exists. However, if it exists but is hidden, + // and the viewer cannot see hidden users, this exposes the fact that the user exists; + // pretend like the user does not exist in such cases, by setting $user to null, which + // is what getRelevantUser returns if there is no user set (though it is documented as + // always returning a User...) See T120883 + if ( $user && $user->isRegistered() && $user->isHidden() && !$this->getUser()->isAllowed( 'hideuser' ) ) { + $user = null; + } + if ( $user ) { $rootUser = $user->getName(); diff --git a/includes/specials/SpecialContributions.php b/includes/specials/SpecialContributions.php index 2812541c16..572d2a980b 100644 --- a/includes/specials/SpecialContributions.php +++ b/includes/specials/SpecialContributions.php @@ -123,7 +123,15 @@ class SpecialContributions extends IncludableSpecialPage { # For IP ranges, we want the contributionsSub, but not the skin-dependent # links under 'Tools', which may include irrelevant links like 'Logs'. - if ( !IP::isValidRange( $target ) ) { + if ( !IP::isValidRange( $target ) && + ( User::isIP( $target ) || $userObj->isRegistered() ) + ) { + // Don't add non-existent users, because hidden users + // that we add here will be removed later to pretend + // that they don't exist, and if users that actually don't + // exist are added here and then not removed, it exposes + // which users exist and are hidden vs. which actually don't + // exist. But, do set the relevant user for single IPs. $this->getSkin()->setRelevantUser( $userObj ); } } @@ -277,7 +285,10 @@ class SpecialContributions extends IncludableSpecialPage { } elseif ( $userObj->isAnon() ) { // No message for non-existing users $message = ''; + } elseif ( $userObj->isHidden() && !$this->getUser()->isAllowed( 'hideuser' ) ) { + $message = ''; } else { + // Not hidden, or hidden but the viewer can still see it $message = 'sp-contributions-footer'; } @@ -301,7 +312,14 @@ class SpecialContributions extends IncludableSpecialPage { * Could be combined. */ protected function contributionsSub( $userObj ) { - if ( $userObj->isAnon() ) { + $isAnon = $userObj->isAnon(); + if ( !$isAnon && $userObj->isHidden() && !$this->getUser->isAllowed( 'hideuser' ) ) { + // T120883 if the user is hidden and the viewer cannot see hidden + // users, pretend like it does not exist at all. + $isAnon = true; + } + + if ( $isAnon ) { // Show a warning message that the user being searched for doesn't exists. // User::isIP returns true for IP address and usemod IPs like '123.123.123.xxx', // but returns false for IP ranges. We don't want to suggest either of these are -- 2.25.1