Closed Bug 1565780 (CVE-2019-11733) Opened 1 year ago Closed 1 year ago

Ability to copy password from password manager without entering master password

Categories

(Toolkit :: Password Manager, defect, P1)

68 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 68+ verified
firefox68 + verified
firefox69 + verified
firefox70 --- verified

People

(Reporter: vb4007, Assigned: MattN, Mentored)

References

(Regression)

Details

(Keywords: csectype-disclosure, regression, sec-moderate)

Attachments

(1 file)

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.

Component: Untriaged → Password Manager
Product: Firefox → Toolkit

Can you confirm this bug?

Flags: needinfo?(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.

Severity: normal → critical
Status: UNCONFIRMED → NEW
Has STR: --- → yes
Ever confirmed: true

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

Flags: needinfo?(aflorinescu)
Regressed by: 1521800

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

Priority: -- → P1

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

Assignee: nobody → MattN+bmo
Group: firefox-core-security
Status: NEW → ASSIGNED
Flags: needinfo?(dveditz)

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.

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.

Keywords: checkin-needed

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

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.

Flags: needinfo?(dveditz)

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

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
  1. Open Saved Logins… (not the new about:logins so clear the pref signon.management.overrideURI if on 70+)
  2. Right click on a saved login and choose "Copy Password". A master password dialog should appear
  3. 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
Attachment #9079137 - Flags: approval-mozilla-release?
Attachment #9079137 - Flags: approval-mozilla-esr68?
Attachment #9079137 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Group: firefox-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
QA Whiteboard: [qa-triaged]

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.

Flags: needinfo?(emil.ghitta)

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.

Attachment #9079137 - Flags: approval-mozilla-esr68?
Attachment #9079137 - Flags: approval-mozilla-esr68+
Attachment #9079137 - Flags: approval-mozilla-beta?
Attachment #9079137 - Flags: approval-mozilla-beta+

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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(emil.ghitta)

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.

Attachment #9079137 - Flags: approval-mozilla-release? → approval-mozilla-release+
Alias: CVE-2019-11733

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.

Flags: in-qa-testsuite?(gasofie)

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

Flags: in-qa-testsuite?(gasofie) → in-qa-testsuite+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.