1770048 - (CVE-2022-1529) Top-Level Await must not rely on Array.prototype

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Ted Campbell [:tcampbell]]

The TLA self-hosting code uses a bare [] internally and then uses std_Array_push which may be affected by polluted prototypes.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: jsshell PoC

Comment 2

[Iain Ireland [:iain]]

Attachment: Bug 1770048: Use DefineDataProperty in GatherAsyncParentCompletions r=tcampbell

Comment 3

[Iain Ireland [:iain]]

Attachment: Bug 1770048: Add testcase r=tcampbell!

Depends on D146745

Comment 4

[Iain Ireland [:iain]]

Attachment: Bug 1770048: Improve self-hosted new_List r=jandem,tcampbell

Comment 5

[Iain Ireland [:iain]]

Attachment: Bug 1770048: Improve self-hosted new_List (ESR) r=jandem,tcampbell

Comment 6

[Iain Ireland [:iain]]

Comment on attachment 9277224 [details]
Bug 1770048: Improve self-hosted new_List r=jandem,tcampbell

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Harder than the more narrowly targeted patch we abandoned (https://phabricator.services.mozilla.com/D146745). The important thing about this patch from a security perspective is that we create array objects with a null proto, but the patch doesn't draw attention to that, and it's not obvious which of the half-dozen cases we modified is the exploitable one.
• 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?: All
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: The effect of the patch is to create array objects without prototypes for internal use in self-hosted code. We do our best to avoid looking up methods on the prototype in self-hosted code anyway, so it should not cause problems. The shell test suites pass locally.
• Is Android affected?: Yes

Comment 7

[Andrew McCreight [:mccr8]]

I'm going to mark this as csectype-priv-escalation, because self-hosted code is kind of like privileged code, and the exploit is tricking it into doing something unintended.

Comment 8

[Tom Ritter [:tjr]]

Comment on attachment 9277224 [details]
Bug 1770048: Improve self-hosted new_List r=jandem,tcampbell

Approved to move forward

Comment 9

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

Improve self-hosted new_List r=jandem,tcampbell
https://hg.mozilla.org/integration/autoland/rev/7fce0c26d9ce9e3340a815b13e5c77452e5c3fb0
https://hg.mozilla.org/mozilla-central/rev/7fce0c26d9ce

Comment 10

[Iain Ireland [:iain]]

Comment on attachment 9277224 [details]
Bug 1770048: Improve self-hosted new_List r=jandem,tcampbell

Beta/Release Uplift Approval Request

• User impact if declined: An internal ModuleObject can be leaked to content code and used to gain code execution in the content process.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch is straightforward. The only risk I can think of is that we missed another exploitable case, and we draw attention to it, but we've already landed this patch in nightly so uplifting it doesn't add any risk.
• String changes made/needed:
• Is Android affected?: Yes

Comment 11

[Iain Ireland [:iain]]

Comment on attachment 9277244 [details]
Bug 1770048: Improve self-hosted new_List (ESR) r=jandem,tcampbell

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Sec-crit bug
• User impact if declined: An internal ModuleObject can be leaked to content code and used to gain code execution in the content process.
• Fix Landed on Version: 102
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch is straightforward. The only risk I can think of is that we missed another exploitable case, and we draw attention to it, but we've already landed this patch in nightly so uplifting it doesn't add any risk.

Comment 12

[Dianna Smith [:diannaS]]

Comment on attachment 9277244 [details]
Bug 1770048: Improve self-hosted new_List (ESR) r=jandem,tcampbell

Approved for 91.9.1esr

Comment 13

[Dianna Smith [:diannaS]]

Comment on attachment 9277224 [details]
Bug 1770048: Improve self-hosted new_List r=jandem,tcampbell

Approved for 100.0.2
Approved for 101.0b9

Comment 14

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/26c83ad7c8a8 (91.9.1)
https://hg.mozilla.org/releases/mozilla-esr91/rev/757b55aab315
https://hg.mozilla.org/releases/mozilla-beta/rev/6db78ef7521a
https://hg.mozilla.org/releases/mozilla-release/rev/15984d19afe0

Comment 15

[Ted Campbell [:tcampbell]]

Attachment: Browser PoC

Use a local HTTP server such as python3 -m http.server and visit the localhost URL it loads and visit the HTML page. Test will report PASS or FAIL. If nothing is reported, you probably tried to used a file:// path instead of a local server. Chrome will show PASS for this test.

Comment 16

[Hani Yacoub, Desktop QA]

Verified as fixed on macOS 11.6, Windows 10 x64 and on Ubuntu 20.04 x64.

Comment 17

[Andrei Jolțan [:Andi]]

Hi,

Verified as fixed, on all the following builds the testing result = PASS using a Google Pixel 3a (Android 11) on the following builds:
• Firefox 100.3.0 🎬 Video
• Firefox 101.0.0 Beta 6 🎬 Video
• Firefox Focus RC 100.3.0 🎬 Video
• Firefox Focus Beta 101.0.0-beta.6 🎬 Video

Summary:
Used Python 2 as a simple HTTP Server to serve the files from directory.
Due to he fact that localhost, refers to the device used to access that very hostname (e.g. laptop or workstation) I had to use a complementary tool called ngrok which generates secure public URLs for localhost endpoints, allowing you to test localhost on mobile devices.

P.S. Just to be 100% everything works properly, tested the page on previous Firefox/Focus RC and Beta builds and in all cases the result was FAIL.

Comment 18

[Ted Campbell [:tcampbell]]

Thanks Andrei. Good tip on ngrok, especially for mobile.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3bcffe7f5e20
Add testcase r=tcampbell

Comment 20

[Cristian Tuns]

Backed out for causing SM bustages

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/modules/bug1770048.js | /builds/worker/checkouts/gecko/js/src/jit-test/tests/modules/bug1770048.js:18:4 TypeError: m2.declarationInstantiation is not a function (code 3, args "") [0.1 s]

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c36b9cf6180e
Add testcase r=tcampbell

Comment 22

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/c36b9cf6180e