Security: The "administer users" permission exposes user/1

Crell - December 2, 2005 - 02:49
Project:Drupal
Version:7.x-dev
Component:user system
Category:feature request
Priority:normal
Assigned:Unassigned
Status:patch (code needs review)
Description

This very simple patch makes it so that users other than uid 1 cannot edit edit uid 1. You may want to have several administrators able to futz with user accounts, but the admin should be protected from a cranky user.

(This was suggested by a comment in another thread, but I felt it should be a separate patch.)

AttachmentSize
no_edit_admin.patch993 bytes

#1

Dries - December 2, 2005 - 15:43
Status:patch (code needs review)» won't fix

I don't think this is necessary.

#2

killes@www.drop.org - January 28, 2006 - 20:31

This is not only not neccessary it also doesn't make much sense. There might be multiple users which have all permissions on a site. A usefull way to prevent tampering with these accounts would be to only allow people who have some permission to edit all these accounts.

#3

ahoeben - February 8, 2006 - 11:42

The point is that if you let anyone who has user edit privileges edit user 1, they can change the password for user 1, log in as user 1 and gain unrestricted access to the site.

No single privilege should give any user the means to gain unrestricted access.

#4

Crell - February 9, 2006 - 00:50

Agreed.

@killes: This isn't as much intended to prevent an arbitrary user from taking over an arbitrary user as it is to ensure that UID 1 remains the site owner's "trump card". Yes, other people can self-escalate, but only UID 1 is UID 1, and only UID 1 can change UID 1.

#5

Crell - February 11, 2006 - 09:26
Status:won't fix» duplicate

I like hunmonk's patch in this thread better: http://drupal.org/node/46149 :-)

#6

gaele - November 19, 2007 - 16:30
Version:x.y.z» 6.x-dev
Category:feature request» bug report
Status:duplicate» active

http://drupal.org/node/46149 is about deletion of user #1. This issue is about editing.

When I first encountered this I was stupefied this is part of core. As has been said before the administer users permission allows anyone to change the password of user #1, effectively allowing "root access" to the site. In my opinion this renders the administer users permission pretty useless. Without any mention of this "feature" in the handbooks it's a security hole.

So, what to do? Block editing of user #1? Require supplying the current password when changing it? (Shouldn't we do this for all users?)

#7

momendo - November 19, 2007 - 17:50

Gaele, speaking from a security standpoint, that (supplying current pw) would be a good security feature. But I think that deserves its own bug report/feature request.

#8

chx - December 17, 2007 - 20:20
Version:6.x-dev» 7.x-dev
Category:bug report» feature request

#9

Rob Loach - December 17, 2007 - 20:46

Subscribing... Although I want the moderators to be able to edit users, I don't want to give them root access.

#10

Rob Loach - February 16, 2008 - 20:34
Title:Don't let anyone else edit uid 1» Security: The "administer users" permission exposes user/1
Assigned to:Crell» Anonymous
Status:active» patch (code needs review)

This is a very important issue, as anyone with the "administer users" permission can take super user access (user/1). The attached patch makes it so that users with "administer users" cannot edit user/1. Only user/1 can edit the super user account.

AttachmentSize
no_edit_admin-39636-2.patch899 bytes

#12

macgirvin - May 12, 2008 - 12:28

Patch works just fine - updated to chase HEAD. For many people, the issue is a critical, top priority security issue which shouldn't have remained in the queue for two days, let alone almost 2 1/2 years.

AttachmentSize
no-edit-admin-39636-3.patch986 bytes

#13

catch - May 12, 2008 - 14:17

I'd like to see less special-casing of user/1 rather than more.
What about something like "administer $role users" permissions (a bit like content types)? That'd allow for proper delegation of user administration, and protect 'admin users' alongside user/1.

#14

ahoeben - May 12, 2008 - 15:20

I think we need both; noone should have the ability to break into the super-user account by resetting its name/password; the super-user account is much too powerful for that. More granular user administration would be nice to have.

#15

catch - May 12, 2008 - 15:37

Plenty of sites have admin roles where they can do everything user/1 can (except running update.php which I think is hardcoded to user/1) - and the patch as it stands doesn't offer protection to those roles. Giving someone the php filter permission allows them to do anything they like in your database too - but I don't think we should special case that module to be only enabled by user/1 or similar.

#16

Crell - May 12, 2008 - 16:08

+1 to catch's suggestion in #13, as more fine-grained permissions are always a good thing. That still doesn't solve the problem of uid 1 still being editable, though, since uid 1 may still be in a role. If we ever introduce a "wheel" group for admins, then I'd give them all special treatment, not remove special treatment.

#17

catch - May 12, 2008 - 16:29

Well if you had an admin role, and put user/1 in that admin role - then you'd have to give users the "administer admin users" permission in order for user/1 to be editable going by #13. I'd be in favour of an admin role with user/1 being placed into it as part of the default install profile, or something similar, in fact it'd be quite nice to get rid of the whole idea of user/1 if there's a clean way to do this.

Various combinations of administer input formats, administer permissions and administer modules can let you escalate yourself to a situation where 'DELETE FROM {users};' is just a couple of googles away for any moderately competent malicious user. In fact that'd be easier than escalating yourself to user/1.

So if we're going to start special casing permissions then this'll need to be done in a lot more places, otherwise it's gives a false sense of security. And if we do that, then http://drupal.org/node/248598 needs some more consideration.

#18

macgirvin - May 13, 2008 - 01:25

A very common configuration is to allow 'staff' members of an organization permission to manage the daily operations of a site, whilst leaving the critical settings (which could mess things up completely) protected. In this regard, we mostly have the ability now to set things up in a manner that works and prevents somebody with 'some' privileges from blowing the site away completely. There's rarely an organizational need to delegate the administer filters or even the administer modules permissions once the site is set up and running smoothly.

However, administering users is one of the primary tasks we (in this case technical operations and IT personnel) wish to delegate to non-technical admin staff - and this doesn't work very well in the current model because it's basically all or nothing. I'm personally ambivalent about whether this is based on user/1; but it's fairly trivial to make this single level of delegation work and seems like the easiest solution short of a general permissions overhaul that would probably be overkill for the vast majority of sites.

#19

gaele - May 13, 2008 - 22:04

#20

David_Rothstein - May 20, 2008 - 06:01

This patch seems like it may go a bit too far. Not being able to edit the email address and password is one thing, but if I'm not mistaken, this would also prevent much more benign things (like signature and picture) from being edited too. Is that really the desired behavior?

In the long run, however, I agree that breaking "administer users" into more fine-grained permissions is the real solution here...

#21

tutube - June 6, 2008 - 06:44

subscribe.

What a lifesaver! now I can open 'administer users' permission to PR guys, it was a nightmare to add new hired to the system every week by myself.

 
 

Drupal is a registered trademark of Dries Buytaert.