2009-03-20
usi
When using split authentication and merging the user groups together it causes issues if there are duplicates coming from the same auth provider. When the list of groups gets to inc/auth.php to check the ACL in auth_checkacl() it fails to perform the check properly. This could allow someone to improperly have higher or lower permissions than what they are supposed to.
The bug is generated from the following code within getUserData() within split.class.php:
if ($this->mergeGroups and !empty($login_user['grps'])) {
$login_user['grps'] = array_unique(array_merge((array) $login_user['grps'], (array) $groups_user['grps']));
} else {
$login_user['grps'] = $groups_user['grps'];
}
The following is an example of using AD and plain authentication:
$login_user['grps'] = array('Domain_Users','Users','Users','Users');
$groups_user['grps'] = array('admin');
Then the above code will return array(0 => 'Domain_Users', 1 => 'Users', 4 => 'admin');
Notice the gap (missing 2 and 3) so when the below code iterates it actually append two more '@' to the end of the array.
This is from line 428 of inc/auth.php:
for($i=0; $i<$cnt; $i++){
$groups[$i] = '@'.auth_nameencode($groups[$i]);
}
One possible solution is to correct the above to the following:
foreach($groups as $i=>$group){
$groups[$i] = '@'.auth_nameencode($group);
}