1732399 - Password auto-fill should not fill in passwords to sites with invalid certificates

Status: NEW

(Toolkit :: Password Manager, defect, P2)

Description

[Chet Wisniewski]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:92.0) Gecko/20100101 Firefox/92.0

Steps to reproduce:

Store a password for a valid https:// website then do a machine-in-the-middle attack and present an imposter website with an invalid TLS certificate, Firefox still offers to fill the password to an imposter site.

Actual results:

Firefox surrendered a stored password to an attacker site.

Expected results:

Firefox should offer no matches if the certificate is invalid. At worst it should offer strong/complex warnings if attempted to bypass.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Toolkit::Password Manager' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Daniel Veditz [:dveditz]]

I assume you mean on sites where the user has instructed Firefox to treat the unknown cert as trusted for that site, in which case I can confirm.

In recent versions we've started noting that there's an override in place whereas in the past we trusted that if the user worked their way through our warnings (iirc it's a multi-click process, though fewer than it used to be) then they really meant it. Now that we do have that warning it would be appropriate to use the http: behavior where we don't automatically fill in fields but instead only offer the autofill when the user interacts with the field.

We could even consider adding a warning to the autofill dropdown to warn about the override, similar to the warning we have on http: password fields (different words entirely, of course). (that behavior is gated by the signon.autofillForms.http pref)

Johann: how does this sound?

Comment 3

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

This sounds like a good plan to me - to treat untrusted cert in the exact same way as an unsecured site.

Comment 4

[Tim Giles [:tgiles]]

Changing severity to S3 because "blocks non-critical functionality and a work around exists "

Comment 5

[Johann Hofmann [:johannh]]

Sounds good to me, too, but probably something for the pwd manager team to implement :)