Skip to content

Commit

Permalink
SECURITY: Require 'read' right for most actions
Browse files Browse the repository at this point in the history
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
  • Loading branch information
legoktm authored and reedy committed Dec 15, 2021
1 parent 03d5b27 commit 5d5a3c0
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 0 deletions.
5 changes: 5 additions & 0 deletions includes/MediaWiki.php
Expand Up @@ -514,6 +514,11 @@ private function performAction( Article $article, Title $requestTitle ) {
$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();
Expand Down
10 changes: 10 additions & 0 deletions includes/actions/Action.php
Expand Up @@ -316,6 +316,16 @@ public function getRestriction() {
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
Expand Down
6 changes: 6 additions & 0 deletions includes/actions/ViewAction.php
Expand Up @@ -37,6 +37,12 @@ public function onView() {
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();

Expand Down

0 comments on commit 5d5a3c0

Please sign in to comment.