1427624 - Avoid saving a munged username

Status: VERIFIED FIXED

(Toolkit :: Password Manager, enhancement, P2)

Description

[Dietrich Ayala (:dietrich)]

We should detect when the username field in a form has been remembered and filled in obfuscated way by the website.

The site I see this regularly on is capitalone.com, but I've seen it happen on other sites as well.

Comment 1

[Dietrich Ayala (:dietrich)]

I consider this more defect than enhancement.

I'm not sure what the ideal behavior is, but allowing the user to save an invalid username is not correct.

Comment 2

[Matthew N. [:MattN]]

IIRC I believe 401k.com is one example.

Comment 3

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1427624 - Pass empty value rather than apparently-obfuscated username in Save Password doorhanger. r?MattN

Comment 4

[Sam Foster [:sfoster] (he/him)]

What if the username with those characters is already saved (because maybe it's the valid username) but not autofilled..

What scenario did you have in mind here?

• The user has saved user**** as their valid username for example.com
• They revisit a login form on example.com.
  • We would normally try to autofill the username field here. I guess if they had multiple matches we wouldn't autofill.
• User types in or selects user**** from autocomplete and hits submit
• We spot they're using an existing login so we should only record usage and not show the save password prompt.

Something like that? I'm not sure how autofill is relevant here if the goal is to just confirm that we are able to correctly handle a munged-looking username value that nonetheless is a saved login.

Comment 5

[Matthew N. [:MattN]]

(In reply to Sam Foster [:sfoster] (he/him) from comment #4)

What if the username with those characters is already saved (because maybe it's the valid username) but not autofilled..

What scenario did you have in mind here?

• The user has saved user**** as their valid username for example.com
• They revisit a login form on example.com.
  • We would normally try to autofill the username field here. I guess if they had multiple matches we wouldn't autofill.
• User types in or selects user**** from autocomplete and hits submit
• We spot they're using an existing login so we should only record usage and not show the save password prompt.

Something like that? I'm not sure how autofill is relevant here if the goal is to just confirm that we are able to correctly handle a munged-looking username value that nonetheless is a saved login.

My concern is if we blanked the username value incorrectly then we wouldn't know that we already have this login saved since we don't compare passwords IIRC. This could easily be a problem if the user manually types everything and therefore never gets the yellow highlight. When we help fill we persist that data in LMC so that's why autofill is relevant. I'm not sure if I'm answering your question though?

Comment 6

[Sam Foster [:sfoster] (he/him)]

Ok yeah that answers my question. It was the "When we help fill we persist that data in LMC" bit I was missing.

Also, I looked into your question (and I'll answer here rather than bury it in the code review):

Can you see what the minimum number of consective munge characters used is on the sites you found? Your notes seem to show at least 4 consecutive for the examples you gave but maybe you also tested with shorter usernames?

I just did some more testing with shorter usernames. Strictly speaking there is no minimum. Each uses a slightly different pattern/algorithm, but basically a really short username "use" could be get obfuscated to *** or u*e. But they seem to want to leave at least 2 non-obfuscated characters, and I would guess usernames shorter than 4 characters are rare. We might get less false positives if I matched on {3,}, and even less for {4,}, but we start to risk missing shorter usernames at that point. Maybe 3 is happy medium?

Comment 7

[Matthew N. [:MattN]]

(In reply to Sam Foster [:sfoster] (he/him) from comment #6)

I just did some more testing with shorter usernames. Strictly speaking there is no minimum. Each uses a slightly different pattern/algorithm, but basically a really short username "use" could be get obfuscated to *** or u*e. But they seem to want to leave at least 2 non-obfuscated characters, and I would guess usernames shorter than 4 characters are rare. We might get less false positives if I matched on {3,}, and even less for {4,}, but we start to risk missing shorter usernames at that point. Maybe 3 is happy medium?

I personally would prefer starting with 3.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/41f4b3d8de33
Pass empty value rather than apparently-obfuscated username in Save Password doorhanger. r=MattN

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/41f4b3d8de33

Comment 10

[Daniel Bodea [:danibodea]]

From the bug's information, I have found these three websites that are or were reproducing the issue:

1. https://www.capitalone.com/ -> does not reproduce any "munged" usernames.
2. https://nb.fidelity.com/public/nb/401k/home -> it DOES reproduce the issue.
3. http://www.td.com/ -> does not have a login form.

In conclusion, only the second one can be used to reproduce and verify the fix for this bug:

• In Nightly 68.0a1 (2019-04-01) (64-bit), both the username (with munged characters) and the password are properly saved by the password manager, while in Nightly 68.0a1 (2019-04-23) (64-bit), only the password is saved by the password manager, leaving the username to be manually inputted by the user.

I am suspecting that this is the intended behavior after the change, but I need confirmation. If this behavior is intended and sufficient to verify the bug, please set firefox68 as verified. If not, please provide me with some test cases or testing ideas.

Thank you!

Comment 11

[Matthew N. [:MattN]]

http://www.td.com/ -> does not have a login form.

See bug 1531651 comment 3 and use https://easyweb.td.com/

See the User Story of https://bugzilla.mozilla.org/show_bug.cgi?id=1530814 for more examples.

Yes, the intended behaviour is to require the user to fill the username themselves if it work otherwise have 3 or more consecutive * or • characters as the username.

Comment 12

[Daniel Bodea [:danibodea]]

The following sites were found to munge the credentials:

1. https://nb.fidelity.com/public/nb/401k/home
2. https://onlinebanking.tdbank.com/#/authentication/login
3. https://online.citi.com/US/login.do
4. https://nb.fidelity.com/public/nb/default/home
5. https://www.security.us.hsbc.com/gsa/SECURITY_LOGON_PAGE/ (does not display the prompt to save credentials for fake accounts)
6. https://stockplanconnect.morganstanley.com/cesreg/Home/Home.html#/home (does not display the prompt to save credentials for fake accounts)

In the case of the first 4 sites, the munged username is not being caught by the password manager credentials saver prompt, only saving the password in this case. The other to sites need valid/existing accounts to be tested.

@Matt: Considering that this is the intended behavior and also considering that this test is enough, I will set this issue's status as VERIFIED in firefox68. Thanks!

P.S. Bug 1530814 and bug 1531651 could be set as a duplicate of this one. Or am I wrong?

Comment 13

[Matthew N. [:MattN]]

Thanks.

Bug 1530814 isn't a duplicate, it's a bug where we made the plan for how to deal with these sites.