It is possible to delete uid=1 which shouldn't be the case. If this account is deleted a Drupal install could be foobared without some database hacking. It is possible to be logged in as uid=1 and delete yourself, as it is possible for another user with access to the user module to be able to delete uid=1 as well. This patch prevents this possible headache.

CommentFileSizeAuthor
#14 user.module_14.patch2.16 KBm3avrck
#5 user.module_11.patch1.14 KBm3avrck
#2 user.module_9.patch1.13 KBm3avrck
user.module_8.patch1.12 KBm3avrck

Comments

Cvbge’s picture

Cvbge commented

I think arg(1) can be a string, e.g. 1.5, in that case it's != 1 but when processed by printf's %d it gets changed to 1.

m3avrck’s picture

m3avrck commented
StatusFileSize
new user.module_9.patch1.13 KB

Good catch Cvbge, I've updated the patch accordingly. Should be locked down now :-)

m3avrck’s picture

m3avrck commented
Status: Needs review » Reviewed & tested by the community
dries’s picture

dries commented

Can't you compare with $account->uid?

m3avrck’s picture

m3avrck commented
StatusFileSize
new user.module_11.patch1.14 KB

Dries very good catch, didn't see that variable before ;-) Updated patch, very clean now.

Tobias Maier’s picture

Tobias Maier commented

shouldnt be there a message which gets shown in this case which tells the user "You cant delete this user because it is the first user (he has special and unique rights)"

sangamreddi’s picture

sangamreddi commented

It's good and fine but there should be a message "You can't delete this user" or others shoudn't be able to edit UID 1 account. If it's editable someone else can change the username and password of UID 1.

killes@www.drop.org’s picture

killes@www.drop.org commented

I actually disagree with the purpose of the patch. It might make sense to delete user No. 1 in some setups.

m3avrck’s picture

m3avrck commented
Status: Reviewed & tested by the community » Active

Yes true, if someone has user_access() to admin users then of course they could change the username/password. The reason for this patch is that I accidently deleted by uid=1 testing something and this really screwed up things. Had to do some database hacks so I could get back into the site.

I'm not sure about *not* being able to delete uid=1, although in some cases I could see why, overall, I don't think you should be able to delete this account. And maybe, as others have suggested, this account should *only* be editable if uid=1 is logged in, stating a message to anyone with admin users access that they can't edit this account, perhaps? If so I can roll a patch to do this. Like to hear some more thoughts as the uid=1 account should really be protected much more IMO.

m3avrck’s picture

m3avrck commented

To follow up, maybe only uid=1 can delete the uid=1 account, in which case, this would satisify killes hesitation. Still wouldn't solve my problem from before, *grin*, but at least this would lock down the account from accidently granting user admin access to malicious users, who could delete uid=1 or change username/password, etc.

Thoughts?

m3avrck’s picture

m3avrck commented

Also, to even further help this issue out, I accidentally blocked uid=1 because I had an error in my forms API code so I'm assuming the code thought I injected some code and blocked my account. Shouldn't a patch also prevent the blocking of uid=1 as well? Thoughts on that too?

jadwigo’s picture

jadwigo commented

Removing uid=1 while logged on als uid=1 is a very bad idea, ever accidentaly removed the user you are logged on as? You could make a workaround for all the errors you will get, but I think that it is not worth it in this case.

If you need to remove uid=1 you probably know your way around in a database and could do it manually.

drewish’s picture

drewish commented

I'm in the camp of, if you need to delete uid 1, do it by adjusting the database directly. If you really need to do it, you'll be able to figure out how.

m3avrck’s picture

m3avrck commented
Status: Active » Needs review
StatusFileSize
new user.module_14.patch2.16 KB

Ok here's a new patch with a few changes. uid=1 cannot be deleted at all and a message is shown to avoid any confusion with the user. Additionally, only uid=1 can edit the uid=1 profile, e.g., uid=1 must be logged in to change the password/username, hence preventing any malicious users with access to user profiles from locking uid=1 out. Appropriate message is displayed too in this case.

This patch really tightens up the uid=1 account, preventing possible accidents and malicious behavior.

m3avrck’s picture

m3avrck commented
Status: Needs review » Reviewed & tested by the community
m3avrck’s picture

m3avrck commented
Status: Reviewed & tested by the community » Closed (duplicate)

Moving this patch to main delete user problem patch: http://drupal.org/node/36716