Modify

Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#11697 closed defect (fixed)

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

Reported by: erinn_tor Owned by: hasienda
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 3 months ago by hasienda

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 3 months ago by hasienda (previous) (diff)

comment:2 in reply to: ↑ description Changed 3 months ago by hasienda

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 follow-up: Changed 3 months 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 3 months ago by hasienda

  • Resolution set to fixed
  • Status changed from new to closed

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 3 months ago by hasienda

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

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from hasienda. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.