1538006 - (CVE-2019-9813) ZDI-CAN-8373: Initial code execution: Type confusion in IonMonkey optimizer

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Alex Gaynor [:Alex_Gaynor]]

Attachment: ZDI-CAN-8373-8374.zip

Initial code execution: Type confusion in IonMonkey optimizer

The vulnerability used by this exploit is an old bug in the type inference engine
of the IonMonkey JIT compiler used by Spidermonkey. It was introduced in 2017:

commit 1c43dd975e76c0aa7e5a84af36844d247baf671a (HEAD, refs/bisect/bad)
Author: Jan de Mooij <[e-mail]>
Date:   Thu Sep 7 12:51:45 2017 +0200

    Bug 1395919 - Don't copy the unknown-properties flag in AddPropertyTypesAfterProtoChange. r=bhackett

Running the poc.js file against a debug build of Spidermonkey will trigger the following assertion

Assertion failure: MIR instruction returned object with unexpected type, at /root/firefox/gecko-dev/js/src/jit/MacroAssembler.cpp:2030
Trace/breakpoint trap (core dumped)

The primitive that results from triggering this issue is essentially the lack
of a shape/group check on an object, which means that we can produce a compiled
JS function that treats an object as if it had a different type.

Specifically, this can be used to confuse a typed array object with a regular
JSObject, causing various possible type confusions between doubles and pointers.

The provided exploit triggers the bug twice, once to leak an object address, and
then a second time to fake a typed array with a fully controlled pointer, achieving
an arbitrary read/write primitive. With this we can map a RWX region to execute our shellcode and escape the sandbox

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

function g(x) {
    y = {};
    y.a = h = function() {
        y.__proto__ = x;
        return 2;
    }
    function f() {
        for (var i = 0; i < 9999; i++) {}
        y.a = h();
    }
    f();
    return y.a[0];
}
g({});
g(Int8Array);

asserts js shell on m-c rev b2f1edb41241 compiled with --enable-debug --enable-more-deterministic with --fuzzing-safe --no-threads at:

Assertion failure: [infer failure] Missing type in object [Object * 0x39281118acd0] a: int, at /home/ubuntu/trees/mozilla-central/js/src/vm/TypeInference.cpp:266

Comment 2

[Christian Holler (:decoder)]

Alternative test for gkw (as discussed on IRC):

function opt(x) {
    x.a[0] = 0;
}
var o1 = new Float64Array(1);
var o2 = {}
function g(proto) {
    var n = {};
    n.a = o1;
    var hack = function() {
        n.__proto__ = proto;
        return o2;
    }
    for (var i = 0; i < 1000; i++) {}
    n.a = hack();
    return opt(n);
}
g({});
g({});

This version will need --fuzzing-safe --no-threads --ion-warmup-threshold=1 to reproduce.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Confirming that bug 1395919 seems to be the regressor.

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/66023b60b9aa
user: Jan de Mooij
date: Thu Sep 07 12:51:45 2017 +0200
summary: Bug 1395919 - Don't copy the unknown-properties flag in AddPropertyTypesAfterProtoChange. r=bhackett

Comment 4

[Ted Campbell [:tcampbell]]

Minor simplification without TypedArrays

function g(x) {
    y = {};
    function h() {
        y.__proto__ = x;
        return 2;
    }
    function f() {
        for (var i = 0; i < 2000; i++) {}
        y.a = h();
    }
    y.a = h;
    f();
    return y.a;
}
g(null);
g({});

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Or this, did it in parallel, after :decoder posted his testcase:

function f() {
    x = [];
    x.z = [];
    function g() {
        x.__proto__ = {};
        return {};
    }
    for (let i = 0; i < 99; i++) {}
    x.z = g();
    function h() {
        x.z;
    }
    return h();
}
f();
f();

asserts js shell on m-c rev b2f1edb41241 compiled with --enable-debug --enable-more-deterministic with --fuzzing-safe --no-threads --ion-warmup-threshold=0 at:

Assertion failure: [infer failure] Missing type in object [Array * 0x8c8d768ab80] z: [Object * 0x8c8d768ab50], at js/src/vm/TypeInference.cpp:266

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1538006 - Propagate unknownProperties when changing prototype

Comment 7

[Ted Campbell [:tcampbell]]

So I iterated on the regressing bug by explicitly calling mark-unknown to propagate rather than copying the other flags. Thoughts, Jan? Did you find other concerns?

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1538006 - Don't emit unbarriered writes to an object if its group might change.

Comment 9

[Ted Campbell [:tcampbell]]

Jan and I would like to take both patches for the sec issue. There are further investigations to do over the next few days, but these patches fix the immediate issue and don't identify anything but TI is hard.

Comment 10

[Ted Campbell [:tcampbell]]

I'm not seeing any change on Speedometer for these patches.

Comment 11

[Ted Campbell [:tcampbell]]

Comment on attachment 9052743 [details]
Bug 1538006 - Propagate unknownProperties when changing prototype

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Both patches point to something around Ion/TI UnknownProperties and prototype-mutation which is unavoidable for any fixes. Unfortunately we give two clues of the pathway, but this is a whack-a-mole game against IonMonkey and more mitigations should out-weight the risk.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
• Which older supported branches are affected by this flaw?: FF57+
• If not all supported branches, which bug introduced the flaw?: Bug 1395919
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: Backports should be straightforward and unlikely to any unique risk. This code has been quite stable since ESR60.
• How likely is this patch to cause regressions; how much testing does it need?: Involves complex parts of the JIT so hard to assess. I don't expect in the wild regressions on real websites. The fixes primarily de-optimize into more well-defined behavior. Patches have been fuzzed and not caused further problems.
• Do you want to request approval of these patches as well?: on

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 9052743 [details]
Bug 1538006 - Propagate unknownProperties when changing prototype

sec-approval=dveditz

Comment 13

[Daniel Veditz [:dveditz]]

The .zip writeup also contains the sandbox escape filed as bug 1538007: keep this one hidden until 1538007 is ready to be opened also.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6b61fd0bb597
https://hg.mozilla.org/mozilla-central/rev/2c49e736571b

https://hg.mozilla.org/releases/mozilla-beta/rev/5aa32b11aaac
https://hg.mozilla.org/releases/mozilla-beta/rev/77536919b121

https://hg.mozilla.org/releases/mozilla-release/rev/eebf74de1376
https://hg.mozilla.org/releases/mozilla-release/rev/662e97c69103

Comment 15

[Ted Campbell [:tcampbell]]

Attachment: Bug 1538006 - [ESR60] Propagate unknownProperties when changing prototype

Comment 16

[Ted Campbell [:tcampbell]]

Attachment: Bug 1538006 - [ESR60] Don't emit unbarriered writes to an object if its group might change.

Depends on D24465

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/4825222c0116
https://hg.mozilla.org/releases/mozilla-esr60/rev/79253cb82ff8

Comment 18

[Christian Holler (:decoder)]

No issues found in fuzzing the combined patch here.

Comment 19

[Ted Campbell [:tcampbell]]

Attachment: crash.html

This is just the JS write-primitive payload that will force a null-ptr crash. It crashes unpatched browser tabs.

Comment 20

[Alex Gaynor [:Alex_Gaynor]]

This was from Niklas Baumstark's pwn2own 2019 entry.

Comment 21

[Ciprian Georgiu, Desktop QA]

Reproduced this crash with the testcase case from comment 19, using an affected Beta build 65.0b12.

This is verified as fixed on the following builds: 68.0a1, 67.0b4, 66.0.1 and 60.6.1esr. Tested on Windows 7 x64, macOS 10.13 and Ubuntu 18.04 x64.

Comment 23

[Daniel Veditz [:dveditz]]

Hiding the original .zip for now because it contains details about the sandbox escape split into bug 1538007.

Comment 24

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Adding :tmaity to CC list as part of RCA Phase II review.

Comment 25

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Requesting a root cause entered for blockers in recent releases in security groups. See :tmaity for details.