Closed Bug 1427624 Opened 6 years ago Closed 5 years ago

Avoid saving a munged username

Categories

(Toolkit :: Password Manager, enhancement, P2)

enhancement

Tracking

()

VERIFIED FIXED
mozilla68
Tracking Status
firefox68 --- verified

People

(Reporter: dietrich, Assigned: sfoster)

References

(Depends on 2 open bugs)

Details

(Whiteboard: [passwords:heuristics])

Attachments

(1 file)

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.
Component: General → Password Manager
Product: Firefox → Toolkit
Severity: normal → enhancement
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.
Whiteboard: [passwords:heuristics]

IIRC I believe 401k.com is one example.

Priority: -- → P2
Severity: enhancement → normal
Summary: password save dialog always pops up when username is "firstnam***************" → Avoid saving a munged username
See Also: → 1530814
Depends on: 1531651
Assignee: nobody → sfoster
Status: NEW → ASSIGNED

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.

Flags: needinfo?(MattN+bmo)

(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?

Flags: needinfo?(MattN+bmo)

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?

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

Attachment #9053462 - Attachment description: Bug 1427624 - Pass empty value rather than munged username in Save Password doorhanger. r?MattN → Bug 1427624 - Pass empty value rather than apparently-obfuscated username in Save Password doorhanger. r?MattN
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/41f4b3d8de33
Pass empty value rather than apparently-obfuscated username in Save Password doorhanger. r=MattN
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Depends on: 1543454
Depends on: 1544883

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!

Flags: needinfo?(MattN+bmo)

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.

Flags: needinfo?(MattN+bmo)
Blocks: 1531651
No longer depends on: 1531651

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?

Status: RESOLVED → VERIFIED
Flags: needinfo?(MattN+bmo)

Thanks.

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

Flags: needinfo?(MattN+bmo)
Blocks: 1313699
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: