HomePhabricator

SECURITY: Fix permissions checks in undo actions
1b10092da181Unpublished

Unpublished Commit · Learn More

  • Publishing Disabled: All publishing is disabled for this repository.
  • Not On Permanent Ref: This commit is not an ancestor of any permanent ref.

Description

SECURITY: Fix permissions checks in undo actions

Both traditional action=edit&undo= and the newer
action=mcrundo/action=mcrrestore endpoints suffer from a flaw that
allows for leaking entire private wikis by enumerating through revision
IDs when at least one page was publicly accessible via $wgWhitelistRead.
This is CVE-2021-44858.

05f06286f4def removed the restriction that user-supplied undo IDs belong
ot the same page, and was then copied into mcrundo. This check has been
restored by using RevisionLookup::getRevisionByTitle(), which returns
null if the revid is on a different page. This will break the workflow
outlined in T58184, but that could be restored in the future with better
access control checks.

action=mcrundo/action=restore suffer from an additional flaw that allows
for bypassing most editing restrictions. It makes no check on whether
user has the 'edit' permission or can even edit that page (page
protection, etc.). This is CVE-2021-44857.

This has been fixed by requiring the 'edit' permission to even invoke
the action (via Action::getRestriction()), as well as checking the
user's permissions to edit the specific page before saving.

The EditFilterMergedContent hook is also run against the revision before
it's saved so SpamBlacklist, AbuseFilter, etc. have a chance to review
the new page contents before saving.

Kudos to Dylsss for the identification and report.

Bug: T297322
Co-authored-by: Taavi Väänänen <hi@taavi.wtf>
Change-Id: I496093adfcf5a0e30774d452b650b751518370ce

Details

Provenance
LegoktmAuthored on Dec 8 2021, 11:31 PM
ReedyCommitted on Dec 15 2021, 5:29 PM
Parents
rMW60543fc48c41: Update git submodules
Branches
Unknown
Tags
Unknown
ChangeId
I496093adfcf5a0e30774d452b650b751518370ce

Event Timeline