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/Special... 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