1565780 - (CVE-2019-11733) Ability to copy password from password manager without entering master password

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect, P1)

Description

[vb4007]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

You must have some passwords saved in Firefox default credentials manager with a master password.

Start Firefox and enter your master password when prompted.

Then open the credentials dialog from the menu. You should see your credentials list.

Rich-click on one of the credentials, and try to copy the password in your clipboard.

Actual results:

You are prompted for your master password again. Close it without entering your master password, but the password is copied in your clipboard anyway.

Expected results:

Password should not have been copied in your clipboard since you did not enter the master password.

Comment 1

[Matthew N. [:MattN]]

Can you confirm this bug?

Comment 2

[Adrian Florinescu [:aflorinescu]]

Yup, it reproduces. I've tested and reproduced with Ubuntu 16.04 x64 on:

        69.0b5 20190715173502
        68.0esr 20190705221915
        68.0 20190705220548
        70.0a1 20190716211651

Also, checked on 60.8.0esr 20190702220624, where it doesn't reproduce.
I'll keep NI on to get back with a regression range.

Comment 3

[Adrian Florinescu [:aflorinescu]]

2019-07-18T11:15:42: DEBUG : Found commit message:
Bug 1521800 - Convert passwordManager.xul and passwordManager.js to Fluent, r=jaws,flod

Differential Revision: https://phabricator.services.mozilla.com/D19613

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

This is the line in question: https://searchfox.org/mozilla-central/rev/4fbcfe6fecf75d7b828fbebda79a31385f7eab5f/toolkit/components/passwordmgr/content/passwordManager.js#708

masterPasswordLogin() was changed to an async function but this line isn't awaiting the result.

To fix this, CopyPassword will need to be changed to an async function and we need to await the result of masterPasswordLogin().

Comment 5

[Matthew N. [:MattN]]

[Tracking Requested - why for this release]: Regression to snooping protection via MP.

Dan, does it make sense to hide this now that it's been open? Can you assign a severity?

Comment 6

[Matthew N. [:MattN]]

Attachment: Bug 1565780 - Await masterPasswordLogin before copying passwords. r=jaws

Comment 7

[Matthew N. [:MattN]]

It doesn't look like there is test coverage for this case but this UI is getting removed from Firefox eventually and will be off by default in 70 so I'm not sure it's worth making one for a trivial fix.

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This needs a severity rating and depending on its level might also need sec-approval to land. See https://wiki.mozilla.org/Security/Bug_Approval_Process for more details.

Comment 9

[Matthew N. [:MattN]]

I would guess this is sec-moderate as it's similar to "Local storage of passwords in unencrypted form" but Dan can correct me.

Comment 10

[Daniel Veditz [:dveditz]]

In 70 this should be replaced by about:logins, but we definitely don't want this behavior for a year on ESR-68. Let's get this into 69/ESR-68.1 at least, and suggest it as a ride-along if there's another 68.0.x point release.

Comment 11

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/autoland/rev/0d5ae2759150
Await masterPasswordLogin before copying passwords. r=jaws

Comment 12

[Matthew N. [:MattN]]

Comment on attachment 9079137 [details]
Bug 1565780 - Await masterPasswordLogin before copying passwords. r=jaws

Beta/Release Uplift Approval Request

• User impact if declined: A user's master password can be bypassed using the "copy password" context menu item
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1) Set a master password

2. Open Saved Logins… (not the new about:logins so clear the pref signon.management.overrideURI if on 70+)
3. Right click on a saved login and choose "Copy Password". A master password dialog should appear
4. Click Cancel in the dialog

Expected result:
The clipboard doesn't have the password on it

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Trivial fix adding an await on an async function which then requires that function to become async
• String changes made/needed: None

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Even those this is only sec-moderate it's still quite bad for users who use a Master Password and it undermines any perceived benefit that one provides. It also makes Firefox look bad when it comes to perceived security features.
• User impact if declined: A user's master password can be bypassed using the "copy password" context menu item
• Fix Landed on Version: 70 (with uplifts)
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Trivial fix adding an await on an async function which then requires that function to become async
• String or UUID changes made by this patch: None

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/0d5ae2759150672ba03fd4cfdff5fbaf59884b68
https://hg.mozilla.org/mozilla-central/rev/0d5ae2759150

Comment 14

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce this issue using Firefox 70.0a1 (BuildId: 20190713215744) on Windows 10 64bit.

This issue is verified fixed using Firefox 70.0a1 (BuildId: 20190721215935) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 18.04 64bit.

Leaving a ni? on myself as a reminder to verify this fix once uplifted.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9079137 [details]
Bug 1565780 - Await masterPasswordLogin before copying passwords. r=jaws

Fixes a regression allowing passwords to be copied even when a master password was set. Approved for 68.0b7 and 68.1esr.

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/f8413df45c55

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/b2e7fa0e9828

Comment 18

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 69.0b7 (BuildId:20190722201635) and Firefox 68.1.0esr (BuildId:20190722212313) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 18.04 64bit.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9079137 [details]
Bug 1565780 - Await masterPasswordLogin before copying passwords. r=jaws

Per comment 10 and later comments, approving this for 68.0.2 and 68.0.2esr as it's a pretty simple fix for an ugly bug.

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-release/rev/b6cc9968b177

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

FIREFOX_ESR_68_0_X_RELBRANCH https://hg.mozilla.org/releases/mozilla-esr68/rev/4706c3d97dbe

Comment 22

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 68.0.2 (BuildId:20190801110126) and Firefox 68.0.1esr (BuildId:20190801112453) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 18.04 64bit.

Comment 23

[Emil Ghitta, QA [:emilghitta]]

Added a regression test case to our Password Manager & Form Autofill test suite.