[Mediawiki-l] UserRightsList 0.3 Announcement

[Jim Hu]

Since the security hole is most important, I'm working on that first:

On May 16, 2008, at 12:56 PM, DanTMan wrote:
<snip>
>>> And for 1.11 it is a security issue, because for $wgAddGroups and
>>> $wgRemoveGroups to work a user needs the userrights permission. On
>>> 1.11
>>> this will allow them to change restricted rights, however on 1.11 if
>>> your extension is enabled it will allow users with limited
>>> permission to
>>> change rights, to use that form to alter rights freely.
>>>
>>> It also looks like a security hazard in general. Rights are not  
>>> being
>>> checked in the correct places, namely the permissions saving. And it
>>> is
>>> relying on the checkboxes not showing up as the security feature.
>>> In other words, it's got a permissions injection vector. And in  
>>> simple
>>> terms... I could probably right now go to any site running the
>>> extension, do a little bit of HTML editing on the rights forms, and
>>> edit
>>> any permission I want on your site without needing the rights at  
>>> all.
>>> And yes, that does go as far as removing the rights for all your
>>> users,
>>> promoting myself to bot/sysop/bureaucrat and putting a flat ban on
>>> every
>>> user. You know, the kind of damage that'll take raw database editing
>>> to fix.
>>>
>>
>> Eek!  This sounds like the highest priority to fix.
>>
> Yup... Your setup of the special page is to. 'createuser' isn't a good
> permission to base on. If anything it would be 'userrights' but you  
> know
> what the issue with that is. For the most part that would likely be
> nothing. And you would do isAllowed, isAnon, isBlocked, readOnly, and
> checks on the global variables in the start of the execute function.
> http://svn.nadir-point.com/viewvc/mediawiki-extensions/trunk/WikiVid/SpecialLinkVideo.php?view=markup
> The way I do the isAllowed, isBlocked, and readOnly checks is  
> basically
> the standard afaik. I took the pieces from some of the more maintained
> extensions.


OK, so far... I changed the class :

	class UserRightsList extends UserrightsPage {

That lets me reuse what's already in the regular UserRights special  
page.

New save method:

	function save_rights(){
		global $wgRequest;
		$users = $this->findMyUsers();
		foreach ($users as $user){
			$u = User::newFromId($user['user_id']);
			if(is_object($u)) {
				$oldGroups = $u->getGroups();
				$newGroups = $wgRequest->getArray('user_'.$user['user_id']);
				if(is_null($wgRequest->getArray('user_'.$user['user_id'])))  
$newGroups = array();;
				// remove then add groups
				$removegroup = array_diff($oldGroups, $newGroups); 			
				$addgroup = array_diff($newGroups, $oldGroups);
				if (count(array_merge($removegroup, $addgroup)) == 0) continue;
				$this->saveUserGroups( $u->getName(), $removegroup, $addgroup);
			}
		}
		return true;
	}

This works in my testing on 1.12.  Since saveUserGroups does  
permission checking, I think this should take care of the hole.  What  
do you think?
Meanwhile, I'm working on the other stuff.

Jim
=====================================
Jim Hu
Associate Professor
Dept. of Biochemistry and Biophysics
2128 TAMU
Texas A&M Univ.
College Station, TX 77843-2128
979-862-4054