Today I stumbled upon a big security hole or at least a way to almost render a Trac environment useless in the AccountManagerPlugin. The problem is that the check for existing users when creating a new one in the Registration module is flawed.
The check doesn't check for the usernames "anonymous" and "authenticated". So it's easily possible to register as "anonymous" or "authenticated". While you can't login as "anonymous" you can do so as "authenticated". While this is already a bad thing it gets worse. Since there is the possibility to delete my account one could register as "authenticated" and then delete the account again. Doing so he/she would thereby remove the group "authenticated" from the permessions store. This way all registered user would be demoted to "anonymous" (unless they were given some permissions directly).
Also it's possible to register as an already existing permission group (like "anonymous" and "authenticated") and inherit all of its permissions. Say I've created a group called "developers", added the permission "TICKET_ADMIN" to this group and assigned all the members of my development team to this group. Now they all have the permission "TICKET_ADMIN". However, a user could register with the name "developers" and also gain the permission "TICKET_ADMIN". And that's very bad! (Fortunately the user has to know the name of the permission group which he/she can't find out so easily. But he/she could simply try some common names.)
I've attached a patch to this ticket the fixes this problem. It also !add the possibility for the !admin to define a regular expression for valid usernames. Moreover the patch prohibits usernames containing characters that are harmful for some password stores (this fixes #4682 and #2630).