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/Specia…
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