Opened 3 years ago

Closed 3 years ago

## #11697 closed defect (fixed)

Reported by: Owned by: erinn_tor hasienda normal AccountManagerPlugin critical 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.

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.

### comment:1 Changed 3 years 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

• upper-cased usernames (reserved for Trac permission actions)

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

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

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.

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: ↓ 4 Changed 3 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.)

### comment:4 in reply to: ↑ 3 Changed 3 years ago by hasienda

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

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.

You're welcome.

### comment:5 Changed 3 years ago by hasienda

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