Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#7187 closed defect (fixed)

AccountManager doesn't verify email when resetting password

Reported by: sagar.behere@… Owned by: pacopablo
Priority: high Component: AccountManagerPlugin
Severity: critical Keywords:
Cc: rjollos Trac Release: 0.12

Description

I am using the account manager plugin with SvnServePasswordStore and the passwords coming from a svnserve passwd file.

I have a user called 'test' with email sagar@…. When I click on Forgot password and enter username test and email wrongemail@… and click ok, i get a message saying "Your new password has been emailed to you at wrongemail@…" Then, when I check the svnserve passwd file, I find that the password for user test has been reset to a random hexadecimal like value.

The reset password feature should have refused to work and stated something like "wrongemail@… is not the email associated with username test and the email will NOT be sent."

System: Ubuntu Lucid, trac 0.12b1, I installed account manager from the trunk on 31st May 2010

Attachments (0)

Change History (7)

comment:1 Changed 4 years ago by dr2chase

I think this bug was caused by r7731. It deleted lines containing:

        notifier = PasswordResetNotification(self.env)

        if email != notifier.email_map.get(username):
            return {'error': 'The email and username do not '
                             'match a known account.'}

See the diffs here.

comment:2 Changed 4 years ago by dr2chase

PS: why is this not of the highest priority/severity? This is a trivially exploitable security hole.

comment:3 Changed 4 years ago by Samuel.Degrande@…

I changed it into a case-insensitive check, because our users sometimes write their email address with uppercases, and sometimes with lowercases:

if email.lower() != notifier.email_map.get(username).lower():

return {'error': 'The email and username do not '

'match a known account.'}

comment:4 Changed 4 years ago by rjollos

  • Cc rjollos added

comment:5 follow-up: Changed 4 years ago by anonymous

When setting "acct_mgr.notification.accountchangelistener = enabled", the check against the registered email is done... The code cited by dr2chase was indeed moved inside the AccountManager notifier...

However, if you don't enable acct_mgr.notification.accountchangelistener, no mail is sent, but the password is changed !

comment:6 in reply to: ↑ 5 Changed 4 years ago by hasienda

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

Replying to anonymous:

When setting "acct_mgr.notification.accountchangelistener = enabled", the check against the registered email is done... The code cited by dr2chase was indeed moved inside the AccountManager notifier...

So the initial issue is solved, right? I can't reproduce it either.

However, if you don't enable acct_mgr.notification.accountchangelistener, no mail is sent, but the password is changed !

This is bad, but a different story and this is, what #3989 is for, if I'm totally wrong.

BTW, see #816 for a related enhancement request.

comment:7 Changed 4 years ago by hasienda

Reading #3989 a second time I found, that this wasn't true, but I added the relevant information and reporter of this ticket to Cc-list.

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 pacopablo. 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.