371270 - Keychain can fill in username/password that doesn't match data prefilled by JavaScript

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

In another bug, we mentioned that we shouldn't let the Keychain fill in data when what it wants to fill doesn't match the existing username.

I've just run into a scenario where this happens:

I just a got a new account ID/password at investools, so yesterday I logged in with the new details.  My cookie got updated with new values, and I told Camino to update the existing Keychain entry with the new values.  Today I came back to log-in, and the cookie prefilled the new username/password, and then I got the "Allow Camino to access the Keychain" prompt, and after allowing, the entries in the login form were now replaced by the old username/password (which were somehow preserved in the Keychain instead of updated; bug to follow on that).

Comment 1

[Stuart Morgan]

As I suspected, they are doing the fill with JS, rather than the more usual technique of inserting it directly into the HTML when creating the page. Still, it's odd that you could see the username and password when we prompted, so I'll have to play around in gdb to see what's happening.

The fact that they store the password in a cookie in plaintext is really a nice touch.

Comment 2

[Stuart Morgan]

Can you still repro this? I'm not seeing it, but I may not be following the steps correctly.

Comment 3

[Stuart Morgan]

I see what's happening now. We check for a "value" attribute, which is what would be used to prefill during content generation, not the content. Tweaking summary slightly to clarify.

Comment 4

[Stuart Morgan]

Attachment: fix

The fix is trivial; we just need to check the value, which reflects user edits, rather than the value attribute, which doesn't.

Comment 5

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 314262 [details] [diff] [review]
fix

sr=pink

Comment 6

[Stuart Morgan]

Landed on trunk.