Segfault at Array.prototype.toSpliced

[0xbigshaq]

• Commit: 74854b6edaa8a76fdc96395cdc7fbdfcd01425b6
• Version: 0.8.5

To reproduce, run:

function trigger() {
    let buggy_arr = new Array(0x10);
    let retval;
    Object.defineProperty(buggy_arr, 0, { get: (s) => { throw new Error("brrrrr"); } });
    try { retval = buggy_arr.toSpliced(0,0); } catch (e) { console.log(`error: ${e}`); }
    return retval;
}

let buggy = [];
buggy = trigger();
console.log('triggering bug');
buggy.toString(); // segfault

output:

error: Error: brrrrr
triggering bug
AddressSanitizer:DEADLYSIGNAL
=================================================================
==110021==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x558d0cb556f7 bp 0x7ffe01cc5380 sp 0x7ffe01cc5330 T0)
==110021==The signal is caused by a READ memory access.
==110021==Hint: this fault was caused by a dereference of a high value address (see register values below).  Dissassemble the provided pc to learn which register was used.
    #0 0x558d0cb556f7 in njs_flathsh_find src/njs_flathsh.c:334
    #1 0x558d0cae5551 in njs_object_property src/njs_object_prop.c:103
    #2 0x558d0ca33a2f in njs_value_to_primitive src/njs_value.c:104
    #3 0x558d0caf17aa in njs_value_to_chain src/njs_value_conversion.h:175
    #4 0x558d0caf17aa in njs_array_prototype_join src/njs_array.c:1713
    #5 0x558d0cb0beeb in njs_function_native_call src/njs_function.c:647
    #6 0x558d0cb0beeb in njs_function_frame_invoke src/njs_function.c:683
    #7 0x558d0cb0bf91 in njs_function_call2 src/njs_function.c:515
    #8 0x558d0caee2e9 in njs_function_apply src/njs_function.h:172
    #9 0x558d0caee2e9 in njs_array_prototype_to_string src/njs_array.c:1637
    #10 0x558d0cb0beeb in njs_function_native_call src/njs_function.c:647
    #11 0x558d0cb0beeb in njs_function_frame_invoke src/njs_function.c:683
    #12 0x558d0ca570cb in njs_vmcode_interpreter src/njs_vmcode.c:1451
    #13 0x558d0ca3cc64 in njs_vm_start src/njs_vm.c:664
    #14 0x558d0ca29253 in njs_engine_njs_eval external/njs_shell.c:1387
    #15 0x558d0ca26514 in njs_process_script external/njs_shell.c:3528
    #16 0x558d0ca2cb6e in njs_process_file external/njs_shell.c:3500
    #17 0x558d0ca2cb6e in njs_main external/njs_shell.c:458
    #18 0x558d0ca2cb6e in main external/njs_shell.c:488
    #19 0x7ffbb92d7d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #20 0x7ffbb92d7e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #21 0x558d0ca263a4 in _start (/tmp/njs/build/njs+0x463a4)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV src/njs_flathsh.c:334 in njs_flathsh_find
==110021==ABORTING

Analysis

In Array.prototype.toSpliced / njs_array_prototype_to_spliced, the code performs the following:

1. allocate an array
2. assign the ptr to retval by calling njs_set_array()
3. access the array members(which also trigger getter/setters) to populate the new array's content
4. if something goes wrong, abort and return NJS_ERROR

https://github.com/nginx/njs/blob/74854b6edaa8a76fdc96395cdc7fbdfcd01425b6/src/njs_array.c#L1451-L1468

It looks like the problem is in the implementation of step 3+4(?) We assign to retval a heap chunk with un-initialized data, then, the throw new Error() exception(triggered by our getter) makes the function to bail out in the middle of the toSpliced operation, before the memory pointed by retval is fully initialized.

gef➤  print value->data->u.array->start
$7 = (njs_value_t *) 0x55555564ee00

gef➤  hexdump byte 0x55555564ee00
0x000055555564ee00     5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a    ZZZZZZZZZZZZZZZZ  <---- buggy[0]
0x000055555564ee10     5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a    ZZZZZZZZZZZZZZZZ        buggy[1]
0x000055555564ee20     5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a    ZZZZZZZZZZZZZZZZ        ...
0x000055555564ee30     5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a    ZZZZZZZZZZZZZZZZ
0x000055555564ee40     5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a    ZZZZZZZZZZZZZZZZ
0x000055555564ee50     5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a    ZZZZZZZZZZZZZZZZ
0x000055555564ee60     5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a    ZZZZZZZZZZZZZZZZ
0x000055555564ee70     5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a    ZZZZZZZZZZZZZZZZ

[xeioex]

Hi @0xbigshaq,

Thank you for the report and a good catch.

The root-cause here is in the step 2. We set a retval value too early. Unfortunately in NJS, the retval value will be visible outside the native JS function even if the exception was thrown.

I found similar places in

Array.prototype.toSpliced()
Array.prototype.toReversed()
Array.prototype.toSorted()

The incorrect pattern to look for is when the retval is set before the code that can throw an exception. The correct pattern is when retval is set right before the return from the function.