Closed Bug 1538006 (CVE-2019-9813) Opened 5 years ago Closed 5 years ago

ZDI-CAN-8373: Initial code execution: Type confusion in IonMonkey optimizer

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla68
Tracking Status
firefox-esr60 66+ verified
firefox66 blocking verified
firefox67 blocking verified
firefox68 blocking verified

People

(Reporter: Alex_Gaynor, Unassigned)

References

Details

(4 keywords, Whiteboard: [Keep hidden while 1538007 is][jsbugmon:update,testComment=5,origRev=b2f1edb41241])

Attachments

(3 files, 2 obsolete files)

Attached file 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 <jdemooij@mozilla.com>
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

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

Whiteboard: [jsbugmon:update,testComment=1,origRev=b2f1edb41241]
Alias: CVE-2019-9813

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.

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

Severity: normal → blocker
Keywords: regression
Priority: -- → P1

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({});

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

Whiteboard: [jsbugmon:update,testComment=1,origRev=b2f1edb41241] → [jsbugmon:update,testComment=5,origRev=b2f1edb41241]

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?

Keywords: sec-critical

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.

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

Blocks: 1395919
Assignee: nobody → 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
Attachment #9052743 - Flags: sec-approval?

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

sec-approval=dveditz

Attachment #9052743 - Flags: sec-approval? → sec-approval+
Attachment #9052747 - Flags: sec-approval+

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

Whiteboard: [jsbugmon:update,testComment=5,origRev=b2f1edb41241] → [Keep hidden while 1538007 is][jsbugmon:update,testComment=5,origRev=b2f1edb41241]

No issues found in fuzzing the combined patch here.

Attached file crash.html

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

This was from Niklas Baumstark's pwn2own 2019 entry.

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.

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

Group: javascript-core-security → core-security-release
Attachment #9052779 - Attachment is obsolete: true
Attachment #9052778 - Attachment is obsolete: true
Blocks: pwn2own-2019

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

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

Root Cause: --- → ?
Root Cause: ? → Coding: Logical Error
Group: core-security-release
Assignee: tcampbell → nobody
Keywords: bugmon
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: