1326022 - All passwords are visible after choosing a username (non-e10s)

Status: RESOLVED DUPLICATE of bug 1329351

(Toolkit :: Password Manager, defect, P1)

Description

[Ioana Chiorean]

Ubuntu 16.04 Nightly 2016-12-19
Have Sync activated and logged in

1 go to a page that needs log in
2. enter your username

Actual :
- a list of all passwords is visible in a dropdown password field

Expected:
- I should not be able to see the pass words 

Prefs:
privacy.cpd.offlineApps	true
privacy.cpd.siteSettings	true
privacy.donottrackheader.enabled	true
privacy.sanitize.migrateFx3Prefs	true
privacy.sanitize.timeSpan	3
security.OCSP.enabled	0
security.ssl.errorReporting.automatic	true

Not sure what logs to share - I am a bit concerned about the privacy here.

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

All password from all domains? Password with in clear text? Could you clarify? This indeed sounds serious enough....

Comment 2

[Ioana Chiorean]

Attachment: Screenshot from 2016-12-28 09-18-20.png

These passwords are already changed now or from account I disabled. 
So this is how it looks. 
At this point i am not sure which passwords are shown ( cause for sure I have many others in my account - aka not sure if only unique ones r are tailored in some other way)

Notes:
- It only occurs when I open a website for the first time - after that it will show me the new password management
- I reproduce it on different machines with same sync account. 
- I will investigate it more.

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Ouch. If you can conform this is indeed an regression from bug 1318203 go ahead and request for a backout from m-c (it's PM hours here and we should address this kind of bug within one Nightly version).

Comment 4

[Adrian Florinescu [:aflorinescu]]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #3)
> Ouch. If you can conform this is indeed an regression from bug 1318203 go
> ahead and request for a backout from m-c (it's PM hours here and we should
> address this kind of bug within one Nightly version).

I'd suggest we do not rush to action, until we have identified exactly what is/was causing this. I'm in the same office with Ioana and while I've seen the bug live, I couldn't reproduce it under new profile or with a different sync account that contained limited data. At this point I feel like driving without front lights and these are my thoughts on the matter:

1. Ioana's profile, sync account and indirectly password management can be considered heavy duty (no doubt we will encounter this kind of environment in wild) -> so, this bug in my opinion should be high priority.
2. Are there any code paths that would somehow leak all passwords to the password drop-down, independent of the login form origin?
3. Is there any preference that would disable the hidden property of the password field? (display ***** into plain_text)? - I couldn't find anything relevant in about:config.
4. I'm not entirely sure that bug 1318203 is causing the regression, since Ioana reported this issue happening as early as Hawaii, but it is definitely something related to the password manager changes.

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Adrian Florinescu [:AdrianSV] [No PTO left this year - NI me if required] from comment #4)
> 1. Ioana's profile, sync account and indirectly password management can be
> considered heavy duty (no doubt we will encounter this kind of environment
> in wild) -> so, this bug in my opinion should be high priority.

Agreed.

> 2. Are there any code paths that would somehow leak all passwords to the
> password drop-down, independent of the login form origin?

Passwords are indeed sent to AutoCompletePopup, but it should be always masked. I couldn't find the flag where we mask it in the code after spending 5 min searching for it. Somewhere in the code we must have send the wrong value there.

> 3. Is there any preference that would disable the hidden property of the
> password field? (display ***** into plain_text)? - I couldn't find anything
> relevant in about:config.

I don't know if there is a about:config pref. I highly doubt it's the case.

> 4. I'm not entirely sure that bug 1318203 is causing the regression, since
> Ioana reported this issue happening as early as Hawaii, but it is definitely
> something related to the password manager changes.

If it's before Hawaii it's definitely not bug 1318203. Shouldn't we be locating the actual regression and link the bug there? Mind that a few patches has already been uplifted to Fx52 so we really need to find out more very soon.

Comment 6

[Sean Lee [:seanlee][:weilonge]]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #5)
> (In reply to Adrian Florinescu [:AdrianSV] [No PTO left this year - NI me if
> required] from comment #4)
> 
> > 2. Are there any code paths that would somehow leak all passwords to the
> > password drop-down, independent of the login form origin?
> 
> Passwords are indeed sent to AutoCompletePopup, but it should be always
> masked. I couldn't find the flag where we mask it in the code after spending
> 5 min searching for it. Somewhere in the code we must have send the wrong
> value there.
> 

AFAIK, UserAutoCompleteResult[1] provides the information including Username and Password to show a popup for login fields.
The human readable string can be retrieved from invoking `getLabelAt`.
`getValueAt` (also `getFinalCompleteValueAt` which invokes `getValueAt`) will return a plain password.

So I think the leak point would be the usage of `getValueAt` or `getFinalCompleteValueAt` for showing some human readable strings.

[1] http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/components/passwordmgr/LoginManagerContent.jsm#1237

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Sean and I have read through the code path again. There isn't anything obvious that could get wrong when displaying autocomplete popup in e10s mode. We are sure that the as long as UserAutoCompleteResult is populated with the right |login.username|s, password should never shown in plain text.

autocomplete-rich-result-popup#_appendCurrentResult
http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/content/widgets/autocomplete.xml#1353
AutoCompleteResultView.getCommentAt (act as nsIAutoCompleteController in parent)
http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/components/satchel/AutoCompletePopup.jsm#46
AutoCompletePopup.showPopupWithResults (parent)
http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/components/satchel/AutoCompletePopup.jsm#163
LoginManagerParent.doAutocompleteSearch
http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/components/passwordmgr/LoginManagerParent.jsm#278

(I am not sure if the below code path is for non-e10s or it's actually dead code)

AutoCompletePopup.openAutocompletePopup (content)
http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/content/browser-content.js#1553
AutoCompletePopup.getResultsFromController (content)
http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/content/browser-content.js#1598
nsAutoCompleteController::GetLabelAt
http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/components/autocomplete/nsAutoCompleteController.cpp#767
nsAutoCompleteController::GetResultLabelAt
http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/components/autocomplete/nsAutoCompleteController.cpp#1966
nsAutoCompleteController::GetResultValueLabelAt
http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/components/autocomplete/nsAutoCompleteController.cpp#2007
UserAutoCompleteResult#getLabelAt
http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/toolkit/components/passwordmgr/LoginManagerContent.jsm#1348

---

It might be worthy look into if the sync causes the login search to be corrupted and whether not this bug can only be produced in e10s or non-e10s mode.

Comment 8

[Adrian Florinescu [:aflorinescu]]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #5)
> If it's before Hawaii it's definitely not bug 1318203. Shouldn't we be
> locating the actual regression and link the bug there? Mind that a few
> patches has already been uplifted to Fx52 so we really need to find out more
> very soon.

This bug was reproducible within Hawaii and not before, so I'm not sure how we could find the regression culpable until we have identified some reproduction steps. I've thought it is a good idea for this bug to block bug 1318203 until we know more.
Also, another thing I missed was that this bug was happening while e10s disabled: updating the bug summary accordingly. I'll try to find some steps to reproduce, but meanwhile I believe that a good parallel approach to this would be to check the code changes that have been landed in the Hawaii(roughly 11.24-12.05) period.

Comment 9

[Matthew N. [:MattN]]

[Tracking Requested - why for this release]: Serious regression

Comment 10

[Matthew N. [:MattN]]

So have we only reproduced with non-e10s?

Comment 11

[Matthew N. [:MattN]]

Some things that may be relevant:
* Here we fallback to values if labels are falsy: https://dxr.mozilla.org/mozilla-central/rev/143bb4b9249e528e658f6ccc449991794b8675f8/toolkit/components/satchel/nsFormAutoCompleteResult.jsm#113
* Here an empty (falsy) label is returned: https://dxr.mozilla.org/mozilla-central/rev/143bb4b9249e528e658f6ccc449991794b8675f8/toolkit/components/satchel/AutoCompletePopup.jsm#37,39

Comment 12

[Marcia Knous [:marcia]]

Tracking 52/53+ for this regression in password manager.

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Maybe this relate to bug 1296638?

Comment 14

[Tanvi Vyas[:tanvi]]

Has anyone had any luck reproducing this?

Comment 15

[Tanvi Vyas[:tanvi]]

Hi Ioana,

Did you have any addons installed when you observed this behavior?

Comment 16

[Mike Conley (:mconley) (:⚙️)]

Whoa, this isn't great.

This Reddit thread I (coincidentally) read recently might be relevant:

https://www.reddit.com/r/firefox/comments/5m7o1k/developer_edition_showing_passwords_in_clear_text/

Choice quotes:

"I was toying around with the pass password manager, and its related extension - passff. This consisted of me slowly removing my passwords from Firefox's built in password manager and adding them to pass.

I stopped, and uninstalled the passff extension after a bit, but it seems like whenever I go to certain pages, the password field pops up an autocomplete suggestion box with my passwords in clear text (as if it were a text field!). This doesn't happen on all logins, but does happen on quite a few (including things such as Slack)

I tried disabling all add-ons as well as refreshing my browser, but the problem seems to have persisted. Interestingly enough, it happens on all of my computers that are sync'ed together. However, I'm not sure if this is just because they all got the passff addon installed by sync at some point."

and then they offered this as a workaround:

"Yeah, I tried a refresh originally and it didn't help. I ended up just opening the autocomplete places database and running some SQL to delete all password like entries - that seemed to do the trick.

For anyone else having the same problem:

# Find formhistory.sqlite in your profile and close Firefox
sqlite3 formhistory.sqlite
> select * from moz_formhistory WHERE fieldname LIKE '%password%';
> delete from moz_formhistory WHERE fieldname LIKE '%password%';

That will delete any items from your autocomplete history that have a field name containing or matching password."

Comment 17

[Mike Conley (:mconley) (:⚙️)]

So in this case, somehow the Form History database was being populated with passwords.

Comment 18

[Matthew N. [:MattN]]

(In reply to Mike Conley (:mconley) from comment #17)
> So in this case, somehow the Form History database was being populated with
> passwords.

https://dxr.mozilla.org/mozilla-central/source/toolkit/components/satchel/formSubmitListener.js#129-131 should prevent that for type=password unless these affected sites are toggling between type=password and type=text.

Comment 19

[Matthew N. [:MattN]]

In bug 1329351 I fixed the case where autocomplete results from form history (not saved passwords) were showing on password fields. Looking at the screenshot again I actually think this is a dupe of that since I don't see the key icons in the screenshot but they have been landed for over a month. If anybody sees this issue can they confirm the value of signon.rememberSignons.

Comment 20

[Ioana Chiorean]

(In reply to Tanvi Vyas - behind on bugmail [:tanvi] from comment #15)
> Hi Ioana,
> 
> Did you have any addons installed when you observed this behavior?

some, but none related to passwords ( generic ones like adblock, anonymoux)(In reply to Mike Conley (:mconley) from comment #16)
> Whoa, this isn't great.
> 
> This Reddit thread I (coincidentally) read recently might be relevant:
> 
> https://www.reddit.com/r/firefox/comments/5m7o1k/
> developer_edition_showing_passwords_in_clear_text/

So I am not the only one..

Comment 21

[Matthew N. [:MattN]]

(Quoting Matthew N. [:MattN] from comment #19)
> If anybody sees this issue can they confirm the value of
> signon.rememberSignons.

Comment 22

[Ioana Chiorean]

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #21)
> (Quoting Matthew N. [:MattN] from comment #19)
> > If anybody sees this issue can they confirm the value of
> > signon.rememberSignons.

services.sync.prefs.sync.signon.rememberSignons; true
signon.rememberSignons; true
signon.rememberSignons.visibilityToggle; true

Comment 23

[Matthew N. [:MattN]]

(In reply to Ioana Chiorean from comment #22)
> (In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from
> comment #21)
> > (Quoting Matthew N. [:MattN] from comment #19)
> > > If anybody sees this issue can they confirm the value of
> > > signon.rememberSignons.
> 
> signon.rememberSignons; true

Are you 100% sure that this was the value when you reproduced the bug? This preference sync with Fx Sync btw.


Duping to bug 1329351 for now but please re-open if it happens in a version with that fix.

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

Fixed in 52 in the duplicate bug.