Page MenuHomePhabricator

CVE-2021-30153: ApiVisualEditor leaks info about hidden users
Closed, ResolvedPublicSecurity

Description

See related {T120883}
rMWc711960e3f15: SECURITY: Act like users don't exist if hidden from viewer fixed EditPage in core; the same code exists in ApiVisualEditor (with the comment // HACK of course this code is partly duplicated from EditPage.php :() and also needs to be fixed there

To reproduce:
with the visual editor (both the visual and wikitext modes), visit
a userpage where the user exists but is hidden and a page where the user doesn't exist.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Applying the changes from the MediaWiki core patch to our code:


sbassett added a subscriber: sbassett.

@matmarex @DannyS712 - If someone's around at 22:00 UTC for today's security deployment window to help test, we can deploy this to .22 today. Otherwise it will likely have to wait until after the upcoming holiday break (1-4-2021 or so).

This comment was removed by Legoktm.

I've deployed the patch for .22. I then tested @matmarex' above examples (thanks), first on mwdebug1002 and then on production; both look good. Logstash seems happy as well. Since VE is bundled as of 1.35, @Reedy is tracking this for the next security release (T270459), so let's keep this task restricted until then.

I've deployed the patch for .22. I then tested @matmarex' above examples (thanks), first on mwdebug1002 and then on production; both look good. Logstash seems happy as well. Since VE is bundled as of 1.35, @Reedy is tracking this for the next security release (T270459), so let's keep this task restricted until then.

I can confirm that it works for me too, thanks for deploying over the holidays! Should this task be closed now?

sbassett changed the task status from Open to Stalled.Dec 22 2020, 3:39 PM
sbassett triaged this task as Low priority.

Should this task be closed now?

I forget what we normally do for held patches like this. This will need a CVE at some point and possibly backports to 1.31 and 1.35, though we definitely have to wait on the latter for now.

Already conflicted with rEVED75f38008d9e6: Use User->isRegistered(), not deprecated isLoggedIn()

New version incoming

Or not; ^ from above works

Reedy changed the task status from Stalled to Open.Mar 30 2021, 1:05 AM

applies fine to REL1_35

REL1_31

commit 93dbd12a600ccd81a8dc38bf4eace3f6857bc49f (HEAD -> REL1_31)
Author: Bartosz Dziewoński <matma.rex@gmail.com>
Date:   Fri Dec 18 16:47:05 2020 +0100

    SECURITY: Act like users don't exist if hidden from viewer (VE edit notices)
    
    Applying the changes from the MediaWiki core patch for T120883
    (Ife272a0eb1f3322bc8eb30ca803bd21801acba3e) to our duplicated
    code implementing the same functionality.
    
    Bug: T270453
    Change-Id: I1b2de322aa0c69eb6d3b3ffadaed3fbaa3a58bca

diff --git a/includes/ApiVisualEditor.php b/includes/ApiVisualEditor.php
index 08410adb4..32457c3ff 100644
--- a/includes/ApiVisualEditor.php
+++ b/includes/ApiVisualEditor.php
@@ -485,10 +485,15 @@ class ApiVisualEditor extends ApiBase {
                                                /* allow IP users*/ false
                                        );
 
-                                       if (
-                                               !( $targetUser && $targetUser->isLoggedIn() ) &&
-                                               !User::isIP( $targetUsername )
+                                       $targetUserExists = ( $targetUser && $targetUser->isLoggedIn() );
+                                       if ( $targetUserExists && $targetUser->isHidden() &&
+                                               !$user->isAllowed( 'hideuser' )
                                        ) {
+                                               // If the user exists, but is hidden, and the viewer cannot see hidden
+                                               // users, pretend like they don't exist at all. See T120883/T270453
+                                               $targetUserExists = false;
+                                       }
+                                       if ( !$targetUserExists && !User::isIP( $targetUsername ) ) {
                                                // User does not exist
                                                $notices[] = "<div class=\"mw-userpage-userdoesnotexist error\">\n" .
                                                        $this->msg( 'userpage-userdoesnotexist', wfEscapeWikiText( $targetUsername ) ) .

Huh, the patch at T270453#6723168 didn't work for me? Anyhow, I made another one:


And re-deployed to wmf.37. Also tracking again at T276237.

Reedy renamed this task from ApiVisualEditor leaks info about hidden users to CVE-2021-30153: ApiVisualEditor leaks info about hidden users.Apr 6 2021, 7:10 PM

Change 678025 had a related patch set uploaded (by Reedy; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@REL1_35] SECURITY: Act like users don't exist if hidden from viewer (VE edit notices)

https://gerrit.wikimedia.org/r/678025

Change 678026 had a related patch set uploaded (by Reedy; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@REL1_31] SECURITY: Act like users don't exist if hidden from viewer (VE edit notices)

https://gerrit.wikimedia.org/r/678026

Change 678028 had a related patch set uploaded (by Reedy; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@master] SECURITY: Act like users don't exist if hidden from viewer (VE edit notices)

https://gerrit.wikimedia.org/r/678028

Change 678025 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@REL1_35] SECURITY: Act like users don't exist if hidden from viewer (VE edit notices)

https://gerrit.wikimedia.org/r/678025

Change 678026 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@REL1_31] SECURITY: Act like users don't exist if hidden from viewer (VE edit notices)

https://gerrit.wikimedia.org/r/678026

Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".Apr 8 2021, 9:06 PM
Reedy changed the edit policy from "Custom Policy" to "All Users".
Restricted Application changed the visibility from "Public (No Login Required)" to "Custom Policy". · View Herald TranscriptApr 8 2021, 9:06 PM
Restricted Application changed the edit policy from "All Users" to "Custom Policy". · View Herald Transcript

Change 678028 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] SECURITY: Act like users don't exist if hidden from viewer (VE edit notices)

https://gerrit.wikimedia.org/r/678028

I am re-posting this comment from earlier (T270453#6706329) with the "private" information removed.

I will not be around, but the testing instructions are:

  1. Open the two links.
    1. Note that in old wikitext editor, they both say "<Username> is not registered on this wiki …" above the edit window.
  2. Switch to visual edtor
  3. Look at the edit notices popup (it should open automatically).
    1. Expected: Both pages say that "<Username> is not registered on this wiki …" near the end of the popup.
    2. Actual (currently): The second page instead says "This user is currently blocked …".

You could also compare these API outputs instead, the notices are at the beginning:

Removing PermanentlyPrivate because I removed the "private" information from the task.

Legoktm changed the visibility from "Custom Policy" to "Public (No Login Required)".Apr 9 2021, 11:27 PM
Legoktm changed the edit policy from "Custom Policy" to "All Users".

Change 678698 had a related patch set uploaded (by Jforrester; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@REL1_36] SECURITY: Act like users don't exist if hidden from viewer (VE edit notices)

https://gerrit.wikimedia.org/r/678698

Change 678698 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@REL1_36] SECURITY: Act like users don't exist if hidden from viewer (VE edit notices)

https://gerrit.wikimedia.org/r/678698