From 68664c2a32b7f57f32b724c981215da2e62214e2 Mon Sep 17 00:00:00 2001 From: Wleeaf <179125857+wleeaf@users.noreply.github.com> Date: Sat, 24 Oct 2026 08:33:28 +0000 Subject: [PATCH] Fix use-after-free when freeing a WeakMap with symbol values js_map_finalizer() walked the records of a WeakMap and, for each one, removed its weak reference and then freed its value. When the value is the last reference to a symbol, freeing it runs reset_weak_ref() on the symbol, which unlinks and frees every record of the same map that uses the symbol as a key. If that record was the next one in the list, the loop went on with freed memory: a segfault, a heap-use-after-free under ASan, or the `wr != NULL` assertion in delete_map_weak_ref(). Remove the weak references of all records first and free the values in a second pass, like reset_weak_ref() does, so that freeing a value can no longer reach the records of the map being finalized. Map and Set are unchanged. Fixes: https://github.com/quickjs-ng/quickjs/issues/1835 --- quickjs.c | 14 +++++++++++--- tests/bug1835.js | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 tests/bug1835.js diff --git a/quickjs.c b/quickjs.c index af401ae37..6208e9b23 100644 --- a/quickjs.c +++ b/quickjs.c @@ -53868,12 +53868,20 @@ static void js_map_finalizer(JSRuntime *rt, JSValueConst val) if (s) { /* if the object is deleted we are sure that no iterator is using it */ + if (s->is_weak) { + /* first pass to remove the weak references, so that freeing + a value (e.g. a symbol that is also a key of this map) + cannot reset and free records we have yet to visit */ + list_for_each(el, &s->records) { + mr = list_entry(el, JSMapRecord, link); + if (!mr->empty) + delete_map_weak_ref(rt, mr); + } + } list_for_each_safe(el, el1, &s->records) { mr = list_entry(el, JSMapRecord, link); if (!mr->empty) { - if (s->is_weak) - delete_map_weak_ref(rt, mr); - else + if (!s->is_weak) JS_FreeValueRT(rt, mr->key); JS_FreeValueRT(rt, mr->value); } diff --git a/tests/bug1835.js b/tests/bug1835.js new file mode 100644 index 000000000..acdef3e8e --- /dev/null +++ b/tests/bug1835.js @@ -0,0 +1,36 @@ +// https://github.com/quickjs-ng/quickjs/issues/1835 +// Freeing a WeakMap whose values are symbols that are also its keys crashed +// in the WeakMap finalizer. Run under ASAN. +import { assert } from "./assert.js"; + +function f() { + var o = {}; + var m = new WeakMap(), b = Symbol("b"); + m.set(o, b); + m.set(b, 1); + assert(m.get(o), b); + assert(m.get(b), 1); + b = undefined; + m = undefined; +} +f(); + +function g() { + var o = {}; + var m = new WeakMap(), a = Symbol("a"), b = Symbol("b"), c = Symbol("c"); + m.set(o, a); + m.set(a, b); + m.set(b, c); + m.set(c, 1); + a = b = c = undefined; + m = undefined; +} +g(); + +// freed when the runtime is freed +const m = new WeakMap(); +const a = Symbol("a"), b = Symbol("b"); +m.set(a, b); +m.set(b, a); +assert(m.get(a), b); +assert(m.get(b), a);