node:zlib: copy dictionary into owned buffer to prevent use-after-free on detach

src/runtime/node/zlib/NativeZlib.rs

@@ -198,11 +198,6 @@ mod _impl {
                 write_callback.with_async_context_if_needed(global),
             );
 
-            // Keep the dictionary alive by keeping a reference to it in the JS object.
-            if dictionary.is_some() {
-                js::dictionary_set_cached(this_value, global, arguments.ptr[6]);
-            }
-
             self.stream
                 .with_mut(|s| s.init(level, window_bits, mem_level, strategy, dictionary));
 
@@ -264,10 +259,13 @@ pub struct Context {
     pub state: c::z_stream,
     pub err: c::ReturnCode,
     pub flush: c::FlushValue,
-    // Borrows a JS ArrayBuffer kept alive via `js::dictionary_set_cached`
-    // (BACKREF/FFI class) for the lifetime of the JS wrapper, which strictly
-    // outlives this Context — `RawSlice` invariant. Default is `EMPTY`.
-    pub dictionary: bun_ptr::RawSlice<u8>,
+    // Owned copy of the user-supplied dictionary. The user's ArrayBuffer can
+    // be detached after `init()` (e.g. `ArrayBuffer.prototype.transfer()`),
+    // freeing its backing store while inflate still reads the dictionary
+    // lazily from the threadpool on `Z_NEED_DICT` (and on `reset()`), so
+    // borrowing the JS buffer would be a use-after-free. Node.js copies too
+    // (`ZlibContext::dictionary_` is a `std::vector<unsigned char>`).
+    pub dictionary: Box<[u8]>,
     pub gzip_id_bytes_read: u8,
 }
 
@@ -278,7 +276,7 @@ impl Default for Context {
             state: bun_core::ffi::zeroed::<c::z_stream>(),
             err: c::ReturnCode::Ok,
             flush: c::FlushValue::NoFlush,
-            dictionary: bun_ptr::RawSlice::EMPTY,
+            dictionary: Box::default(),
             gzip_id_bytes_read: 0,
         }
     }
@@ -290,7 +288,7 @@ impl Context {
 
     #[inline]
     fn dictionary(&self) -> &[u8] {
-        self.dictionary.slice()
+        &self.dictionary
     }
 
     pub fn init(
@@ -315,10 +313,10 @@ impl Context {
             ZSTD_COMPRESS | ZSTD_DECOMPRESS => unreachable!(),
         };
 
-        // See field comment on `dictionary` — `RawSlice` invariant.
+        // See field comment on `dictionary` — copy into an owned buffer.
         self.dictionary = match dictionary {
-            Some(d) => bun_ptr::RawSlice::new(d),
-            None => bun_ptr::RawSlice::EMPTY,
+            Some(d) => Box::from(d),
+            None => Box::default(),
         };
 
         match self.mode {
@@ -369,11 +367,13 @@ impl Context {
         };
         self.err = c::ReturnCode::Ok;
         match self.mode {
-            // SAFETY: FFI — state is an initialized deflate stream; dict_ptr/dict_len borrow a rooted ArrayBuffer.
+            // SAFETY: FFI — state is an initialized deflate stream; dict_ptr/dict_len point into
+            // `self.dictionary` (owned Box), which outlives this call.
             DEFLATE | DEFLATERAW => unsafe {
                 self.err = c::deflateSetDictionary(&raw mut self.state, dict_ptr, dict_len);
             },
-            // SAFETY: FFI — state is an initialized inflate stream; dict_ptr/dict_len borrow a rooted ArrayBuffer.
+            // SAFETY: FFI — state is an initialized inflate stream; dict_ptr/dict_len point into
+            // `self.dictionary` (owned Box), which outlives this call.
             INFLATERAW => unsafe {
                 self.err = c::inflateSetDictionary(&raw mut self.state, dict_ptr, dict_len);
             },
@@ -556,7 +556,8 @@ impl Context {
                 let dict = self.dictionary();
                 (dict.as_ptr(), u32::try_from(dict.len()).expect("int cast"))
             };
-            // SAFETY: FFI — state is an initialized inflate stream; dict is rooted.
+            // SAFETY: FFI — state is an initialized inflate stream; dict_ptr/dict_len point into
+            // `self.dictionary` (owned Box), which outlives this call.
             self.err = unsafe { c::inflateSetDictionary(&raw mut self.state, dict_ptr, dict_len) };
 
             if self.err == c::ReturnCode::Ok {
@@ -625,6 +626,7 @@ impl Context {
         }
         debug_assert!(status == c::ReturnCode::Ok || status == c::ReturnCode::DataError);
         self.mode = NONE;
+        self.dictionary = Box::default();
     }
 }
 

test/js/node/zlib/zlib-dictionary-detach.test.ts

@@ -0,0 +1,167 @@
+// Regression test: node:zlib stored a raw pointer into the user-supplied
+// dictionary ArrayBuffer and read it lazily from the threadpool
+// (inflateSetDictionary on Z_NEED_DICT, and on reset()). Caching the JS view
+// does not prevent the underlying ArrayBuffer from being detached —
+// ArrayBuffer.prototype.transfer(newLength) with a different length
+// synchronously frees the old backing store, leaving the native handle with
+// a dangling pointer. Under ASAN this is a heap-use-after-free in
+// adler32()/inflateSetDictionary() on the worker thread.
+//
+// The fix copies the dictionary into an owned buffer in Context.init(),
+// matching Node.js (ZlibContext::dictionary_ is a std::vector).
+
+import { expect, test } from "bun:test";
+import { bunEnv, bunExe, isWindows } from "harness";
+
+// Malloc=1 routes JSC ArrayBuffer allocations through system malloc instead
+// of bmalloc/libpas so that ASAN poisons freed ArrayBuffer backing stores.
+// Without it bmalloc keeps the freed region in its own free list and ASAN
+// never sees the UAF. bmalloc's SystemHeap is unimplemented on Windows
+// (RELEASE_BASSERT_NOT_REACHED), so skip it there — Windows CI isn't ASAN
+// anyway, and the test still verifies correctness.
+//
+// detect_leaks=0: Malloc=1 also exposes pre-existing small runtime leaks
+// (parser/transpiler allocations normally hidden behind bmalloc) to
+// LeakSanitizer, which would print them to stderr at exit. We only care
+// about the heap-use-after-free here.
+// symbolize=0: when this test is run against an unfixed build, ASAN aborts
+// and symbolizing the debug binary takes longer than the default test
+// timeout. We only care that the subprocess prints "OK" and exits 0.
+// allow_user_segv_handler=1 suppresses JSC's "ASAN interferes with JSC
+// signal handlers" stderr banner on ASAN builds where bunEnv didn't set it.
+const asanOptions = [bunEnv.ASAN_OPTIONS, "allow_user_segv_handler=1", "symbolize=0", "detect_leaks=0"]
+  .filter(Boolean)
+  .join(":");
+const env = { ...bunEnv, ...(isWindows ? {} : { Malloc: "1" }), ASAN_OPTIONS: asanOptions };
+
+const inflateFixture = /* js */ `
+  const zlib = require("zlib");
+
+  const expected = Buffer.alloc(64, "a").toString();
+  const ab = new ArrayBuffer(4096);
+  const dict = Buffer.from(ab);
+  dict.fill("a");
+
+  // Deflate with a dictionary so the stream sets FDICT and inflate() will
+  // return Z_NEED_DICT, which triggers inflateSetDictionary() on the
+  // threadpool with the stored dictionary pointer.
+  const payload = zlib.deflateSync(Buffer.alloc(64, "a"), { dictionary: dict });
+
+  const inf = zlib.createInflate({ dictionary: dict });
+  inf.on("error", err => {
+    console.error("error:", err.message);
+    process.exitCode = 1;
+  });
+  let out = Buffer.alloc(0);
+  inf.on("data", chunk => {
+    out = Buffer.concat([out, chunk]);
+  });
+  inf.on("end", () => {
+    console.log(out.toString() === expected ? "OK" : "WRONG: " + out.toString());
+  });
+
+  // transfer() with a different length allocates a new backing, memcpy's, and
+  // synchronously frees the old backing (Gigacage::free -> system free under
+  // Malloc=1). The native zlib handle still holds a pointer into it.
+  ab.transfer(1);
+
+  inf.write(payload, () => inf.end());
+`;
+
+const resetFixture = /* js */ `
+  const zlib = require("zlib");
+
+  const expected = Buffer.alloc(64, "a").toString();
+  const ab = new ArrayBuffer(4096);
+  const dict = Buffer.from(ab);
+  dict.fill("a");
+
+  const payload = zlib.deflateRawSync(Buffer.alloc(64, "a"), { dictionary: dict });
+
+  // INFLATERAW applies the dictionary synchronously in init(), and reset()
+  // re-applies it via setDictionary() — both read the stored pointer.
+  const inf = zlib.createInflateRaw({ dictionary: dict });
+  inf.on("error", err => {
+    console.error("error:", err.message);
+    process.exitCode = 1;
+  });
+  let out = Buffer.alloc(0);
+  inf.on("data", chunk => {
+    out = Buffer.concat([out, chunk]);
+  });
+  inf.on("end", () => {
+    console.log(out.toString() === expected ? "OK" : "WRONG: " + out.toString());
+  });
+
+  ab.transfer(1);
+
+  // reset() re-applies the dictionary from the stored (now stale) pointer.
+  inf.reset();
+  inf.write(payload, () => inf.end());
+`;
+
+const deflateResetFixture = /* js */ `
+  const zlib = require("zlib");
+
+  const expected = Buffer.alloc(64, "a").toString();
+  const ab = new ArrayBuffer(4096);
+  const dict = Buffer.from(ab);
+  dict.fill("a");
+  const dictCopy = Buffer.from(dict);
+
+  const def = zlib.createDeflate({ dictionary: dict });
+  def.on("error", err => {
+    console.error("error:", err.message);
+    process.exitCode = 1;
+  });
+  let out = Buffer.alloc(0);
+  def.on("data", chunk => {
+    out = Buffer.concat([out, chunk]);
+  });
+  def.on("end", () => {
+    const result = zlib.inflateSync(out, { dictionary: dictCopy }).toString();
+    console.log(result === expected ? "OK" : "WRONG: " + result);
+  });
+
+  ab.transfer(1);
+
+  // reset() calls deflateReset() then deflateSetDictionary() with the stored
+  // pointer on the JS thread.
+  def.reset();
+  def.end(Buffer.alloc(64, "a"));
+`;
+
+async function run(fixture: string) {
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "-e", fixture],
+    env,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  return { stdout, stderr, exitCode };
+}
+
+test.concurrent(
+  "inflate: detaching the dictionary ArrayBuffer after createInflate does not use-after-free",
+  async () => {
+    const { stdout, stderr, exitCode } = await run(inflateFixture);
+    expect(stderr).toBe("");
+    expect(stdout.trim()).toBe("OK");
+    expect(exitCode).toBe(0);
+  },
+);
+
+test.concurrent("inflateRaw: reset() after detaching the dictionary ArrayBuffer does not use-after-free", async () => {
+  const { stdout, stderr, exitCode } = await run(resetFixture);
+  expect(stderr).toBe("");
+  expect(stdout.trim()).toBe("OK");
+  expect(exitCode).toBe(0);
+});
+
+test.concurrent("deflate: reset() after detaching the dictionary ArrayBuffer does not use-after-free", async () => {
+  const { stdout, stderr, exitCode } = await run(deflateResetFixture);
+  expect(stderr).toBe("");
+  expect(stdout.trim()).toBe("OK");
+  expect(exitCode).toBe(0);
+});