quickjs icon indicating copy to clipboard operation
quickjs copied to clipboard
verified publisher icon bellard

Running GC or exiting with pending promise in top level await causes use-after-free crash in GC.

Open chmeyer-ms opened this issue 1 year ago • 1 comments

Version 3b45d155c77bbdfe9177b1e03db830d2aff0b2a8

Tested on Windows (vc and clang) and Linux (gcc).

This will crash on Windows but appears to succeed on Linux, however Valgrind reports a use-after-free issue. Windows builds with /fsanitize=address (or -fsanitize) result in a similar report.

On Windows the crash comes from JS_RunGC (either called manually or during JS_FreeRuntime). The top of the call stack is JS_FreeValueRT coming from the finalizer for js_c_function_data_finalizer with a JSCFunctionDataRecord containing js_async_module_execution_fulfilled.

A few things to note:

  • If the promise is first resolved or rejected, no crash or UAF reported.
  • If the await is omitted, no issue.
  • Running with standard qjs.c with this test script does not repro because it will not exit with an unhandled promise.

Test code

int main(int argc, char **argv)
{
    const char* script = 
    "let p = new Promise((resolve, reject) => {"
    "});"
    "await p;";

    JSRuntime* rt = JS_NewRuntime();
    JSContext* ctx = JS_NewContext(rt);
    JSValue ret = JS_Eval(ctx, script, -1, "eval", JS_EVAL_TYPE_MODULE);
    JS_FreeValue(ctx, ret);
    JS_FreeContext(ctx);
    JS_FreeRuntime(rt);
}

Valgrind output.

==29640== Memcheck, a memory error detector
==29640== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==29640== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==29640== Command: ./qjs-debug
==29640==
==29640== Invalid read of size 4
==29640==    at 0x116951: JS_FreeValueRT (quickjs.h:661)
==29640==    by 0x120984: js_c_function_data_finalizer (quickjs.c:5106)
==29640==    by 0x121937: free_object (quickjs.c:5467)
==29640==    by 0x121A14: free_gc_object (quickjs.c:5487)
==29640==    by 0x1226A1: gc_free_cycles (quickjs.c:5837)
==29640==    by 0x1227E8: JS_RunGC (quickjs.c:5868)
==29640==    by 0x1185F1: JS_FreeRuntime (quickjs.c:1956)
==29640==    by 0x116141: main (qjs.c:581)
==29640==  Address 0x4baaa60 is 0 bytes inside a block of size 280 free'd
==29640==    at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==29640==    by 0x117E8A: js_def_free (quickjs.c:1744)
==29640==    by 0x117124: js_free_rt (quickjs.c:1320)
==29640==    by 0x1172E9: js_free (quickjs.c:1375)
==29640==    by 0x168CBC: js_free_module_def (quickjs.c:27323)
==29640==    by 0x118C4C: js_free_modules (quickjs.c:2231)
==29640==    by 0x118FAA: JS_FreeContext (quickjs.c:2317)
==29640==    by 0x120E31: js_autoinit_free (quickjs.c:5188)
==29640==    by 0x120F68: free_property (quickjs.c:5208)
==29640==    by 0x12185D: free_object (quickjs.c:5449)
==29640==    by 0x121A14: free_gc_object (quickjs.c:5487)
==29640==    by 0x1226A1: gc_free_cycles (quickjs.c:5837)
==29640==  Block was alloc'd at
==29640==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==29640==    by 0x117DDE: js_def_malloc (quickjs.c:1728)
==29640==    by 0x1170F2: js_malloc_rt (quickjs.c:1315)
==29640==    by 0x1171AC: js_mallocz_rt (quickjs.c:1336)
==29640==    by 0x117291: js_mallocz (quickjs.c:1365)
==29640==    by 0x168739: js_new_module_def (quickjs.c:27245)
==29640==    by 0x17C751: __JS_EvalInternal (quickjs.c:34420)
==29640==    by 0x17CC80: JS_EvalInternal (quickjs.c:34504)
==29640==    by 0x17CE32: JS_EvalThis (quickjs.c:34535)
==29640==    by 0x17CE9A: JS_Eval (quickjs.c:34543)
==29640==    by 0x11610A: main (qjs.c:578)
==29640==
==29640== Invalid write of size 4
==29640==    at 0x11695A: JS_FreeValueRT (quickjs.h:661)
==29640==    by 0x120984: js_c_function_data_finalizer (quickjs.c:5106)
==29640==    by 0x121937: free_object (quickjs.c:5467)
==29640==    by 0x121A14: free_gc_object (quickjs.c:5487)
==29640==    by 0x1226A1: gc_free_cycles (quickjs.c:5837)
==29640==    by 0x1227E8: JS_RunGC (quickjs.c:5868)
==29640==    by 0x1185F1: JS_FreeRuntime (quickjs.c:1956)
==29640==    by 0x116141: main (qjs.c:581)
==29640==  Address 0x4baaa60 is 0 bytes inside a block of size 280 free'd
==29640==    at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==29640==    by 0x117E8A: js_def_free (quickjs.c:1744)
==29640==    by 0x117124: js_free_rt (quickjs.c:1320)
==29640==    by 0x1172E9: js_free (quickjs.c:1375)
==29640==    by 0x168CBC: js_free_module_def (quickjs.c:27323)
==29640==    by 0x118C4C: js_free_modules (quickjs.c:2231)
==29640==    by 0x118FAA: JS_FreeContext (quickjs.c:2317)
==29640==    by 0x120E31: js_autoinit_free (quickjs.c:5188)
==29640==    by 0x120F68: free_property (quickjs.c:5208)
==29640==    by 0x12185D: free_object (quickjs.c:5449)
==29640==    by 0x121A14: free_gc_object (quickjs.c:5487)
==29640==    by 0x1226A1: gc_free_cycles (quickjs.c:5837)
==29640==  Block was alloc'd at
==29640==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==29640==    by 0x117DDE: js_def_malloc (quickjs.c:1728)
==29640==    by 0x1170F2: js_malloc_rt (quickjs.c:1315)
==29640==    by 0x1171AC: js_mallocz_rt (quickjs.c:1336)
==29640==    by 0x117291: js_mallocz (quickjs.c:1365)
==29640==    by 0x168739: js_new_module_def (quickjs.c:27245)
==29640==    by 0x17C751: __JS_EvalInternal (quickjs.c:34420)
==29640==    by 0x17CC80: JS_EvalInternal (quickjs.c:34504)
==29640==    by 0x17CE32: JS_EvalThis (quickjs.c:34535)
==29640==    by 0x17CE9A: JS_Eval (quickjs.c:34543)
==29640==    by 0x11610A: main (qjs.c:578)
==29640==
==29640== Invalid read of size 4
==29640==    at 0x116960: JS_FreeValueRT (quickjs.h:661)
==29640==    by 0x120984: js_c_function_data_finalizer (quickjs.c:5106)
==29640==    by 0x121937: free_object (quickjs.c:5467)
==29640==    by 0x121A14: free_gc_object (quickjs.c:5487)
==29640==    by 0x1226A1: gc_free_cycles (quickjs.c:5837)
==29640==    by 0x1227E8: JS_RunGC (quickjs.c:5868)
==29640==    by 0x1185F1: JS_FreeRuntime (quickjs.c:1956)
==29640==    by 0x116141: main (qjs.c:581)
==29640==  Address 0x4baaa60 is 0 bytes inside a block of size 280 free'd
==29640==    at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==29640==    by 0x117E8A: js_def_free (quickjs.c:1744)
==29640==    by 0x117124: js_free_rt (quickjs.c:1320)
==29640==    by 0x1172E9: js_free (quickjs.c:1375)
==29640==    by 0x168CBC: js_free_module_def (quickjs.c:27323)
==29640==    by 0x118C4C: js_free_modules (quickjs.c:2231)
==29640==    by 0x118FAA: JS_FreeContext (quickjs.c:2317)
==29640==    by 0x120E31: js_autoinit_free (quickjs.c:5188)
==29640==    by 0x120F68: free_property (quickjs.c:5208)
==29640==    by 0x12185D: free_object (quickjs.c:5449)
==29640==    by 0x121A14: free_gc_object (quickjs.c:5487)
==29640==    by 0x1226A1: gc_free_cycles (quickjs.c:5837)
==29640==  Block was alloc'd at
==29640==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==29640==    by 0x117DDE: js_def_malloc (quickjs.c:1728)
==29640==    by 0x1170F2: js_malloc_rt (quickjs.c:1315)
==29640==    by 0x1171AC: js_mallocz_rt (quickjs.c:1336)
==29640==    by 0x117291: js_mallocz (quickjs.c:1365)
==29640==    by 0x168739: js_new_module_def (quickjs.c:27245)
==29640==    by 0x17C751: __JS_EvalInternal (quickjs.c:34420)
==29640==    by 0x17CC80: JS_EvalInternal (quickjs.c:34504)
==29640==    by 0x17CE32: JS_EvalThis (quickjs.c:34535)
==29640==    by 0x17CE9A: JS_Eval (quickjs.c:34543)
==29640==    by 0x11610A: main (qjs.c:578)
==29640==
==29640==
==29640== HEAP SUMMARY:
==29640==     in use at exit: 0 bytes in 0 blocks
==29640==   total heap usage: 1,501 allocs, 1,501 frees, 135,153 bytes allocated
==29640==
==29640== All heap blocks were freed -- no leaks are possible
==29640==
==29640== For lists of detected and suppressed errors, rerun with: -s
==29640== ERROR SUMMARY: 6 errors from 3 contexts (suppressed: 0 from 0)

chmeyer-ms avatar Apr 15 '24 18:04 chmeyer-ms

Does force-rejecting all of the promises in the system resolve the use-after-free here?

JakeShirley avatar May 08 '24 21:05 JakeShirley

Closing the loop on this. The issue here is that the top level async functions take a ref to the module but the module is deleted on shutdown without any regard for the ref counts held by these functions. Then when these function release after the module is destroyed they try to decref their data, accessing freed memory.

static int js_execute_async_module(JSContext *ctx, JSModuleDef *m)
{
    JSValue promise, m_obj;
    JSValue resolve_funcs[2], ret_val;
    promise = js_async_function_call(ctx, m->func_obj, JS_UNDEFINED, 0, NULL, 0);
    if (JS_IsException(promise))
        return -1;
    m_obj = JS_NewModuleValue(ctx, m);
    resolve_funcs[0] = JS_NewCFunctionData(ctx, js_async_module_execution_fulfilled, 0, 0, 1, (JSValueConst *)&m_obj);
    resolve_funcs[1] = JS_NewCFunctionData(ctx, js_async_module_execution_rejected, 0, 0, 1, (JSValueConst *)&m_obj);
    ret_val = js_promise_then(ctx, promise, 2, (JSValueConst *)resolve_funcs);
    JS_FreeValue(ctx, ret_val);
    JS_FreeValue(ctx, m_obj);
    JS_FreeValue(ctx, resolve_funcs[0]);
    JS_FreeValue(ctx, resolve_funcs[1]);
    JS_FreeValue(ctx, promise);
    return 0;

My workaround was to wrap the raw pointer in a JS_NewArrayBufferCopy, and assign that to the JS_NewCFunctionData, preventing the use after free. Not a great solution, but worked.

new code:

static int js_execute_async_module(JSContext *ctx, JSModuleDef *m)
{
    JSValue promise, module_ref;
    JSValue resolve_funcs[2], ret_val;
    promise = js_async_function_call(ctx, m->func_obj, JS_UNDEFINED, 0, NULL, 0);
    if (JS_IsException(promise))
        return -1;
    module_ref = JS_NewArrayBufferCopy(ctx, (uint8_t*)&m, sizeof(JSModuleDef*)); // ptr to moduleDef without ref count of module itself
    resolve_funcs[0] = JS_NewCFunctionData(ctx, js_async_module_execution_fulfilled, 0, 0, 1, (JSValueConst *)&module_ref);
    resolve_funcs[1] = JS_NewCFunctionData(ctx, js_async_module_execution_rejected, 0, 0, 1, (JSValueConst *)&module_ref);
    ret_val = js_promise_then(ctx, promise, 2, (JSValueConst *)resolve_funcs);
    JS_FreeValue(ctx, ret_val);
    JS_FreeValue(ctx, module_ref);
    JS_FreeValue(ctx, resolve_funcs[0]);
    JS_FreeValue(ctx, resolve_funcs[1]);
    JS_FreeValue(ctx, promise);
    return 0;
}

chmeyer-ms avatar Mar 18 '25 20:03 chmeyer-ms

fixed

bellard avatar Jun 28 '25 15:06 bellard