1211780 - cannot save password if two password fields with same values

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect)

Description

[Tristan]

I think there's a null pointer bug in LoginManagerContent.jsm. In the code below, oldPasswordField being set to null, and then we try to use the id and name attributes from it. 


    } else { // pwFields.length == 2
      if (pw1 == pw2) {
        // Treat as if 1 pw field
        newPasswordField = pwFields[0].element;
        oldPasswordField = null;
      } else {
        // Just assume that the 2nd password is the new password
        oldPasswordField = pwFields[0].element;
        newPasswordField = pwFields[1].element;
      }
    }

    log("Password field (new) id/name is: ", newPasswordField.id, " / ", newPasswordField.name);
    log("Password field (old) id/name is: ", oldPasswordField.id, " / ", oldPasswordField.name);
    return [usernameField, newPasswordField, oldPasswordField];
  },


This occurs when submitting a form with two password fields containing the same value (eg, registration screen where you choose then confirm your password). The consequence is that the password never gets saved.

Comment 1

[Tristan]

This is the line where the error occurs:

log("Password field (old) id/name is: ", oldPasswordField.id, " / ", oldPasswordField.name);

Comment 2

[:glob ✱]

Attachment: 1211780.patch

Comment 3

[Justin Dolske [:Dolske]]

Comment on attachment 8670123 [details] [diff] [review]
1211780.patch

Review of attachment 8670123 [details] [diff] [review]:
-----------------------------------------------------------------

Uhh. Wow, this broke in Firefox 40 but hasn't been noticed until now?! I'm actually a bit shocked we don't have test coverage for this?

::: toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ +713,5 @@
>        }
>      }
>  
>      log("Password field (new) id/name is: ", newPasswordField.id, " / ", newPasswordField.name);
> +    if (oldPasswordField != null)

Just "if (oldPasswordField)", please.

Comment 4

[:glob ✱]

Attachment: 1211780.patch

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8752bb087900

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8752bb087900

Comment 7

[Justin Dolske [:Dolske]]

Comment on attachment 8670613 [details] [diff] [review]
1211780.patch

Approval Request Comment
[Feature/regressing bug #]: bug 1211780
[User impact if declined]: Can't save passwords when signing up for accounts on some sites
[Describe test coverage new/current, TreeHerder]: Apparently none specifically for this, since it was missed!
[Risks and why]: Low risk. Otherwise well-exercised common code.
[String/UUID change made/needed]: n/a

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8670613 [details] [diff] [review]
1211780.patch

Fix an issue in the password management, taking it. Should be in 42 beta 5

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6d2260ddac36

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8ecb6fafa497