diff -Nru mediawiki-1.35.3/debian/changelog mediawiki-1.35.3/debian/changelog --- mediawiki-1.35.3/debian/changelog 2021-08-20 23:56:23.000000000 -0700 +++ mediawiki-1.35.3/debian/changelog 2021-12-19 12:44:13.000000000 -0800 @@ -1,3 +1,13 @@ +mediawiki (1:1.35.3-1ubuntu0.1) impish-security; urgency=high + + * SECURITY UPDATE: Information leak and editing permissions bypass + through various actions (LP: #1955352) + - CVE-2021-44857 + - CVE-2021-44858 + - CVE-2021-45038 + + -- Kunal Mehta Sun, 19 Dec 2021 12:44:13 -0800 + mediawiki (1:1.35.3-1) unstable; urgency=medium [ Kunal Mehta ] diff -Nru mediawiki-1.35.3/debian/patches/0002-SECURITY-Fix-permissions-checks-in-undo-actions.patch mediawiki-1.35.3/debian/patches/0002-SECURITY-Fix-permissions-checks-in-undo-actions.patch --- mediawiki-1.35.3/debian/patches/0002-SECURITY-Fix-permissions-checks-in-undo-actions.patch 1969-12-31 16:00:00.000000000 -0800 +++ mediawiki-1.35.3/debian/patches/0002-SECURITY-Fix-permissions-checks-in-undo-actions.patch 2021-12-19 12:39:20.000000000 -0800 @@ -0,0 +1,134 @@ +From: Kunal Mehta +Date: Wed, 8 Dec 2021 15:31:43 -0800 +Subject: SECURITY: Fix permissions checks in undo actions +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +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 +Change-Id: I496093adfcf5a0e30774d452b650b751518370ce +--- + includes/EditPage.php | 6 +++-- + includes/actions/McrUndoAction.php | 47 ++++++++++++++++++++++++++++++++++++-- + 2 files changed, 49 insertions(+), 4 deletions(-) + +diff --git a/includes/EditPage.php b/includes/EditPage.php +index 76bd643..f0206dc 100644 +--- a/includes/EditPage.php ++++ b/includes/EditPage.php +@@ -1271,8 +1271,10 @@ class EditPage implements IEditObject { + $undo = $request->getInt( 'undo' ); + + if ( $undo > 0 && $undoafter > 0 ) { +- $undorev = $this->revisionStore->getRevisionById( $undo ); +- $oldrev = $this->revisionStore->getRevisionById( $undoafter ); ++ // The use of getRevisionByTitle() is intentional, as allowing access to ++ // arbitrary revisions on arbitrary pages bypass partial visibility restrictions (T297322). ++ $undorev = $this->revisionStore->getRevisionByTitle( $this->mTitle, $undo ); ++ $oldrev = $this->revisionStore->getRevisionByTitle( $this->mTitle, $undoafter ); + $undoMsg = null; + + # Sanity check, make sure it's the right page, +diff --git a/includes/actions/McrUndoAction.php b/includes/actions/McrUndoAction.php +index 7685865..5b8904d 100644 +--- a/includes/actions/McrUndoAction.php ++++ b/includes/actions/McrUndoAction.php +@@ -42,6 +42,11 @@ class McrUndoAction extends FormAction { + return ''; + } + ++ public function getRestriction() { ++ // Require 'edit' permission to even see this action (T297322) ++ return 'edit'; ++ } ++ + public function show() { + // Send a cookie so anons get talk message notifications + // (copied from SubmitAction) +@@ -114,8 +119,10 @@ class McrUndoAction extends FormAction { + + $revisionLookup = MediaWikiServices::getInstance()->getRevisionLookup(); + +- $undoRev = $revisionLookup->getRevisionById( $this->undo ); +- $oldRev = $revisionLookup->getRevisionById( $this->undoafter ); ++ // We use getRevisionByTitle to verify the revisions belong to this page (T297322) ++ $title = $this->getTitle(); ++ $undoRev = $revisionLookup->getRevisionByTitle( $title, $this->undo ); ++ $oldRev = $revisionLookup->getRevisionByTitle( $title, $this->undoafter ); + + if ( $undoRev === null || $oldRev === null || + $undoRev->isDeleted( RevisionRecord::DELETED_TEXT ) || +@@ -321,8 +328,44 @@ class McrUndoAction extends FormAction { + return Status::newFatal( 'mcrundo-changed' ); + } + ++ $permissionManager = MediaWikiServices::getInstance()->getPermissionManager(); ++ $errors = $permissionManager->getPermissionErrors( ++ 'edit', $this->context->getUser(), $this->getTitle() ++ ); ++ if ( count( $errors ) ) { ++ throw new PermissionsError( 'edit', $errors ); ++ } ++ + $newRev = $this->getNewRevision(); + if ( !$newRev->hasSameContent( $curRev ) ) { ++ $hookRunner = Hooks::runner(); ++ foreach ( $newRev->getSlotRoles() as $slotRole ) { ++ $slot = $newRev->getSlot( $slotRole, RevisionRecord::RAW ); ++ ++ $status = new Status(); ++ $hookResult = $hookRunner->onEditFilterMergedContent( ++ $this->getContext(), ++ $slot->getContent(), ++ $status, ++ trim( $this->getRequest()->getVal( 'wpSummary' ) ), ++ $this->getUser(), ++ false ++ ); ++ ++ if ( !$hookResult ) { ++ if ( $status->isGood() ) { ++ $status->error( 'hookaborted' ); ++ } ++ ++ return $status; ++ } elseif ( !$status->isOK() ) { ++ if ( !$status->getErrors() ) { ++ $status->error( 'hookaborted' ); ++ } ++ return $status; ++ } ++ } ++ + $revisionStore = MediaWikiServices::getInstance()->getRevisionStore(); + + // Copy new slots into the PageUpdater, and remove any removed slots. diff -Nru mediawiki-1.35.3/debian/patches/0003-RollbackAction-fix-missing-pagetitle.patch mediawiki-1.35.3/debian/patches/0003-RollbackAction-fix-missing-pagetitle.patch --- mediawiki-1.35.3/debian/patches/0003-RollbackAction-fix-missing-pagetitle.patch 1969-12-31 16:00:00.000000000 -0800 +++ mediawiki-1.35.3/debian/patches/0003-RollbackAction-fix-missing-pagetitle.patch 2021-12-19 12:39:20.000000000 -0800 @@ -0,0 +1,27 @@ +From: Derk-Jan Hartman +Date: Sat, 4 Dec 2021 23:09:03 +0100 +Subject: RollbackAction: fix missing pagetitle + +This reimplemented the FormAction::show() but was missing the call to +setHeaders(), which is responsible for setting the page title. + +Bug: T225888 +Change-Id: Ie3ec5ce08beb6d2207abc30bd9b48c89a95bfb2a +(cherry picked from commit dda5355c0ee804c94ff371a8a16c4a2a8e4436bd) +--- + includes/actions/RollbackAction.php | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/includes/actions/RollbackAction.php b/includes/actions/RollbackAction.php +index 37b98d0..8fbfd98 100644 +--- a/includes/actions/RollbackAction.php ++++ b/includes/actions/RollbackAction.php +@@ -82,6 +82,8 @@ class RollbackAction extends FormAction { + * @throws ThrottledError + */ + public function show() { ++ $this->setHeaders(); ++ + if ( $this->getUser()->getOption( 'showrollbackconfirmation' ) == false || + $this->getRequest()->wasPosted() ) { + $this->handleRollbackRequest(); diff -Nru mediawiki-1.35.3/debian/patches/0004-SECURITY-Fix-permissions-check-in-action-rollback-CV.patch mediawiki-1.35.3/debian/patches/0004-SECURITY-Fix-permissions-check-in-action-rollback-CV.patch --- mediawiki-1.35.3/debian/patches/0004-SECURITY-Fix-permissions-check-in-action-rollback-CV.patch 1969-12-31 16:00:00.000000000 -0800 +++ mediawiki-1.35.3/debian/patches/0004-SECURITY-Fix-permissions-check-in-action-rollback-CV.patch 2021-12-19 12:39:20.000000000 -0800 @@ -0,0 +1,43 @@ +From: Kunal Mehta +Date: Tue, 14 Dec 2021 18:30:17 -0800 +Subject: SECURITY: Fix permissions check in action=rollback (CVE-2021-45038) + +Because RollbackAction (as of 0a8403271109) overrided +FormAction::show(), it was no longer checking that the user had the +"rollback" userright. This restores that check, so people without the +"rollback" right will not be able to even get to the rollback form. + +Then escape the user-supplied "from" parameter so it can't be used to +reveal the contents of other pages through transclusion, e.g. +"{{:Secret}}". wfEscapeWikiText() is also good practice for usernames in +general, as they can contain markup like bullets or single quotes that +affect output. + +Bug: T297574 +Change-Id: I7424f67f1217482b977f9617f0275c41fb94b60f +--- + includes/actions/RollbackAction.php | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/includes/actions/RollbackAction.php b/includes/actions/RollbackAction.php +index 8fbfd98..4db3c14 100644 +--- a/includes/actions/RollbackAction.php ++++ b/includes/actions/RollbackAction.php +@@ -83,6 +83,8 @@ class RollbackAction extends FormAction { + */ + public function show() { + $this->setHeaders(); ++ // This will throw exceptions if there's a problem ++ $this->checkCanExecute( $this->getUser() ); + + if ( $this->getUser()->getOption( 'showrollbackconfirmation' ) == false || + $this->getRequest()->wasPosted() ) { +@@ -111,7 +113,7 @@ class RollbackAction extends FormAction { + if ( $from !== $userText ) { + throw new ErrorPageError( 'rollbackfailed', 'alreadyrolled', [ + $this->getTitle()->getPrefixedText(), +- $from, ++ wfEscapeWikiText( $from ), + $userText + ] ); + } diff -Nru mediawiki-1.35.3/debian/patches/0005-SECURITY-Require-read-right-for-most-actions.patch mediawiki-1.35.3/debian/patches/0005-SECURITY-Require-read-right-for-most-actions.patch --- mediawiki-1.35.3/debian/patches/0005-SECURITY-Require-read-right-for-most-actions.patch 1969-12-31 16:00:00.000000000 -0800 +++ mediawiki-1.35.3/debian/patches/0005-SECURITY-Require-read-right-for-most-actions.patch 2021-12-19 12:39:20.000000000 -0800 @@ -0,0 +1,76 @@ +From: Kunal Mehta +Date: Mon, 13 Dec 2021 22:30:16 -0800 +Subject: SECURITY: Require 'read' right for most actions + +As a security hardening measure to limit exposure on private wikis from +actions on $wgWhitelistRead pages, require an explicit 'read' right on +actions by default. Currently only ViewAction disables this check since +it does its own permissions checking. + +This is somewhat duplicative of the permissions check in +MediaWiki::performRequest() but we'll call it defense in depth. It also +matches similar logic in the Action and REST APIs. + +Bug: T34716 +Bug: T297416 +Change-Id: Ib2a6c08dc50c69c3ed6e5708ab72441a90fcd3e1 +--- + includes/MediaWiki.php | 5 +++++ + includes/actions/Action.php | 10 ++++++++++ + includes/actions/ViewAction.php | 6 ++++++ + 3 files changed, 21 insertions(+) + +diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php +index da0c231..6158690 100644 +--- a/includes/MediaWiki.php ++++ b/includes/MediaWiki.php +@@ -505,6 +505,11 @@ class MediaWiki { + $action = Action::factory( $act, $article, $this->context ); + + if ( $action instanceof Action ) { ++ // Check read permissions ++ if ( $action->needsReadRights() && !$user->isAllowed( 'read' ) ) { ++ throw new PermissionsError( 'read' ); ++ } ++ + // Narrow DB query expectations for this HTTP request + $trxLimits = $this->config->get( 'TrxProfilerLimits' ); + $trxProfiler = Profiler::instance()->getTransactionProfiler(); +diff --git a/includes/actions/Action.php b/includes/actions/Action.php +index bf98382..bce11f3 100644 +--- a/includes/actions/Action.php ++++ b/includes/actions/Action.php +@@ -398,6 +398,16 @@ abstract class Action implements MessageLocalizer { + return null; + } + ++ /** ++ * Indicates whether this action requires read rights ++ * @since 1.35.5 ++ * @stable to override ++ * @return bool ++ */ ++ public function needsReadRights() { ++ return true; ++ } ++ + /** + * Checks if the given user (identified by an object) can perform this action. Can be + * overridden by sub-classes with more complicated permissions schemes. Failures here +diff --git a/includes/actions/ViewAction.php b/includes/actions/ViewAction.php +index 957e647..f9237c0 100644 +--- a/includes/actions/ViewAction.php ++++ b/includes/actions/ViewAction.php +@@ -37,6 +37,12 @@ class ViewAction extends FormlessAction { + return null; + } + ++ public function needsReadRights() { ++ // Pages in $wgWhitelistRead can be viewed without having the 'read' ++ // right. We rely on Article::view() to properly check read access. ++ return false; ++ } ++ + public function show() { + $config = $this->context->getConfig(); + diff -Nru mediawiki-1.35.3/debian/patches/series mediawiki-1.35.3/debian/patches/series --- mediawiki-1.35.3/debian/patches/series 2021-05-06 11:57:25.000000000 -0700 +++ mediawiki-1.35.3/debian/patches/series 2021-12-19 12:39:20.000000000 -0800 @@ -1 +1,5 @@ 0001-Have-Scribunto-use-packaged-lua5.1-rather-than-bundl.patch +0002-SECURITY-Fix-permissions-checks-in-undo-actions.patch +0003-RollbackAction-fix-missing-pagetitle.patch +0004-SECURITY-Fix-permissions-check-in-action-rollback-CV.patch +0005-SECURITY-Require-read-right-for-most-actions.patch