blob: stop aliasing allocated content_type in dupeWithContentType

Merged
RO
6mo ago
+130-34

What

Blob.dupeWithContentType guarded its content_type handling on duped.isHeapAllocated() immediately after calling duped.setNotHeapAllocated(), so both branches were dead. When the source Blob's content_type is heap-allocated, the bitwise-copied dupe aliased the same allocation while both sides had content_type_allocated == true.

This is a regression from #23015: the pre-refactor code checked duped.allocator != null before clearing it at the end of the function; the refactor moved the clear to the top but left the (renamed) guard in place.

Repro

const file = Bun.file(path, { type: "application/x-custom-type-not-in-registry-abcdefghijklm" });
const response = new Response(file);               // body holds a dupe that aliases file.content_type
await file.write("hello", { type: "application/x-..." }); // frees file.content_type
response.headers.get("content-type");              // reads freed memory

On ASAN builds:

==716==ERROR: AddressSanitizer: use-after-poison on address 0x71df454301c0
    #1 in Zig::toStringCopy(ZigString) helpers.h:217
    #2 in WebCore__FetchHeaders__put bindings.cpp:2082
    #5 in bun.js.webcore.Response.getOrCreateHeaders Response.zig:358

On release builds the freed slot gets reused and the read produces garbage:

TypeError: Header '25' has invalid value: 'ion/x-custom-type-not-in-registry-abcdefghijklm'

Fix

Drop the isHeapAllocated() guard and always deep-copy an allocated content_type in dupeWithContentType. The old !include_content_type branch's "resolve to static mime or fall back to empty" is gone — it would have dropped FormData's multipart/form-data; boundary=... (and any non-registry type) on Response.clone(), and the branch itself was marked // TODO: fix this / this is a bug. The include_content_type parameter is now a no-op.

Since every dupe now owns its content_type copy, Blob.deinit() frees it. That in turn required closing a few places that held a bitwise-copied Blob alongside the live owner:

  • fromJSWithoutDeferGC move=true: deep-copy name/content_type into the moved-out value so the source JS Blob keeps sole ownership; the BuildArtifact arm now dupe()s (its "move" only nulled the store on a local copy).
  • getSliceFrom(): free the dupe's copy before overwriting it with the slice's own type.
  • doWrite/getWriter: clear content_type_allocated after the in-place free so a registry-resolved static string isn't later freed by deinit().
  • BlobOrStringOrBuffer.deinitAndUnprotect: only deref the store (matching its deinit()) since .blob is a raw view of a live JS Blob.

Verified

  • bun bd test test/js/web/fetch/blob.test.ts — 16/16 pass
  • New UAF test fails on both debug/ASAN (use-after-poison) and system bun (garbage header) without the fix, passes with it
  • New clone test guards against dropping FormData's boundary on Response.clone()
  • ASAN stress: 1k× Response.clone/blob.slice/createObjectURL+revoke/new Response([blob])/write({type}) — no double-free
  • RSS is flat across 50k × 1KB-type Response.clone() and blob.slice()