From 55b60ae3ee08e76168a8adedc53f38a2d11b2c69 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Fri, 10 Dec 2021 22:27:08 -0800 Subject: [PATCH] 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. 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 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. Kudos to Dylsss for the identification and report. Bug: T297322 Change-Id: I496093adfcf5a0e30774d452b650b751518370ce --- includes/EditPage.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/includes/EditPage.php b/includes/EditPage.php index 1d19123b3f..32747a3ba9 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -1190,8 +1190,10 @@ class EditPage { $undo = $request->getInt( 'undo' ); if ( $undo > 0 && $undoafter > 0 ) { - $undorev = Revision::newFromId( $undo ); - $oldrev = Revision::newFromId( $undoafter ); + // The use of newFromTitle() is intentional, as allowing access to + // arbitrary revisions on arbitrary pages bypass partial visibility restrictions (T297322). + $undorev = Revision::newFromTitle( $this->mTitle, $undo ); + $oldrev = Revision::newFromTitle( $this->mTitle, $undoafter ); # Sanity check, make sure it's the right page, # the revisions exist and they were not deleted. -- 2.33.1