From 0a5b1f7412a2e7571eec690cafe0ae9d26808fd6 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Thu, 7 Jul 2016 17:24:50 -0400 Subject: [PATCH] SECURITY: Move 'UserGetRights' call before application of Session::getAllowedUserRights() This prevents hook functions from accidentally adding rights that should be denied based on the session grants. If some extension really needs to be able to override session grants, add a new hook where the old call was, with documentation explicitly warning about the security implications. Bug: T139670 Change-Id: I6392cf4d7cc9d3ea96554b25bb5f8abb66e9031b --- includes/user/User.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/user/User.php b/includes/user/User.php index 4a92f65..19e273b 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -3253,6 +3253,7 @@ class User implements IDBAccessObject { public function getRights() { if ( is_null( $this->mRights ) ) { $this->mRights = self::getGroupPermissions( $this->getEffectiveGroups() ); + Hooks::run( 'UserGetRights', [ $this, &$this->mRights ] ); // Deny any rights denied by the user's session, unless this // endpoint has no sessions. @@ -3263,7 +3264,6 @@ 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 ) ); } -- 2.8.1