udp: root sendMany payloads in MarkedArgumentBuffer / reorder send() to prevent UAF by robobun · Pull Request #30057 · oven-sh/bun

robobun · 2026-05-01T18:14:31Z

What

UDPSocket.sendMany() and UDPSocket.send() both captured raw pointers into the payload's ArrayBuffer backing store (or borrowed WTFStringImpl storage for Latin-1 strings) and then hit JSC safepoints before handing those pointers to bsd_sendmmsg:

sendMany: subsequent loop iterations call iter.next() (slow path → JSObject.getIndex), coerceToInt32 on the port, and toBunString on the address
send: parseAddr calls coerceToInt32 on the port and toBunString on the address after the payload is captured

Any of these can run user JS that detaches an earlier payload's ArrayBuffer via .transfer(newLen) (which synchronously frees the old backing store) or drops the last reference to a JSString, leaving the captured pointer dangling.

Repro

const buf = new ArrayBuffer(4096);
const payload = new Uint8Array(buf);
const evilPort = {
  valueOf() {
    buf.transfer(0); // synchronously frees the 4096-byte backing store
    return server.port;
  },
};
client.sendMany([payload, evilPort, "127.0.0.1"]); // or client.send(payload, evilPort, "127.0.0.1")
// bsd_sendmmsg reads 4096 bytes from the freed region

Under ASAN (with Malloc=1 so bmalloc routes through the system heap):

==…==ERROR: AddressSanitizer: heap-use-after-free on address … at pc …
READ of size 4096 at … thread T0
    #0 … in read_iovec(…)
    #2 … in sendmmsg
    #3 … in bsd_sendmmsg packages/bun-usockets/src/bsd.c:123
freed by thread T0 here:
    …
    #14 … in JSC::arrayBufferCopyAndDetach(…) JSArrayBufferPrototype.cpp:365
    …
    #30 … in JSC::JSValue::toInt32(…)         ← parseAddr's coerceToInt32

Fix

sendMany: root every payload JSValue in a MarkedArgumentBuffer for the duration of the call and split the loop into two phases. Phase 1 collects/validates payload JSValues and runs all user-JS re-entrance (iter.next, parseAddr). Phase 2 borrows byte slices from the rooted JSValues once no more user JS sits between capture and socket.send. GC cannot collect a rooted payload; an ArrayBuffer that was detached during phase 1 reports a zero-length slice instead of a dangling pointer. No payload bytes are copied.
send: reorder so parseAddr runs before the payload pointer is captured. payload_arg stays rooted in the callframe, and nothing between capture and socket.send hits a JSC safepoint — so no copy is needed.

Verification

Without fix: bun bd test test/js/bun/udp/udp_socket.test.ts -t 'detaching an ArrayBuffer' → ASAN heap-use-after-free in read_iovec → bsd_sendmmsg for both send and sendMany, tests fail
With fix: both tests pass; received bytes match the original payload
Full test/js/bun/udp/ suite (207 tests) passes
zig:check-all passes on all targets

sendMany() captured raw pointers into each payload's ArrayBuffer backing store (or borrowed JSString storage) and then kept iterating the input array. Subsequent iterations hit JSC safepoints — iter.next()'s slow path, coerceToInt32 on the port, toBunString on the address — that can run user JS. That JS can detach an earlier payload's ArrayBuffer via transfer(n) (which synchronously frees the old backing store) or drop the last reference to a JSString, leaving payloads[] pointing at freed memory when it's handed to bsd_sendmmsg. Copy every payload into the per-call arena so the captured pointers are independent of JS heap lifetimes. For strings, toSlice already allocates into the arena when encoding conversion is needed; only dupe when it borrowed. Empty payloads use a static "" pointer rather than Zig's zero-length allocation sentinel, which the kernel rejects with EFAULT. The regression test spawns a fixture with Malloc=1 so bmalloc routes ArrayBuffer allocations through the system heap, making the free visible to ASAN in sanitizer builds; release builds fall through and verify the received bytes match the original payload.

coderabbitai · 2026-05-01T18:14:38Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e5495f1b-2fd0-47d3-a9e4-bf0a97079db2

📥 Commits

Reviewing files that changed from the base of the PR and between 6324ccb and 3cee342.

📒 Files selected for processing (2)

src/bun.js/api/bun/udp_socket.zig
test/js/bun/udp/udp_socket.test.ts

Walkthrough

sendMany and send were refactored to root JS payload values and split processing into two phases: a JS-visible validation/coercion phase that roots payloads and resolves addresses, then a native-only phase that borrows raw byte slices and builds iovecs. New regression fixture and tests exercise detachment during coercion.

Changes

UDP socket payload handling + regression test

Layer / File(s)	Summary
Rooting / control flow `src/bun.js/api/bun/udp_socket.zig`	`sendMany` now runs inside a `jsc.MarkedArgumentBuffer` and delegates to a new `sendManyImpl` that accepts rooted payload storage.
Phase 1 — validation & coercion `src/bun.js/api/bun/udp_socket.zig`	`sendManyImpl` first iterates inputs, validates entries, coerces boxed strings to primitive `JSString` via `toJSString`, resolves per-packet port/address, and stores rooted `JSValue`s without deriving raw slices.
Phase 2 — borrow bytes after JS safe point `src/bun.js/api/bun/udp_socket.zig`	After JS re-entry is closed, implementation borrows `[]const u8` slices once per payload to populate `payloads`/`lens`/`addr_ptrs`; detached `ArrayBufferView`s are treated as empty via a static `""` slice.
Ordering change for single send `src/bun.js/api/bun/udp_socket.zig`	`send` now calls `parseAddr` before deriving payload bytes and uses `toJSString` for boxed-string payloads to avoid pointer invalidation during address coercion.
Regression fixture `test/js/bun/udp/sendMany-payload-uaf-fixture.ts`	Adds a fixture that detaches a shared `ArrayBuffer` during port/address coercion using `evilPort.valueOf()` and `EvilString.toString()`, exercising `sendMany`/`send` paths and retrying until expected datagram is received.
Automated test `test/js/bun/udp/udp_socket.test.ts`	Adds tests: one for boxed `String` payloads and one that spawns the fixture in modes `sendMany`, `sendMany-stringobj`, and `send`; enables `Malloc` on non-Windows, filters ASAN-interference stderr lines, and asserts `stdout === "OK\n"` and empty stderr.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically describes the main changes: rooting sendMany payloads in MarkedArgumentBuffer and reordering send() to prevent use-after-free bugs.
Description check	✅ Passed	The description comprehensively covers what the PR does, includes detailed repro steps, explains the fix approach, and provides verification methods beyond the required template sections.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Review rate limit: 4/5 reviews remaining, refill in 12 minutes.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

robobun · 2026-05-01T18:14:42Z

^{Updated 3:05 AM PT - May 2nd, 2026}

❌ @robobun, your commit 3cee342 has 3 failures in Build #50088 (All Failures):

test/cli/run/require-cache.test.ts - code 1 on 🍎 14 aarch64
test/cli/run/no-orphans.test.ts - timeout on 🍎 14 x64

Details

test/cli/run/no-orphans.test.ts - timeout on 🍎 26 aarch64

Details

test/cli/run/no-orphans.test.ts - timeout on 🍎 14 aarch64

test/bake/dev-and-prod.test.ts - code 1 on 🪟 11 aarch64

🧪 To try this PR locally:

bunx bun-pr 30057

That installs a local version of the PR into your bun-30057 executable, so you can run:

bun-30057 --bun

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/js/bun/udp/sendMany-payload-uaf-fixture.ts`:
- Around line 31-56: Add two sibling test cases alongside the existing
ArrayBuffer detachment case in sendMany-payload-uaf-fixture.ts to cover the
borrowed-string and empty-payload branches: create an accessor-returned ASCII
string payload (e.g., an object with a valueOf() or toString() that returns a
short ASCII JS string) and pass it to client.sendMany in the same triple shape
used for the ArrayBuffer test so the borrowed-string path in the native send is
exercised; also add a zero-length payload case (e.g., a Uint8Array of length 0
or empty string) and call client.sendMany with that empty payload to exercise
the empty-payload/static-"" branch. Ensure both new cases follow the same
pattern as the existing test (coercion via valueOf/toString where appropriate)
and verify they do not crash or misbehave after sendMany returns.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5434e856-8861-4b45-afe6-585155a8fe45

📥 Commits

Reviewing files that changed from the base of the PR and between a6685d0 and 755dec2.

📒 Files selected for processing (3)

src/bun.js/api/bun/udp_socket.zig
test/js/bun/udp/sendMany-payload-uaf-fixture.ts
test/js/bun/udp/udp_socket.test.ts

Same UAF as sendMany(): send() captured a raw pointer into the payload's ArrayBuffer backing store (or borrowed JSString storage), then called parseAddr() which runs coerceToInt32 on the port and toBunString on the address — both JSC safepoints that can execute user JS. A port object's valueOf() that calls buf.transfer(0) frees the backing store before socket.send() reads from it. Fix by reordering: resolve the destination first, then capture the payload. payload_arg stays rooted in the callframe, and after capture nothing runs user JS before socket.send(). If the buffer is detached during parseAddr the payload simply reflects the detached (empty) state, matching the pre-existing behavior of send(alreadyDetachedView). Also: - Extend the UAF fixture to cover both send and sendMany modes. - Add a sendRec retry loop so a dropped UDP packet on a loaded CI host doesn't hang the fixture until the 30s harness timeout.

robobun · 2026-05-01T19:30:10Z

Got a duplicate request for this one and went with a slightly different angle before spotting this PR — pushed to farm/482d2efe/udp-sendmany-root-payloads in case it's useful:

Roots each payload JSValue in a MarkedArgumentBuffer (via MarkedArgumentBuffer.run) and splits the loop into two phases: phase 1 collects/validates payload JSValues + runs all user JS (iter.next, parseAddr); phase 2 borrows byte slices once no more user JS sits between capture and socket.send(). Avoids copying the payload bytes.
Same Malloc=1 + ab.transfer() ASAN repro; gate verified (fails without fix with heap-use-after-free in sendmmsg, passes with).

Happy to fold any of it in here or drop it — this PR already covers the bug.

In send mode, after the reorder, the payload is captured from the now-detached view (length 0). On Linux socket.send() with the resulting empty slice surfaces as EFAULT and nothing is sent; on Windows it succeeds and a 0-byte packet arrives before the retry loop delivers the real 4096-byte payload, failing the size check. Filter out non-size packets in the data handler so both platforms settle on the retry packet. The ASAN regression guard is unaffected — a heap-use-after-free aborts the process before any packet arrives.

Jarred-Sumner · 2026-05-01T21:28:53Z

Do not copy. Use a MarkedArgumentsBuffer to copy the JSValue (you can store the jsString there)

then iterate on the JSValue there

Per review: rather than duping every payload into the arena, root each payload JSValue in a MarkedArgumentBuffer for the duration of the call and split the loop into two phases. Phase 1 iterates the input array, validates each payload is an ArrayBufferView or string, roots it, and runs all user-JS re-entrance (iter.next's slow path, parseAddr's coerceToInt32 / toBunString). Phase 2 borrows byte slices from the rooted JSValues once no more user JS sits between capture and socket.send. GC cannot collect a rooted payload; an ArrayBuffer that was detached during phase 1 now reports a zero-length slice (substituted with a valid static pointer so sendmmsg doesn't EFAULT on Zig's empty-slice sentinel). No payload bytes are copied.

robobun · 2026-05-01T21:59:23Z

Switched to the MarkedArgumentBuffer approach in 28e3992 / a4a7cb1 — no more byte copying:

Phase 1 iterates the array, roots each payload JSValue (payload_roots.append(val)), validates ArrayBufferView/string, and runs all user JS (iter.next, parseAddr).
Phase 2 borrows byte slices from the rooted JSValues. No user JS runs between capture and socket.send; rope resolution / UTF-16 conversion may GC, but every payload is rooted so borrowed WTFStringImpl/backing-store pointers stay valid. ArrayBuffers detached during phase 1 are observed as zero-length (valid static pointer substituted so sendmmsg doesn't EFAULT on Zig's empty-slice sentinel).

send() keeps the simple reorder (resolve destination before capturing payload) since the callframe already roots the single payload.

All 207 test/js/bun/udp/ tests pass; zig:check-all green.

…dmany-payload-uaf

…hase 1 Bun's JSValue.isString() is jsType().isStringLike(), which accepts StringObject and DerivedStringObject in addition to primitive JSString. Calling toJSString() on a boxed/derived String object goes through toStringSlowCase -> toPrimitive and invokes user-defined toString()/ valueOf()/Symbol.toPrimitive — so phase 2 could still run user JS: class Evil extends String { toString() { buf.transfer(0); ... } } client.sendMany([view, port, addr, new Evil('x'), port, addr]); Phase 2 would capture payloads[0] from the live view, then the second payload's toString() frees buf's backing store, and bsd_sendmmsg reads freed memory — the same UAF this PR is closing. Resolve string-like payloads to primitive JSStrings in phase 1 (where user-JS re-entrance is expected and no pointers have been captured) and root the result. Phase 2 then uses asString() — a plain cast, no toPrimitive — so it genuinely cannot run user JS. Adds a sendMany-stringobj fixture mode covering this path.

Same isString()/asString() mismatch just fixed in sendMany: Bun's isString() is isStringLike() and accepts StringObject/ DerivedStringObject, but asString() is a raw static_cast<JSString*> guarded only by a debug ASSERT on primitive StringType. client.send(new String('x'), port, addr) → debug ASSERTION FAILED: value.isStringSlow(), release type-confusion in toSlice. Use toJSString() instead, which resolves boxed strings via toPrimitive. Safe because parseAddr has already run (there is only one payload so toPrimitive cannot invalidate an earlier captured pointer) and the 'this.socket orelse throw' that follows handles close-during-toPrimitive. Adds a subprocess test covering StringObject and DerivedStringObject payloads for both send() and sendMany().

…to prevent UAF (oven-sh#30057) ## What `UDPSocket.sendMany()` and `UDPSocket.send()` both captured raw pointers into the payload's ArrayBuffer backing store (or borrowed `WTFStringImpl` storage for Latin-1 strings) and then hit JSC safepoints before handing those pointers to `bsd_sendmmsg`: - **`sendMany`**: subsequent loop iterations call `iter.next()` (slow path → `JSObject.getIndex`), `coerceToInt32` on the port, and `toBunString` on the address - **`send`**: `parseAddr` calls `coerceToInt32` on the port and `toBunString` on the address after the payload is captured Any of these can run user JS that detaches an earlier payload's ArrayBuffer via `.transfer(newLen)` (which synchronously frees the old backing store) or drops the last reference to a JSString, leaving the captured pointer dangling. ## Repro ```js const buf = new ArrayBuffer(4096); const payload = new Uint8Array(buf); const evilPort = { valueOf() { buf.transfer(0); // synchronously frees the 4096-byte backing store return server.port; }, }; client.sendMany([payload, evilPort, "127.0.0.1"]); // or client.send(payload, evilPort, "127.0.0.1") // bsd_sendmmsg reads 4096 bytes from the freed region ``` Under ASAN (with `Malloc=1` so bmalloc routes through the system heap): ``` ==…==ERROR: AddressSanitizer: heap-use-after-free on address … at pc … READ of size 4096 at … thread T0 #0 … in read_iovec(…) oven-sh#2 … in sendmmsg oven-sh#3 … in bsd_sendmmsg packages/bun-usockets/src/bsd.c:123 freed by thread T0 here: … oven-sh#14 … in JSC::arrayBufferCopyAndDetach(…) JSArrayBufferPrototype.cpp:365 … oven-sh#30 … in JSC::JSValue::toInt32(…) ← parseAddr's coerceToInt32 ``` ## Fix - **`sendMany`**: root every payload JSValue in a `MarkedArgumentBuffer` for the duration of the call and split the loop into two phases. Phase 1 collects/validates payload JSValues and runs all user-JS re-entrance (`iter.next`, `parseAddr`). Phase 2 borrows byte slices from the rooted JSValues once no more user JS sits between capture and `socket.send`. GC cannot collect a rooted payload; an ArrayBuffer that was detached during phase 1 reports a zero-length slice instead of a dangling pointer. No payload bytes are copied. - **`send`**: reorder so `parseAddr` runs before the payload pointer is captured. `payload_arg` stays rooted in the callframe, and nothing between capture and `socket.send` hits a JSC safepoint — so no copy is needed. ## Verification - **Without fix:** `bun bd test test/js/bun/udp/udp_socket.test.ts -t 'detaching an ArrayBuffer'` → ASAN heap-use-after-free in `read_iovec` → `bsd_sendmmsg` for both `send` and `sendMany`, tests fail - **With fix:** both tests pass; received bytes match the original payload - Full `test/js/bun/udp/` suite (207 tests) passes - `zig:check-all` passes on all targets --------- Co-authored-by: robobun <robobun@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

github-actions Bot added the claude label May 1, 2026

[autofix.ci] apply automated fixes

755dec2

coderabbitai Bot reviewed May 1, 2026

View reviewed changes

Comment thread test/js/bun/udp/sendMany-payload-uaf-fixture.ts Outdated

claude Bot reviewed May 1, 2026

View reviewed changes

Comment thread test/js/bun/udp/sendMany-payload-uaf-fixture.ts Outdated

Comment thread src/bun.js/api/bun/udp_socket.zig Outdated

robobun changed the title ~~udp: copy sendMany payloads into arena to prevent use-after-free~~ udp: copy sendMany payloads / reorder send() to prevent use-after-free May 1, 2026

claude Bot reviewed May 1, 2026

View reviewed changes

Comment thread test/js/bun/udp/sendMany-payload-uaf-fixture.ts

robobun added 2 commits May 1, 2026 21:54

test(udp): update comments for MarkedArgumentBuffer approach

a4a7cb1

robobun changed the title ~~udp: copy sendMany payloads / reorder send() to prevent use-after-free~~ udp: root sendMany payloads in MarkedArgumentBuffer / reorder send() to prevent UAF May 1, 2026

Merge remote-tracking branch 'origin/main' into farm/daabf1e4/udp-sen…

a01bdfc

…dmany-payload-uaf

claude Bot reviewed May 2, 2026

View reviewed changes

Comment thread src/bun.js/api/bun/udp_socket.zig Outdated

claude Bot reviewed May 2, 2026

View reviewed changes

Comment thread src/bun.js/api/bun/udp_socket.zig

Jarred-Sumner merged commit 9aa7a4d into main May 3, 2026
73 of 77 checks passed

Jarred-Sumner deleted the farm/daabf1e4/udp-sendmany-payload-uaf branch May 3, 2026 00:28

claude Bot mentioned this pull request May 3, 2026

Fix crash in Blob.writer() when options parsing returns an error #28736

Closed

Navigation Menu

udp: root sendMany payloads in MarkedArgumentBuffer / reorder send() to prevent UAF - #30057

udp: root sendMany payloads in MarkedArgumentBuffer / reorder send() to prevent UAF#30057
Jarred-Sumner merged 9 commits into
mainfrom
farm/daabf1e4/udp-sendmany-payload-uaf

robobun commented May 1, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 1, 2026 •

edited

Loading

Uh oh!

robobun commented May 1, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robobun commented May 1, 2026

Uh oh!

Uh oh!

Jarred-Sumner commented May 1, 2026

Uh oh!

robobun commented May 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

robobun commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Repro

Fix

Verification

Uh oh!

coderabbitai Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

robobun commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robobun commented May 1, 2026

Uh oh!

Uh oh!

Jarred-Sumner commented May 1, 2026

Uh oh!

robobun commented May 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented May 1, 2026 •

edited

Loading

coderabbitai Bot commented May 1, 2026 •

edited

Loading

robobun commented May 1, 2026 •

edited

Loading