Cms: Permission escalation vector via group assignment

[brandonkelly]

Description

Related to #915. If a user has the “Assign user groups and permissions” permission, they may be able to grant themselves or other users new permissions that they don’t already have, by assigning their/another user’s account to a new group that has additional permissions.

Steps to reproduce

1. Create a user with the following permissions:
   
   
   
   • Edit users
   
   
   • Assign user groups and permissions
   
   
2. Create a user group with different permissions (e.g. “Access the site when the system is off”).
3. Log in as the user created in step 1, and go to My Account. The user will have permission to assign themselves to the user group created in step 2, thus granting themselves new permissions that they didn’t have to begin with.

Additional info

• Craft version: 2.6.2994 & 3.0.0-beta.30

[brandonkelly]

A few possible solutions to this:

1. Restrict users to only assign user groups that they are already a member of. This is similar to the approach we took to fixing #915, but it’s probably too heavy-handed in this case. because user groups are not only used for permission assignment; they may be used to organize users, etc.

2. Separate the “Assign user groups and permissions” permission into “Assign user groups” and “Assign user permissions”. The permission escalation vector would still exist for users with the “Assign user groups” permission, but at least it would be possible to _only_ give someone the ability to assign new user permissions, avoiding the user group bit.

3. Add per-user-group assignment permissions. Users would have to be explicitly granted permission to assign each individual user group. (E.g. “Assign ‘Authors’ user group”, “Assign ‘Editors’ user group”, etc.)

4. Only allow users to assign user groups that don’t have extra permissions. So if a user group has the “Access the site when the system is off” permission, only users that already have that permission will be allowed to assign that group to themselves/another user. Main issue with this is it will be confusing as to why certain groups are not available, when they’ve been filtered out.

There may be other options I’m not considering as well.

[tidy]

Appreciate your efforts on this Brandon thanks.

I think option 3 sounds good, especially if it could be applied at the user group level rather than having to set the assignable groups for each individual user.

Or if it was possible to combine options 2 & 3, so that you split the permissions into 'assign user groups' and 'assign user permissions', and then if you tick 'assign group permissions', you then tick the relevant groups you want to allow access to.

I don't know how complex these changes would be to implement though, so I would go with whatever is the simplest approach, because the key thing is just to be able to restrict user group assignment.

[brandonkelly]

Yeah the 3rd is probably the simplest one to implement/least confusing. Only downside is the extra admin hassle. Adding a way to chose which user groups can assign the current one to the user group settings could help a bit, but might be a little confusing UI-wise. We’ll see…

[tidy]

Many thanks for implementing this!

[smockensturm]

This still doesn't seem right.

Seems like if a user group can add new users, their perms should be cloned. i.e. "I'm adding a new user to my group"

Say I grant Users permissions to a user group with everything checked except _Assign User Groups_ nor _Assign User Permissions_, and a member of said user group creates a new user, of course that new creation gets relegated to limbo and has no permissions (not even to log in apparently)

If I do check only _Assign User Groups_ (and thus assigning to their _own_ group) the ensuing creation also has no permissions, no group.

So... we end up checking everything. Which essentially makes this group a super admin. With all that implies.

And from your docs...

Considering the amount of damage an admin can do, it’s strongly recommended that you be conservative with creating new admin accounts. Only do it if you trust that they know what they’re doing.

It seems like we're stuck making our client a super admin if they would like to add and administrate their own users.

Again... seems like if a user group can add new users, their perms should be cloned. i.e. "I'm adding a new user to my group"

[brandonkelly]

@smockensturm Thanks for the feedback. I agree for the most part: if a user already belongs to a group – _and they have the “Assign user groups” permission_ – they should be allowed to assign the same group to other users. That will be the case for the next release.

So... we end up checking everything. Which essentially makes this group a super admin. With all that implies.

To be clear, an Admin account has permissions that aren’t grantable to non-admins. Only admins can access the Settings area, for example.

[smockensturm]

To be clear, an Admin account has permissions that aren’t grantable to non-admins. Only admins can access the Settings area, for example.

Right. And that's what scares me.

[brandonkelly]

Why? I’m saying that even if you check every permission, it’s not actually the same as making someone an admin, so you shouldn’t have to worry about that.

[smockensturm]

OK. Right.

But they get the whole raft of the permissions panel anyway? In addition to assigning a member to their own group?

[brandonkelly]

There was a separate change I just made too (b0c5309c) – previously the Permissions tab was only shown for users with the “Assign user permissions” permission. Now it is also shown if the user has the “Assign user groups” permission. And within that tab, only the appropriate settings are shown, based on the user’s permissions. (So if a user can assign groups but not permissions, they will only see the User Groups section.)

So to summarize, going forward if you want a user to be allowed to create new users with the same group(s) they belong to, all you will need to do is give them the “Assign user groups” permission (but none of its nested permissions).

[smockensturm]

Got it. Solid. Thank you.

[smockensturm]

There was a separate change I just made too (b0c5309) – previously the Permissions tab was only shown for users with the “Assign user permissions” permission. Now it is also shown if the user has the “Assign user groups” permission. And within that tab, only the appropriate settings are shown, based on the user’s permissions. (So if a user can assign groups but not permissions, they will only see the User Groups section.)

So to summarize, going forward if you want a user to be allowed to create new users with the same group(s) they belong to, all you will need to do is give them the “Assign user groups” permission (but none of its nested permissions).

@brandonkelly You gonna' do this for Craft 3 as well? 'Cuz it seems right now as if this is not the case?

[brandonkelly]

Those changes were ported over in 8db7a33a910d67432ef5aec1556581ad7e7312f7 . If something’s not working as expected, please open a new issue, thanks!