[patch] Add optional username regexp to registration checks
|Reported by:||manski||Owned by:||hasienda|
|Severity:||normal||Keywords:||security precaution user register name check regexp|
|Cc:||thijs, felix.schwarz@…, rjollos, olemis, AdrianFritz, mmitar@…||Trac Release:||0.11|
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).
Change History (48)
Changed 6 years ago by manski
comment:18 Changed 5 years ago by AdrianFritz
- Summary changed from Registration module is a potential security hole to [Patch & WorkArround] Registration module is a potential security hole
comment:25 Changed 4 years ago by hasienda
- Keywords user register name check regexp added; delete removed
- Owner changed from pacopablo to hasienda
- Priority changed from highest to normal
- Severity changed from critical to normal
- Status changed from new to assigned
- Summary changed from [Patch & WorkArround] Registration module is a potential security hole to [patch] Add Registration module is a potential security hole
- Type changed from defect to enhancement
comment:26 Changed 4 years ago by hasienda
- Summary changed from [patch] Add Registration module is a potential security hole to [patch] Add optional username regexp to registration checks