Shell: fix unhandled rejection report in QuickJS

Previously, njs_engine_qjs_unhandled_rejection() kept a pointer to
the rejected promise list while converting the first rejection reason
to a string.  The conversion may call user JavaScript code, which can
reject more promises and cause the list to be reallocated.  After that,
the old pointer was used together with the new item count to free the
promise and reason values, which caused out-of-bounds reads and invalid
frees.

The fix is to take a reference to the first reason and free the list
before conversion, so that rejections created during conversion are
tracked in a new list.

The test/js/promise_rejection_tracker_recursive.t.js test is no longer
skipped for the QuickJS engine.

diff --git a/auto/make b/auto/make
index 113ccbf8a239d158045480cc30e68e0890d0e8d8..1c62b3df07a5923f7ef28ef7fc18bcee48db5ec2 100644 (file)
--- a/auto/make
+++ b/auto/make
@@ -372,8 +372,7 @@ if [ $NJS_HAVE_QUICKJS = YES ]; then
 js_test: js_test_njs js_test_quickjs
 
 js_test_quickjs: njs
-       NJS_SKIP_LIST="test/js/promise_rejection_tracker_recursive.t.js \\
-test/js/async_exception_in_await.t.js" \\
+       NJS_SKIP_LIST="test/js/async_exception_in_await.t.js" \\
        test/js_test --binary='$NJS_BUILD_DIR/njs -n QuickJS -m'
 
 END

diff --git a/external/njs_shell.c b/external/njs_shell.c
index 276a2964e0e150cea061312741fb0e850cec0d1a..126ca9db54f5e1312171de65c892c8cee740f71c 100644 (file)
--- a/external/njs_shell.c
+++ b/external/njs_shell.c
@@ -2787,19 +2787,35 @@ done:
 }
 
 
-static njs_int_t
-njs_engine_qjs_destroy(njs_engine_t *engine)
+static void
+njs_qjs_rejected_promises_free(JSContext *ctx, njs_arr_t *promises)
 {
     uint32_t                i;
-    njs_ev_t                *ev;
-    njs_int_t               ret;
-    JSContext               *cx;
-    njs_queue_t             *events;
-    njs_console_t           *console;
-    njs_262agent_t          *agent;
-    njs_queue_link_t        *link;
     njs_rejected_promise_t  *rejected_promise;
 
+    rejected_promise = promises->start;
+
+    for (i = 0; i < promises->items; i++) {
+        JS_FreeValue(ctx, rejected_promise[i].u.qjs.promise);
+        JS_FreeValue(ctx, rejected_promise[i].u.qjs.message);
+    }
+
+    njs_arr_destroy(promises);
+}
+
+
+static njs_int_t
+njs_engine_qjs_destroy(njs_engine_t *engine)
+{
+    njs_ev_t          *ev;
+    njs_int_t         ret;
+    JSContext         *cx;
+    njs_arr_t         *promises;
+    njs_queue_t       *events;
+    njs_console_t     *console;
+    njs_262agent_t    *agent;
+    njs_queue_link_t  *link;
+
     (void) qjs_call_exit_hook(engine->u.qjs.ctx);
 
     for ( ;; ) {
@@ -2811,13 +2827,11 @@ njs_engine_qjs_destroy(njs_engine_t *engine)
 
     console = JS_GetRuntimeOpaque(engine->u.qjs.rt);
 
-    if (console->rejected_promises != NULL) {
-        rejected_promise = console->rejected_promises->start;
+    promises = console->rejected_promises;
+    console->rejected_promises = NULL;
 
-        for (i = 0; i < console->rejected_promises->items; i++) {
-            JS_FreeValue(engine->u.qjs.ctx, rejected_promise[i].u.qjs.promise);
-            JS_FreeValue(engine->u.qjs.ctx, rejected_promise[i].u.qjs.message);
-        }
+    if (promises != NULL) {
+        njs_qjs_rejected_promises_free(engine->u.qjs.ctx, promises);
     }
 
     events = &console->posted_events;
@@ -2907,9 +2921,10 @@ static njs_int_t
 njs_engine_qjs_unhandled_rejection(njs_engine_t *engine)
 {
     size_t                  len;
-    uint32_t                i;
+    JSValue                 reason;
     JSContext               *ctx;
     const char              *str;
+    njs_arr_t               *promises;
     njs_console_t           *console;
     njs_rejected_promise_t  *rejected_promise;
 
@@ -2922,9 +2937,16 @@ njs_engine_qjs_unhandled_rejection(njs_engine_t *engine)
         return 0;
     }
 
-    rejected_promise = console->rejected_promises->start;
+    promises = console->rejected_promises;
+    rejected_promise = promises->start;
+    reason = JS_DupValue(ctx, rejected_promise->u.qjs.message);
+
+    console->rejected_promises = NULL;
+    njs_qjs_rejected_promises_free(ctx, promises);
+
+    str = JS_ToCStringLen(ctx, &len, reason);
+    JS_FreeValue(ctx, reason);
 
-    str = JS_ToCStringLen(ctx, &len, rejected_promise->u.qjs.message);
     if (njs_slow_path(str == NULL)) {
         return -1;
     }
@@ -2932,14 +2954,6 @@ njs_engine_qjs_unhandled_rejection(njs_engine_t *engine)
     JS_ThrowTypeError(ctx, "unhandled promise rejection: %.*s", (int) len, str);
     JS_FreeCString(ctx, str);
 
-    for (i = 0; i < console->rejected_promises->items; i++) {
-        JS_FreeValue(ctx, rejected_promise[i].u.qjs.promise);
-        JS_FreeValue(ctx, rejected_promise[i].u.qjs.message);
-    }
-
-    njs_arr_destroy(console->rejected_promises);
-    console->rejected_promises = NULL;
-
     return 1;
 }
 

diff --git a/test/shell_test.exp b/test/shell_test.exp
index f394070b68ced6ce945db27d2d85971126d3e194..b56e1be40037574a87f6168579876ed0e2ac8850 100644 (file)
--- a/test/shell_test.exp
+++ b/test/shell_test.exp
@@ -408,6 +408,15 @@ njs_test {
      ">> " "unhandled promise rejection"}
 }
 
+njs_test {
+    {"var later\r\n" "undefined\r\n>> "}
+    {"var message = {toString() { later.catch(() => {}); return 1 }}\r\n"
+     "undefined\r\n>> "}
+    {"Promise.reject(message); later = Promise.reject(2); undefined\r\n"
+     "unhandled promise rejection: 1\r\n*>> "}
+    {"42\r\n" "42\r\n>> " "unhandled promise rejection"}
+}
+
 # CLI OPTIONS
 
 # help