Modify

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#11697 closed defect (fixed)

Remote password change possible upon register of new user w/ already existing username

Reported by: erinn_tor Owned by: Steffen Hoffmann
Priority: normal Component: AccountManagerPlugin
Severity: critical Keywords:
Cc: Trac Release: 1.0

Description

Hi,

Someone on our bug tracker recently discovered that it's possible to change users' passwords by re-registering the pre-existing username. This is with AccountManagerPlugin 0.4.3.

https://trac.torproject.org/projects/tor/ticket/11545

I don't think this should be possible with any combination of trac.ini options, but just in case, here are the ones we were using before disabling the registration of new users entirely:

acct_mgr.admin.accountmanageradminpanel = enabled
acct_mgr.guard.accountguard = enabled
acct_mgr.register.registrationmodule = enabled
acct_mgr.web_ui.accountmodule = enabled

Basically, they were able to reset anyone's account including admins, so I consider this a critical security flaw. Please let me know if there is any more information I can provide.

Attachments (0)

Change History (5)

comment:1 Changed 10 years ago by Steffen Hoffmann

Thanks for the report. But I'm not convinced right now, because such concerns drove the re-design of the current registration process. Please check your configuration against the following possible errors, because I suspect this to be a local configuration issue.

If you enabled acct_mgr.register.BasicCheck AND added it to register_check in the [account-manager] section of your trac.ini, this would not happen. Default value for register_check would be safe to. Cite from basic check doc-string:

A collection of basic checks.

This includes checking for

  • emptiness (no user input for username and/or password)
  • some blacklisted username characters
  • upper-cased usernames (reserved for Trac permission actions)
  • some reserved usernames
  • a username duplicate in configured password stores

This check is bypassed for requests regarding user's own preferences.

I've just verified that the check against existing session IDs is even case-insensitive, so it should be ok.

Btw, you should have upgraded to acct_mgr-0.4.4 by now, if you really cared for least attack profile of your Trac application.

Last edited 10 years ago by Steffen Hoffmann (previous) (diff)

comment:2 in reply to:  description Changed 10 years ago by Steffen Hoffmann

Replying to erinn_tor:

Someone on our bug tracker recently discovered that it's possible to change users' passwords by re-registering the pre-existing username. This is with AccountManagerPlugin 0.4.3.

https://trac.torproject.org/projects/tor/ticket/11545

Just verified, that this is impossible in the standard configuration. Double-checking your configuration according to aforementioned comment about possible miss of BasicCheck one way or the other will likely bring you on track.

comment:3 Changed 10 years ago by erinn_tor

Thanks a lot for the configuration tips! I've updated to 0.4.4 and implemented the changes you suggested.

What is the standard configuration? Is there any scenario in which the outcome I listed would be desirable? Misconfigured software is a problem for which users are responsible, but I still don't think there should be any situation under which users should be able to take control of a trac instance in this way.

(BTW 0.4.3 is still listed as the stable version on the AccountManagerPlugin page.)

And thanks again, also for your quick reply.

comment:4 in reply to:  3 Changed 10 years ago by Steffen Hoffmann

Resolution: fixed
Status: newclosed

Replying to erinn_tor:

Thanks a lot for the configuration tips! I've updated to 0.4.4 and implemented the changes you suggested.

Fine. Glad you found it working for you.

What is the standard configuration?

Look at you TracIni to see default values for any/you Trac and Trac Plugin application. As noted before, the default is safe, as it always included BasicCheck, and the name implies rather important checks too, right? For the sake of plugin flexibility there will always be ways to misconfigure plugins, sure. Btw, advertising known-good examples is what our configuration cookbook page is for.

Is there any scenario in which the outcome I listed would be desirable? Misconfigured software is a problem for which users are responsible, but I still don't think there should be any situation under which users should be able to take control of a trac instance in this way.

(BTW 0.4.3 is still listed as the stable version on the AccountManagerPlugin page.)

Oh, my bad. Fixed now. This shall also resolve the issue reported here.

And thanks again, also for your quick reply.

You're welcome.

comment:5 Changed 10 years ago by Steffen Hoffmann

I suppose, something like [13865] (for TagsPlugin) would help here too, wouldn't it? I'll give it a try.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.