If two administrators try to delete the same user at about the same time, the anonymous user (uid 0) will get deleted from the {users} table, which has real bad consequences.

I'm attaching a simple patch to user.module to avoid this particular way of deleting uid 0.

In addition, below (because I couldn't figure out how to attach two patches ..) is a separate optional patch that will restore the uid 0 to the users table if it gets deleted via some other mechanism. This second patch is not elegant, and won't catch all conditions (because it'll fail if there's an existing entry in the users table with an empty name, for example if the uid 0 entry just gets uid changed to something else like I did while testing), but the serious nature of a missing uid 0, and difficult nature of debugging it, merits the small extra code I think.

Index: session.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/session.inc,v
retrieving revision 1.28
diff -r1.28 session.inc
23a24,27
>     if (!db_num_rows($result)) {
>       db_query("INSERT INTO users (uid, name, mail) VALUES ('0', '', '')");
>       $result = db_query("SELECT u.* FROM {users} u WHERE u.uid = 0");
>     }

I should add that I found this bug the hard way, on a production install. The likelyhood of two administrators both trying to delete the same user at about the same time doesn't seem to me all that unlikely, and I'd be surprised if this hasn't happened to others.

CommentFileSizeAuthor
#1 user.patch_8.txt678 byteskilles@www.drop.org
user.diff.txt352 bytesadixon

Comments

killes@www.drop.org’s picture

StatusFileSize
new678 bytes

Interesting, some people have had the problem in the past.

I have re-rolled the patch from the docroot.

Maybe we should take the opportunity to properly introduce the forms API...

adixon’s picture

Ah sorry, yes I should have included a bit more context in my patch.

Tell me the connection to the forms API?

killes@www.drop.org’s picture

There is no real connection to forms API, I just noticed that the form isnt fully converted.

chx’s picture

I was always for removing the user delete feature. Another good reason...

drumm’s picture

Status: Needs review » Fixed

I committed a slightly more verbose version (=== FALSE instead of !).

killes@www.drop.org’s picture

backported to 4.7.

Anonymous’s picture

Status: Fixed » Closed (fixed)