Closed Bug 1388674 Opened 8 years ago Closed 6 years ago

Only prompt to save logins if a field in a login form was modified (off-by-default)

Tracking

()

Status:

VERIFIED FIXED

Milestone:

mozilla73

Tracking Flags:

Tracking

Status

relnote-firefox

---

73+

firefox68

---

wontfix

firefox69

---

wontfix

firefox70

---

wontfix

firefox71

---

wontfix

firefox72

---

wontfix

firefox73

---

verified

People

(Reporter: jukka.lindgren, Assigned: sfoster)

References

(Depends on 1 open bug, Blocks 3 open bugs, Regressed 1 open bug)

Details

(Keywords: perf-alert, Whiteboard: [passwords:heuristics])

User Story

Preference: signon.userInputRequiredToCapture.enabled

* Use a one-time `input` event listener on the detected un/pw fields to update `fieldModified` below. Check isTrusted.
* Ensure that an autofill by Firefox doesn't count as a user modification.
* Check event.isTrusted
* Since LoginManagerContent._getFormFields can return different field depending on whether we're submitting (`isSubmission`), check if the event listener was added to the field upon submission and if it wasn't tracked then prompt anyways (see `trackedFields` below). 

```js
stateForDocument(doc).fieldModificationsByRootElement.set(formLike.rootElement, {
  fieldModified: false, // true if one of the tracked fields was modified
  trackedFields: new WeakSet(),
});
```

Attachments

(8 files, 4 obsolete files)

Bug 1388674 - Only prompt to save logins if a login field was modified by the user. r?MattN 6 years ago :prathiksha 47 bytes, text/x-phabricator-request		Details \| Review
Testcase 1: Username and password fields filled by JS 6 years ago Matthew N. [:MattN] 864 bytes, text/html		Details
Testcase 2: From bug 1568940 without JS 6 years ago Matthew N. [:MattN] 317 bytes, text/html		Details
Bug 1388674 - (WIP) Only prompt to save logins if a login field was modified by the user 6 years ago Sam Foster [:sfoster] (he/him) 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1388674 - Update tests to use setUserInput, ensuring we get an input event for field modifications. r?MattN 6 years ago Sam Foster [:sfoster] (he/him) 47 bytes, text/x-phabricator-request		Details \| Review
mochitest-plain-audit.md 6 years ago Sam Foster [:sfoster] (he/him) 11.37 KB, text/plain		Details
browser-chrome-audit.md 6 years ago Sam Foster [:sfoster] (he/him) 6.94 KB, text/plain		Details
Bug 1388674 - Use document state to track generated password fields, not just the event handlers. r?MattN 6 years ago Sam Foster [:sfoster] (he/him) 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1388674 - Set signon.userInputRequiredToCapture.enabled in AutofillDelegateTest.kt. r=sfoster 6 years ago Matthew N. [:MattN] 47 bytes, text/x-phabricator-request		Details \| Review
Log for "In a form" testcase fail 6 years ago Timea Cernea [:tbabos][inactive] 10.78 KB, text/plain		Details
Testcase 3: Multiple forms without JS 5 years ago Matthew N. [:MattN] 602 bytes, text/html		Details
Testcase 3: Multiple forms without JS 5 years ago Matthew N. [:MattN] 678 bytes, text/html		Details

jukka.lindgren

Reporter

Description

•

8 years ago

Attached file Password_displayed.zip — Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:55.0) Gecko/20100101 Firefox/55.0 Build ID: 20170803105720 Steps to reproduce: 1. Browse to any web page, where a Login + Password fields are shown. The existing password is shown as dots, to stop a passer-by to see the password. 2. Do not edit any of the Login / Password fields. 3. Click any link to browse to another page. Do not type a new address. 4. The "Would you like Firefox to save this login for [domain name]?" dialog will pop up - as if the user had entered a new password to the appropriate field. 5. Click "Show Password" on that dialog. 6. The password from that page is displayed in clear text. If the page contains a hash value (as opposed to actual password in text), the hash value is displayed. Actual results: A third-party user's password may be revealed, if a page containing that users Login + Password is visited. If the page has saved the password hash value, instead of the actual password, the hash value is displayed. It may concievably be used to obtain the actual password. Expected results: Leaving a page containing an existing password in hidden form should not pop up the "Save password?" dialog, making the saved password visible to other user. Please note: Attached screenshots (in zip file) show the case with a hashed password only. Due to privacy/security concerns, I do not wish to attach screenshot with a clear-text password.

:Gijs (he/him)

Comment 1

•

8 years ago

This is an expected part of how the web browser works. The same values could be inspected with e.g. the developer tools or a purpose-built add-on. It's not clear to me how you believe this feature poses a greater risk to the user or their machine. As a result, removing the 'security sensitive' flag from this bugreport (as it keeps it hidden from the rest of the world). (In reply to jukka.lindgren from comment #0) > A third-party user's password may be revealed, if a page containing that > users Login + Password is visited. A webpage should never include a third-party user password in its markup or password fields if the entity requesting the page should not see said password. If it does, the website must assume that the password is compromised from that point onwards. > If the page has saved the password hash value, instead of the actual > password, the hash value is displayed. It may concievably be used to obtain > the actual password. There are two cases here: 1) the hash is reversible or not salted. Consider using a better hash algorithm and a salt. As above, the website should consider the password compromised from the point where it sent the compromisable hash. 2) the hash is not reversible and properly salted. In this case, exposing the hash has limited risk - but that risk is incurred by the website as soon as it exposes the hash, not by the browser for displaying the hash to the user. > Please note: Attached screenshots (in zip file) show the case with a hashed > password only. Due to privacy/security concerns, I do not wish to attach > screenshot with a clear-text password. I've marked your attachment as private just in case.

Group: firefox-core-security

Component: Untriaged → Password Manager

Product: Firefox → Toolkit

jukka.lindgren

Reporter

Comment 2

•

8 years ago

(In reply to :Gijs from comment #1) This phenomenon (dialog box displayed when leaving the page) only appeared on Firefox 55.0 on Mac. This is new. I cannot see any rationale on displaying this dialog, if no new Login or Password has been entered - and there is nothing to save. On all previous versions of Firefox, only entering something onto Login/Password fields triggered the "Save?" dialog. > This is an expected part of how the web browser works. The same values could > be inspected with e.g. the developer tools or a purpose-built add-on. It's > not clear to me how you believe this feature poses a greater risk to the > user or their machine. While it is possible to find the hash value via e.g. "Inspect element" function, any additional layer of security is desirable. Revealing it so obviously to a common, non-expert user gives this issue much vider audience. In terms of 'security through obscurity', this is never a good idea.

:Gijs (he/him)

Comment 3

•

8 years ago

(In reply to jukka.lindgren from comment #2) > (In reply to :Gijs from comment #1) > This phenomenon (dialog box displayed when leaving the page) only appeared > on Firefox 55.0 on Mac. This is new. > I cannot see any rationale on displaying this dialog, if no new Login or > Password has been entered - and there is nothing to save. > On all previous versions of Firefox, only entering something onto > Login/Password fields triggered the "Save?" dialog. It's impossible to comment on this precisely without insight into what pages you're seeing this on and what their markup looks like. However, on earlier versions of Firefox we didn't do a good job of allowing users to save passwords in password fields outside <form> elements. Given that webpages do use such password fields, that got fixed. If, as a side-effect, we are now showing the prompt when we shouldn't, that is interesting and potentially fixable because of correctness rather than security concerns. However, we can't really diagnose this without a testcase. Can you provide one, either a link or an attachment, that you think shouldn't prompt but does?

Flags: needinfo?(jukka.lindgren)

jukka.lindgren

Reporter

Comment 4

•

8 years ago

(In reply to :Gijs from comment #3) > Can you provide one, either a link or an attachment, that you > think shouldn't prompt but does? Unfortunately I found this on an FTP server administrator page. This is CrushFTP 7.7.0_69, which is available as a trial version (I believe). The problem occurs on the "User Manager" on the admin section of that software's web UI. If you cannot find a way to reproduce this on any other web site, I will try to arrange access under high confidentiality.

Flags: needinfo?(jukka.lindgren)

Matthew N. [:MattN]

Comment 5

•

8 years ago

I thought we had a bug on not asking to save for fields that weren't changed but I can't find it. I think that would mostly address this. Otherwise the prompt and toggling visibility are working as expected. If you don't want the password to be revealed in the prompt, you can set a Master Password.

Status: UNCONFIRMED → NEW

Ever confirmed: true

Priority: -- → P3

Summary: Hidden password on a visited page revealed by "Save login?" dialog → Only prompt to save logins if a login field was modified