Page MenuHomePhabricator
Create Task
Maniphest T307278

CVE-2022-41766: On action=rollback the message "alreadyrolled" can leak revision deleted user name
Closed, ResolvedPublicSecurity
Actions

Assigned To
mmartorana
Authored By
Umherirrender
Apr 30 2022, 7:28 PM
Tags
Referenced Files
F35538751: Screenshot 2022-09-28 at 20-53-34 Action complete - Trunk Test.png
Sep 29 2022, 12:54 AM
F35538605: T307278-master.patch
Sep 28 2022, 10:52 PM
F35538604: T307278-REL1_38.patch
Sep 28 2022, 10:52 PM
F35485641: 02-T307278.patch
Aug 23 2022, 5:06 PM
F35474504: T307278.patch
Aug 18 2022, 3:56 PM
Subscribers
Aklapper
gerritbot
Krinkle
Legoktm
Reedy
sbassett
Umherirrender

Description

  • Create a page with an Rollbacker (but Rollbacker is not Admin)
  • Move the page with MalicousUser (to create a null revision)
  • Revision delete/suppress the user of the null revision
  • Use the rollback from action=history with Rollbacker and the "alreadyrolled" message shows "last edit of MalicousUser" and leak the revdeled user name of the null revision

Good points: The link to action=rollback contains a empty from and does not leak the name.
Bad points: The rollback link is useless, but provided (thats T6433 for move, but the link is also provided for other null revisions like import or protection) and should not be part of this task

Possible a regression from the refactor in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/675236

Details

Risk Rating
Low
SubjectRepoBranchLines +/-
SECURITY: Hide suppressed users from rollback page error messagesmediawiki/coremaster+8 -1
SECURITY: Hide suppressed users from rollback page error messagesmediawiki/coreREL1_38+7 -1
SECURITY: Hide suppressed users from rollback page error messagesmediawiki/coreREL1_39+8 -1
SECURITY: Hide suppressed users from rollback page error messagesmediawiki/coreREL1_37+7 -1
Customize query in gerrit

Related Objects
Search...

Event Timeline

Umherirrender created this task.Apr 30 2022, 7:28 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 30 2022, 7:28 PM
Umherirrender added projects: MediaWiki-Revision-deletion, MediaWiki-User-Interface (actions).Apr 30 2022, 7:29 PM
mmartorana claimed this task.May 4 2022, 7:52 PM
mmartorana added a project: Vuln-Infoleak.
mmartorana changed Risk Rating from N/A to Low.

Hi,

I am working on a patch to fix this issue.

mmartorana changed the task status from Open to In Progress.May 9 2022, 4:11 PM
mmartorana triaged this task as Low priority.
sbassett moved this task from Incoming to In Progress on the Security-Team board.May 16 2022, 4:26 PM
mmartorana added a comment.Aug 18 2022, 3:56 PM

I am proposing this patch to fix this issue:

T307278.patch2 KBDownload

sbassett added a project: Patch-For-Review.Aug 18 2022, 7:19 PM
sbassett subscribed.Aug 19 2022, 8:45 PM
In T307278#8166003, @mmartorana wrote:

I am proposing this patch to fix this issue:

T307278.patch2 KBDownload

+1 just based upon the logic alone. I think we'd want to test this a bit more to see if $userFactory->newFromName( $this->context->msg( 'rev-deleted-user' )->escaped() ); has any unintended consequences. At the very least, MediaWiki doesn't seem to have a problem creating a User object that should work fine:

shell.php
>>> $userFactory = \MediaWiki\MediaWikiServices::getInstance()->getUserFactory();
=> MediaWiki\User\UserFactory {#3362}

>>> $revUser = $userFactory->newFromName( RequestContext::getMain()->msg( 'rev-deleted-user' )->escaped() );
=> User {#5855
     +mId: null,
     +mName: "(Username or IP removed)",
     +mActorId: null,
     +mRealName: null,
     +mEmail: null,
     +mTouched: null,
     +mEmailAuthenticated: null,
     +mFrom: "name",
   }
sbassett moved this task from In Progress to Security Patch To Deploy on the Security-Team board.Aug 22 2022, 9:35 PM
Legoktm subscribed.Aug 23 2022, 3:06 PM
In T307278#8166003, @mmartorana wrote:

I am proposing this patch to fix this issue:

T307278.patch2 KBDownload

It seems weird to squeeze a message into a User object like that. If the issue is the alreadyrolled message leaking the username, we should have a separate message like alreadyrolled-deleted that doesn't output the username, and use that when necessary. Probably that needs to be done in RollbackPage though.

sbassett added a parent task: T311776: Tracking bug for MediaWiki 1.35.8/1.37.5/1.38.3.Aug 23 2022, 3:39 PM
sbassett added a subscriber: Krinkle.Aug 23 2022, 4:06 PM

The above patch was deployed to wmf.25 and wmf.26 this morning. The patch applied fine and didn't seem to cause any issues within the logs, but likely wouldn't anyways, given the scope of the patch and likely affected users. That being said - if anyone has user-suppress rights on a project and would like to further test, that would be great. This is now being tracked as a deployed patch at T276237 and for the next security release at T311776.

Thanks to @Legoktm and @Krinkle for real-time help with the patch prior to deployment. There are a few cleanup items (spacing, message function, patch subject) that need to be cleaned up prior to release - I'll try to get a refactored patch posted soon. And then as @Legoktm notes above, it might make sense to create a more optimal follow-up patch with a new message, etc.

sbassett moved this task from Security Patch To Deploy to Watching on the Security-Team board.Aug 23 2022, 4:06 PM
sbassett removed a project: Patch-For-Review.
sbassett added a comment.Aug 23 2022, 5:06 PM

Refactored patch:

02-T307278.patch1 KBDownload

Changelog:

  1. Fixed whitespace issues
  2. Condensed comments, changed to // style
  3. Use plain() instead of escaped() for rev-deleted-user message
  4. Added SECURITY: to patch subject line, removed bug id
  5. Slightly changed patch subject text
  6. Added Bug: to commit message

Should now pass gerrit at the very least.

Reedy added a project: MW-1.39-release.Sep 25 2022, 6:45 PM
Reedy added projects: MW-1.35-release, MW-1.37-release, MW-1.38-release.Sep 25 2022, 6:56 PM
Reedy mentioned this in T311776: Tracking bug for MediaWiki 1.35.8/1.37.5/1.38.3.Sep 25 2022, 7:05 PM
Reedy mentioned this in T311777: Obtain CVEs for 1.35.8/1.37.5/1.38.3 security releases.Sep 25 2022, 7:11 PM
Reedy subscribed.Sep 25 2022, 7:21 PM

Patch should be good for REL1_39 and master, but for REL1_35, REL1_37 and REL1_38, an adjusted patch will be needed due to lack of getAuthority().

Legoktm added a comment.EditedSep 28 2022, 10:52 PM

master / REL1_39 (same, just added Change-Id)

T307278-master.patch1 KBDownload

REL1_38 / REL1_37 (swapped getAuthority() for $user->isAllowedAny(...))

T307278-REL1_38.patch1 KBDownload

for REL1_35 I need to test, but I *think* it's not present. I think it was introduced in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/675236 which is 1.37+

Legoktm added a comment.Sep 29 2022, 12:54 AM

Confirmed that REL1_35 (with no patch) is fine.

Screenshot 2022-09-28 at 20-53-34 Action complete - Trunk Test.png (429×1 px, 85 KB)

Reedy renamed this task from On action=rollback the message "alreadyrolled" can leak revision deleted user name to CVE-2022-41766: On action=rollback the message "alreadyrolled" can leak revision deleted user name.Sep 29 2022, 5:29 PM
Reedy added a subscriber: gerritbot.
Reedy closed this task as Resolved.Sep 29 2022, 5:38 PM
gerritbot added a comment.Sep 29 2022, 6:58 PM

Change 836896 had a related patch set uploaded (by Reedy; author: Mmartorana):

[mediawiki/core@REL1_37] SECURITY: Hide suppressed users from rollback page error messages

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

gerritbot added a project: Patch-For-Review.Sep 29 2022, 6:58 PM

Change 836900 had a related patch set uploaded (by Reedy; author: Mmartorana):

[mediawiki/core@REL1_38] SECURITY: Hide suppressed users from rollback page error messages

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

gerritbot added a comment.Sep 29 2022, 6:59 PM

Change 836905 had a related patch set uploaded (by Reedy; author: Mmartorana):

[mediawiki/core@REL1_39] SECURITY: Hide suppressed users from rollback page error messages

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

gerritbot added a comment.Sep 29 2022, 7:00 PM

Change 836910 had a related patch set uploaded (by Reedy; author: Mmartorana):

[mediawiki/core@master] SECURITY: Hide suppressed users from rollback page error messages

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

gerritbot added a comment.Sep 29 2022, 7:28 PM

Change 836896 merged by Reedy:

[mediawiki/core@REL1_37] SECURITY: Hide suppressed users from rollback page error messages

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

gerritbot added a comment.Sep 29 2022, 7:35 PM

Change 836905 merged by jenkins-bot:

[mediawiki/core@REL1_39] SECURITY: Hide suppressed users from rollback page error messages

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

gerritbot added a comment.Sep 29 2022, 7:36 PM

Change 836900 merged by Reedy:

[mediawiki/core@REL1_38] SECURITY: Hide suppressed users from rollback page error messages

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

gerritbot added a comment.Sep 29 2022, 7:58 PM

Change 836910 merged by jenkins-bot:

[mediawiki/core@master] SECURITY: Hide suppressed users from rollback page error messages

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

Jdforrester-WMF added a project: MW-1.40-notes (1.40.0-wmf.4; 2022-10-03).Sep 29 2022, 8:05 PM
Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".Nov 1 2022, 6:07 PM
Reedy changed the edit policy from "Custom Policy" to "All Users".
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL