Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

SECURITY: Make $wgBlockDisablesLogin also restrict logged in permissions

Browse source 
Does both Title and user related methods, so it catches things that only
call $wgUser->isAllowed( 'read' ), as well as giving a nicer error message
for things that use $title->userCan().

Otherwise, the user can still do stuff and read pages if they have an
ongoing session.

Issue reported by Multichill

Bug: T129738
Change-Id: Ic929a385fa81c27cbc6ac3a0862f51190d3ae993
This commit is contained in:
Brian Wolff 2016-06-29 10:45:25 -04:00 committed by Chad Horohoe
parent d2d12dc578
commit b675be2083
3 changed files with 47 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
includes/Block.php
View file

@ -969,28 +969,40 @@ class Block {
/**
* Get/set whether the Block prevents a given action
* @param string $action
* @param bool|null $x
* @return bool
*
* @param string $action Action to check
* @param bool|null $x Value for set, or null to just get value
* @return bool|null Null for unrecognized rights.
*/
public function prevents( $action, $x = null ) {
global $wgBlockDisablesLogin;
$res = null;
switch ( $action ) {
case 'edit':
# For now... <evil laugh>
return true;
$res = true;
break;
case 'createaccount':
return wfSetVar( $this->mCreateAccount, $x );
$res = wfSetVar( $this->mCreateAccount, $x );
break;
case 'sendemail':
return wfSetVar( $this->mBlockEmail, $x );
$res = wfSetVar( $this->mBlockEmail, $x );
break;
case 'editownusertalk':
return wfSetVar( $this->mDisableUsertalk, $x );
default:
return null;
$res = wfSetVar( $this->mDisableUsertalk, $x );
break;
case 'read':
$res = false;
break;
}
if ( !$res && $wgBlockDisablesLogin ) {
// If a block would disable login, then it should
// prevent any action that all users cannot do
$anon = new User;
$res = $anon->isAllowed( $action ) ? $res : true;
}
return $res;
}
/**

7
includes/Title.php
View file

@ -2271,13 +2271,17 @@ class Title implements LinkTarget {
* @return array List of errors
*/
private function checkUserBlock( $action, $user, $errors, $rigor, $short ) {
global $wgEmailConfirmToEdit, $wgBlockDisablesLogin;
// Account creation blocks handled at userlogin.
// Unblocking handled in SpecialUnblock
if ( $rigor === 'quick' || in_array( $action, [ 'createaccount', 'unblock' ] ) ) {
return $errors;
}
global $wgEmailConfirmToEdit;
// Optimize for a very common case
if ( $action === 'read' && !$wgBlockDisablesLogin ) {
return $errors;
}
if ( $wgEmailConfirmToEdit && !$user->isEmailConfirmed() ) {
$errors[] = [ 'confirmedittext' ];
@ -2434,6 +2438,7 @@ class Title implements LinkTarget {
$checks = [
'checkPermissionHooks',
'checkReadPermissions',
'checkUserBlock', // for wgBlockDisablesLogin
];
# Don't call checkSpecialsAndNSPermissions or checkCSSandJSPermissions
# here as it will lead to duplicate error messages. This is okay to do

16
includes/user/User.php
View file

@ -3147,6 +3147,22 @@ class User implements IDBAccessObject {
Hooks::run( 'UserGetRights', [ $this, &$this->mRights ] );
// Force reindexation of rights when a hook has unset one of them
$this->mRights = array_values( array_unique( $this->mRights ) );
// If block disables login, we should also remove any
// extra rights blocked users might have, in case the
// blocked user has a pre-existing session (T129738).
// This is checked here for cases where people only call
// $user->isAllowed(). It is also checked in Title::checkUserBlock()
// to give a better error message in the common case.
$config = RequestContext::getMain()->getConfig();
if (
$this->isLoggedIn() &&
$config->get( 'BlockDisablesLogin' ) &&
$this->isBlocked()
) {
$anon = new User;
$this->mRights = array_intersect( $this->mRights, $anon->getRights() );
}
}
return $this->mRights;
}