1566020 - (CVE-2019-10744) Update vulnerable lodash version in DevTools Debugger

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P1)

Description

[Christiane Ruetten [INACTIVE]]

The lodash team released a security update for a critical prototype pollution vulnerability that can lead to remote code execution.

• https://github.com/lodash/lodash/issues/4348
• https://github.com/lodash/lodash/pull/4336
• https://github.com/lodash/lodash/pull/4355

Only the latest release of lodash is not vulnerable. Please update dependencies to the latest release version, or at least verify that our code does not expose the vulnerability to untrusted input. (If not, I'd advise to update anyway.)

The non-vulnerable release versions are:

• lodash 4.17.14
• lodash.merge 4.6.2
• lodash.template 4.5.0

Any older releases contain the vulnerable code.

I'm assuming that this lodash code is actually shipping. Please report back here if it's not.

Comment 1

[Christiane Ruetten [INACTIVE]]

• list of files referencing lodash

@jlaster, who else to involve here?

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Jason, please reassign to someone else ASAP if you are not the right person to work on this.

Comment 3

[Jason Laster [:jlast]]

Julian, routing this to you as you are familiar with the process for updating Lodash.

Comment 4

[Julian Descottes [:jdescottes]]

Sorry, I missed the bugmail where this got assigned to me. Will take a look.

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1566020 - Update vendored lodash version to 4.17.14

This is the only version of lodash that actually ships with Firefox.
We have some package.json files that also reference lodash but they are used for tests only.

Comment 6

[Julian Descottes [:jdescottes]]

Still pending review on the patch, but should we lift the security tag here?

The vulnerabilities were about:

• template -> not used by the debugger
• safeGet -> introduced in 4.17.5, while we use 4.17.4 right now. The functions relying on this are baseMerge and baseMergeDeep, also not used by the debugger

Comment 7

[Julian Descottes [:jdescottes]]

Comment on attachment 9080373 [details]
Bug 1566020 - Update vendored lodash version to 4.17.14

Security Approval Request

• How easily could an exploit be constructed based on the patch?: No exploit is possible, the debugger is not using the methods that had a security issue.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?:
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: No exploit is possible, the debugger is not using the methods that had a security issue.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely, debugger only uses basic features from lodash. Regressions should have been caught on try.

Comment 8

[Christiane Ruetten [INACTIVE]]

Patch LGTM from a security standpoint. I'm a little concerned about lifting the sec tag until we addressed all the other lodash dependencies in m-c as well, either by bumping all the refs, or by ensuring non-exploitability.

Comment 9

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 9080373 [details]
Bug 1566020 - Update vendored lodash version to 4.17.14

sec-approval+ for trunk. It doesn't sound like something we need to backport to Beta.

Comment 10

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Al Billings [:abillings] from comment #9)

Comment on attachment 9080373 [details]
Bug 1566020 - Update vendored lodash version to 4.17.14

sec-approval+ for trunk. It doesn't sound like something we need to backport to Beta.

There is no risk in this instance, as per comment 16. I don't think this should be rated sec-critical.

Comment 11

[Julian Descottes [:jdescottes]]

Adding checkin-needed, as I can't land this myself on Lando.

Comment 12

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

If you add you Phabricator key to Lando, you should also be able to land restricted revisions (but Lando sometimes forgets the key).

https://hg.mozilla.org/integration/autoland/rev/f245cd44c403ae536656c123856a82a1991610ad
https://hg.mozilla.org/mozilla-central/rev/f245cd44c403

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Per comment 9, it doesn't sound like we're planning to backport this. Will leave changing the security rating to someone else, though.

Comment 14

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: advisory.txt