quickjs-tart

commit 3061ee2130e4a08a9257e47552014be553129f9b
parent 2dcd3c12b280ef35b5acfedb44377c160653092a
Author: Florian Dold <florian@dold.me>
Date:   Thu,  6 Jul 2023 18:52:13 +0200

fix quickjs use-after-free

Diffstat:
A
promtest3.js
 | 
33
+++++++++++++++++++++++++++++++++
M
quickjs/quickjs.c
 | 
19
++++++++++++++++++-

2 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/promtest3.js b/promtest3.js
@@ -0,0 +1,33 @@
+// Running this file crashes vanilla quickjs-2021-03-27
+// due to a use-after-free bug.
+// That happens because innerFun keeps a reference
+// to the local variable p, but p gets erroneously
+// garbage-collected.
+// The outerFun infinitely waits for the promise
+// to finish. Thus, its local variables are kept on the
+// stack, and innerFun doesn't directly reference
+// outerFun, so outerFun gets collected
+// by the cycle collector.
+//
+// This seems to only happens in the cycle collector,
+// because in other scenarios, the async function
+// is finalized (and var refs are closed)
+// before the referenced values are garbage-collected.
+
+import * as os from "os";
+import * as std from "std";
+
+let f;
+
+async function outerFun() {
+  let p = { foo: "bar" };
+  function innerFun() {
+    console.log(p.foo);
+  }
+  f = innerFun;
+  // Wait indefinitely
+  await new Promise(function promIni() {});
+}
+
+outerFun();
+os.setTimeout(() => { std.gc(); f(); }, 0);
diff --git a/quickjs/quickjs.c b/quickjs/quickjs.c
@@ -913,11 +913,12 @@ struct JSObject {
         struct JSAsyncFunctionData *async_function_data; /* JS_CLASS_ASYNC_FUNCTION_RESOLVE, JS_CLASS_ASYNC_FUNCTION_REJECT */
         struct JSAsyncFromSyncIteratorData *async_from_sync_iterator_data; /* JS_CLASS_ASYNC_FROM_SYNC_ITERATOR */
         struct JSAsyncGeneratorData *async_generator_data; /* JS_CLASS_ASYNC_GENERATOR */
-        struct { /* JS_CLASS_BYTECODE_FUNCTION: 12/24 bytes */
+        struct { /* JS_CLASS_BYTECODE_FUNCTION: 14/32 bytes */
             /* also used by JS_CLASS_GENERATOR_FUNCTION, JS_CLASS_ASYNC_FUNCTION and JS_CLASS_ASYNC_GENERATOR_FUNCTION */
             struct JSFunctionBytecode *function_bytecode;
             JSVarRef **var_refs;
             JSObject *home_object; /* for 'super' access */
+            JSValue outer_func; /* XXX: combine with var_refs to save space */
         } func;
         struct { /* JS_CLASS_C_FUNCTION: 12/20 bytes */
             JSContext *realm;
@@ -5316,6 +5317,7 @@ static void js_bytecode_function_finalizer(JSRuntime *rt, JSValue val)
     }
     b = p->u.func.function_bytecode;
     if (b) {
+        JS_FreeValueRT(rt, p->u.func.outer_func);
         var_refs = p->u.func.var_refs;
         if (var_refs) {
             for(i = 0; i < b->closure_var_count; i++)
@@ -5338,6 +5340,8 @@ static void js_bytecode_function_mark(JSRuntime *rt, JSValueConst val,
         JS_MarkValue(rt, JS_MKPTR(JS_TAG_OBJECT, p->u.func.home_object),
                      mark_func);
     }
+    JS_MarkValue(rt, p->u.func.outer_func,
+                 mark_func);
     if (b) {
         if (var_refs) {
             for(i = 0; i < b->closure_var_count; i++) {
@@ -5434,6 +5438,7 @@ static void free_object(JSRuntime *rt, JSObject *p)
     p->u.opaque = NULL;
     p->u.func.var_refs = NULL;
     p->u.func.home_object = NULL;
+    p->u.func.outer_func = JS_UNDEFINED;
 
     remove_gc_object(&p->header);
     if (rt->gc_phase == JS_GC_PHASE_REMOVE_CYCLES && p->header.ref_count != 0) {
@@ -15762,11 +15767,22 @@ static JSValue js_closure2(JSContext *ctx, JSValue func_obj,
     p->u.func.function_bytecode = b;
     p->u.func.home_object = NULL;
     p->u.func.var_refs = NULL;
+    p->u.func.outer_func = JS_UNDEFINED;
     if (b->closure_var_count) {
         var_refs = js_mallocz(ctx, sizeof(var_refs[0]) * b->closure_var_count);
         if (!var_refs)
             goto fail;
         p->u.func.var_refs = var_refs;
+        if (JS_IsObject(sf->cur_func)) {
+          // When the outer function is an async or generator
+          // we store a reference to it, to avoid the outer
+          // function from being collected before it
+          // has finished and the var refs are closed.
+          JSObject *fp = JS_VALUE_GET_PTR(sf->cur_func);
+          if (fp->class_id != JS_CLASS_BYTECODE_FUNCTION) {
+            p->u.func.outer_func = JS_DupValue(ctx, sf->cur_func);
+          }
+        }
         for(i = 0; i < b->closure_var_count; i++) {
             JSClosureVar *cv = &b->closure_var[i];
             JSVarRef *var_ref;
@@ -28055,6 +28071,7 @@ static int js_create_module_bytecode_function(JSContext *ctx, JSModuleDef *m)
     b->header.ref_count++;
     p->u.func.home_object = NULL;
     p->u.func.var_refs = NULL;
+    p->u.func.outer_func = JS_UNDEFINED;
     if (b->closure_var_count) {
         var_refs = js_mallocz(ctx, sizeof(var_refs[0]) * b->closure_var_count);
         if (!var_refs)