Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F34883084
0001-SECURITY-Require-read-right-for-most-actions.patch
Legoktm (Legoktm)
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Authored By
Legoktm
Dec 14 2021, 6:37 AM
2021-12-14 06:37:52 (UTC+0)
Size
2 KB
Referenced Files
None
Subscribers
None
0001-SECURITY-Require-read-right-for-most-actions.patch
View Options
From a0996b9865d6b70486e6a20e11b1181545d3881a Mon Sep 17 00:00:00 2001
From: Kunal Mehta <legoktm@debian.org>
Date: Mon, 13 Dec 2021 22:30:16 -0800
Subject: [PATCH] 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 217e232e7c..475454b94f 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -510,6 +510,11 @@ class MediaWiki {
$action = Action::factory( $actionName, $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 f4eadf6417..7f84e60ed6 100644
--- a/includes/actions/Action.php
+++ b/includes/actions/Action.php
@@ -316,6 +316,16 @@ abstract class Action implements MessageLocalizer {
return null;
}
+ /**
+ * Indicates whether this action requires read rights
+ * @since 1.38
+ * @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 9ec73b9650..aea7a5da8f 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();
--
2.33.1
File Metadata
Details
Attached
Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
9303432
Default Alt Text
0001-SECURITY-Require-read-right-for-most-actions.patch (2 KB)
Attached To
Mode
T297416: Restrict access to most actions on $wgWhitelistRead pages on private wikis
Attached
Detach File
Event Timeline
Log In to Comment