When does WebSecurity.ChangePassword fail when "new password is invalid"?

Question

This is the code for changing a password in the default AccountController scaffolded in MVC 4:

// ChangePassword will throw an exception rather 
//than return false in certain failure scenarios.
bool changePasswordSucceeded;
try
{
    string userName = User.Identity.Name;    
    changePasswordSucceeded = WebSecurity.ChangePassword(userName, 
                                                         model.OldPassword,
                                                         model.NewPassword);
}
catch (Exception)
{
    changePasswordSucceeded = false;
}

if (changePasswordSucceeded)
{
    return RedirectToAction("Manage", new { Message = ManageMessageId.ChangePasswordSuccess });
}
else
{
    ModelState.AddModelError("",
    "The current password is incorrect or the new password is invalid.");
}

My issue is that the message is unclear. If the current password is incorrect then that's fine but if the new password is invalid I'd like to present a better message to the user telling them what's wrong and I'd like to understand the "failure scenarios" better so that I can tailor the message.

The documentation here is specific about the exceptions and I don't think these should be swallowed in the Action and reported as invalid passwords.

So why is the "new password is invalid" a possibility here? Can I remove it if I'm only using the SimpleMembershipProvider without OAuth in my application?

Edit: NB This Data Annotation also appears in the "RegisterModel" class so there is a password validity check here too

[StringLength(100, 
ErrorMessage = "The {0} must be at least {2} characters long.", 
MinimumLength = 6)]

Answer 1

Summary

This is now a long answer, so I have edited it into sections. I am not a security expert compared to a user like rook, and even he says not to trust his security answers, or anybody else's. Test and understand it yourself, always. Any web developers reading this should read the OWASP top ten web security flaws, use online guidance such as Trustworthy Computing and, of course, Mr. Bruce Schneier. Security is complicated and too big for any one of us, so use frameworks which do the work according to best practice when you can. Above all, keep it all in perspective.

See below for more detail on these summary answers:

1. The message to the user is deliberately ambiguous, it can be argued that this is for security reasons (see below)
2. Ref. the "failure scenarios", they are mainly due to invalid input which should already be caught by model validation, an incorrect current password, or edge case exceptions
3. The exceptions are swallowed and presented as one either because the original developer was lazy, or because they believed there should be a single return message for all cases (see 1 above)
4. "New password is invalid" is probably not a possibility for the code you've highlighted as long as your attributes match, or are more restrictive than the length restrictions within ChangePassword. But, see (1) above.
5. Can I remove it? See (1) above.
6. Good point, see below for discussion.

Original Answer - the conceptual attack vector

Think about it from a security perspective:

• I wander up to your mobile/tablet/laptop sitting on the bar/coffee shop table (or desktop computer in the home/office)
• I think I might know your password
• I go to "change password" and type in what I think your password is NOT
• I deliberately a put a 1 character new password in

The machine tells me

"current password incorrect"

Now I put in try again with what I think your current password is:

"new password invalid"

I now know your password, but you don't know I do as I haven't changed anything.

Footnote: I got the first set of logic wrong, deleted it, edited it and reposted, but hey - I'm not Bruce

A similar attack

There are many combinations of this attack, but it's just like email harvesting from a bad login system. Think what happens if I get two different messages when I try to "recover password" using an email address:

• An email has been sent to your address (for an email in your database)
• Your email could not be found (for an obviously fake email address)

then I can easily find out valid member emails from your site for a targeted phishing attack on that user, or just build a list of valid email addresses for spam.

A possible solution

A more user friendly, but equally secure error message for our password change in both cases would be:

"The current password is incorrect or the new password is invalid. Remember that your password is case sensitive, and new passwords must have [64 letters/5 numbers/4 characters/3 names of greek gods]"

To stay sane, keep this in the context of your application use, but remember you have a responsibility in a bigger eco-system, and users share passwords between websites, whether you like it or not.

When does ChangePassword return false?

Regarding the question part about when ChangePassword returns false:

Essentially WebSecurity.ChangePassword:

In summary, it returns false if:

1. any of the arguments fail null, empty or length checks; or
2. the database connection fails; or
3. the UserId no longer exists in the database; or
4. the current password is incorrect; or
5. (and it will, obviously, set changePasswordSucceeded = false if any other exception happens).
6. There is no newPassword validity check within the call to ChangePassword

So in theory, if you have your validation attributes in place correctly, and ignoring the edge cases, it can only return false if we have an edge case (the user has been deleted, between the Get and Post, or we can't hit the database, or the current password is invalid).

This all holds true even in the case that attackers bypass the UI in the browser (which they will) as the Action itself calls IsValid on the model.

The client side message

There is a serious disclaimer here: I don't spend my life working on security. I follow secure-by-design principles (I keep up with the OWASP top ten project, for instance) and believe I have a good awareness of the most important security principles. I may therefore have got some of this logic wrong.

If the client side "password requirements" validation is not there - what happens? The user has to wait for the Post to return to find out what happened.

If the client side "password requirements" validation is there, does it weaken our interface? I don't believe so.:

• the server still performs both requirements and current password validation
• the server does not distinguish between the two failure scenarios (actually it does, see below)
• therefore the server is only going to tell you if the current password is valid when it is, and when the password therefore changes.
• Hopefully the site also:
  • Prevents changing back to an old password
  • Sends an email notification that your password has been changed (maybe wait for 15 minutes in order to prevent the attacker deleting it from the email you also left open when you left your phone lying around unlocked in a bar with our website opened in it, and our email open as well)

MVC doesn't, AFAIK, do either of these things, so that is where this starts to fall apart.

It also falls apart because we return a different error and message from the server due to the Model.IsValid test inside the action, which enforces the new password validation and the different error message for this failure. Therefore, in its current implementation, the single error message approach is flawed.

Summary

So would I get change the way this is all done? In the hope that I would improve the other parts in time, I probably wouldn't, other than changing the return message to be a bit longer and more informative.

This is a personal opinion, it could be argued the other way

---

The detail:

• calls Membership.GetUser(userName, true).ChangePassword(...). In the case of an ArgumentException see docs it will return false.
• it then calls ChangePassword on the actual provider (e.g. SimpleMembershipProvider.ChangePassword), and will return false if that fails.

The actual provider implementation gets complex. For SimpleMembershipProvider, for example,

• it might pass the call to the "previous provider" if it hasn't been initialised yet.
• Otherwise it might throw an ArgumentExceptions (bubbling up to the catch and return false in MembershipUser.ChangePassword) if the password/username is empty, null, too long etc.
• If that doesn't happen then it will
  • try and get the UserId from the database (returns false if fails),
  • checks the current password is correct (returns false if fails),
  • and finally updates the password with an UPDATE query, setting PasswordChangedDate for you while it goes
• it then updates the internal data on the MembershipUser