Security question Joomla User group change

Status
Not open for further replies.

OndrejH

Member
I want to let users join some of J! usegroups. However I am not sure whether following approach is safe or not.
The field element submits value to do a search within #___usergroup table and calc element does ajax insert or update of the #___user_usergroup_map:

// field to be searched for
$code = '{#___sometable_code_raw}';

// select J usegroup id
$db = JFactory::getDbo();
$query = $db->getQuery(true);

$query = "SELECT id FROM #___usergroups WHERE title = '$code'";

$db->setQuery($query);
$id = $db->loadResult();

// insert user to J usergroup map with group id corresponding to the code/group-title

if(isset($id) && $id > '12') { // IS THIS CRITICAL POINT?

// Get the connected user
$user = JFactory::getUser();

// Add the user in the group ID = x
JUserHelper::addUserToGroup($user->get('id'), $id );

// reload the user object so the user is connected with its new access rights
$session = JFactory::getSession();
$session->set('user', new JUser($user->get('id')));
} else {
echo "impossible to add user to the group";
}

Whatever group IDs below or equal to 12 should be restricted to access via this form.
Can you guide me in safe direction here?
Should I add protection on database side by triggers? I mean not allowing any NEW.group_id value inserted into #___user_usegroup_map to be less than X?
 
My temporary solution is a db trigger as with X set to restricted usergroup:

CREATE TRIGGER `prevent_check_trigger` BEFORE INSERT ON `jos_user_usergroup_map`
FOR EACH ROW if new.group_id = 'x' then
signal sqlstate '45000';
end if

Is this acceptable or is my entire logic completely wrong?
 
Reading your other explanations, I don't think the trigger is necessary since you are protecting usergroups < 12.
However, I would sanitize better the firsty query using the Joomla! quote method:

$query = "SELECT id FROM #___usergroups WHERE title = " . $db->quote($code);
 
Status
Not open for further replies.
We are in need of some funding.
More details.

Thank you.

Members online

Back
Top