Update vulnerable lodash version in DevTools Debugger
Categories
(DevTools :: Debugger, defect, P1)
Tracking
(firefox-esr60 wontfix, firefox-esr68 wontfix, firefox68 wontfix, firefox69 wontfix, firefox70 fixed)
People
(Reporter: cr, Assigned: jdescottes)
References
Details
(Keywords: csectype-priv-escalation, sec-other, Whiteboard: [third-party-lib-shipping][post-critsmash-triage][adv-main70-])
Attachments
(2 files)
47 bytes,
text/x-phabricator-request
|
abillings
:
sec-approval+
|
Details | Review |
246 bytes,
text/plain
|
Details |
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.
Reporter | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 2•5 years ago
|
||
Jason, please reassign to someone else ASAP if you are not the right person to work on this.
Comment 3•5 years ago
|
||
Julian, routing this to you as you are familiar with the process for updating Lodash.
Assignee | ||
Comment 4•5 years ago
|
||
Sorry, I missed the bugmail where this got assigned to me. Will take a look.
Assignee | ||
Comment 5•5 years ago
|
||
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.
Updated•5 years ago
|
Assignee | ||
Comment 6•5 years ago
|
||
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
Assignee | ||
Comment 7•5 years ago
|
||
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.
Reporter | ||
Comment 8•5 years ago
|
||
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.
Reporter | ||
Updated•5 years ago
|
Comment 9•5 years ago
|
||
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.
(In reply to Al Billings [:abillings] from comment #9)
Comment on attachment 9080373 [details]
Bug 1566020 - Update vendored lodash version to 4.17.14sec-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.
Assignee | ||
Comment 11•5 years ago
|
||
Adding checkin-needed, as I can't land this myself on Lando.
Comment 12•5 years ago
|
||
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•5 years ago
|
||
Per comment 9, it doesn't sound like we're planning to backport this. Will leave changing the security rating to someone else, though.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 14•5 years ago
|
||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•3 years ago
|
Description
•