fix(buffer): prevent heap read-after-free in indexOf via valueOf-triggered detachment by robobun · Pull Request #26927 · oven-sh/bun

robobun · 2026-02-12T04:38:49Z

Summary

Fix a heap read-after-free vulnerability in Buffer.indexOf, Buffer.lastIndexOf, and Buffer.includes where the raw typedVector pointer was cached before calling toNumber() on the byteOffset argument. A user-supplied valueOf() callback could call ArrayBuffer.prototype.transfer() to detach the underlying ArrayBuffer, freeing the original memory, causing these methods to scan freed heap data.
The fix defers fetching the typedVector pointer until after toNumber() completes, adds a detachment check, and throws a TypeError if the buffer was detached.

Test plan

New test file test/js/node/buffer-indexOf-detach.test.ts with 7 test cases:
- indexOf throws TypeError when buffer detached via valueOf
- lastIndexOf throws TypeError when buffer detached via valueOf
- includes throws TypeError when buffer detached via valueOf
- indexOf with string value throws TypeError when buffer detached
- indexOf with Buffer value throws TypeError when buffer detached
- Normal indexOf/lastIndexOf/includes functionality still works
- indexOf with non-detaching valueOf still works correctly
All 7 tests pass with bun bd test
The 5 detachment tests fail with USE_SYSTEM_BUN=1 (confirming they test the fix)

🤖 Generated with Claude Code

claude · 2026-02-12T04:38:58Z

🤖 Claude is monitoring this PR

I'll review code, respond to comments, and investigate CI failures.
Start any comment with (skip) to have me ignore it.

Push fix commits (uncheck to only post review comments)

robobun · 2026-02-12T04:38:58Z

^{Updated 12:22 AM PT - Mar 2nd, 2026}

❌ @Jarred-Sumner, your commit 268745d has 3 failures in Build #38508 (All Failures):

test/integration/next-pages/test/dev-server.test.ts - code 1 on 🍎 14 aarch64
test/js/node/watch/fs.watchFile.test.ts - pid 2104 segmentation fault on 🪟 11 aarch64
test/integration/vite-build/vite-build.test.ts - code 1 on 🪟 11 aarch64
test/integration/vite-build/vite-build.test.ts - code 1 on 🪟 2019 x64-baseline

🧪 To try this PR locally:

bunx bun-pr 26927

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

bun-26927 --bun

coderabbitai · 2026-02-12T04:45:30Z

Walkthrough

Refreshes Buffer backing-pointer and length after any JS-triggering operation in indexOf paths to detect ArrayBuffer detachment mid-operation; on detection, short-circuits to the appropriate empty-match or not-found results (e.g., -1 for indexOf/lastIndexOf, false for includes) and updates needle handling accordingly.

Changes

Cohort / File(s)	Summary
Buffer indexOf detachment resilience `src/bun.js/bindings/JSBuffer.cpp`	Adds a helper lambda to re-fetch `typedVector()` and `byteLength` after JS-executing steps, checks for detachment and zero-length buffers, and updates number/string/array needle paths to re-validate buffer state before continuing. Handles detached needles with empty-match semantics and avoids using stale pointers.
Detachment behavior tests `test/js/node/buffer-indexOf-detach.test.ts`	Adds tests exercising buffer/needle detachment via `valueOf`/`toString` callbacks, asserting indexOf/lastIndexOf return -1 and includes returns false for detached haystacks, and verifying correct behavior for detached needles and non-detaching cases.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing a heap read-after-free vulnerability in Buffer.indexOf via valueOf-triggered detachment.
Description check	✅ Passed	The description includes all required template sections: a comprehensive 'Summary' explaining what the fix does, and a detailed 'Test plan' describing verification. The description is complete and well-structured.

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

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

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/bun.js/bindings/JSBuffer.cpp`:
- Around line 1507-1526: The code currently caches buffer->typedVector() and
buffer->byteLength() before calling encodingValue.toWTFString()/other JS
conversions, leaving a UAF if those conversions detach the ArrayBuffer; update
the logic in the JSBuffer functions (where encodingValue.toWTFString() and
subsequent string/encoding normalization happen) to re-check
buffer->isDetached() and re-fetch typedVector and byteLength after the encoding
normalization (and any call that can run JS) and before any use in
indexOfString/indexOfBuffer, throwing the same VMTypeError on detachment and
returning -1 as currently done.

robobun · 2026-02-12T04:53:10Z

Addressed the review feedback in 760c32c: the typedVector pointer and byteLength are now re-fetched (with a detachment check) right before each use site (indexOfNumber, indexOfString, indexOfBuffer) rather than once after toNumber(). This covers all JS execution points including encodingValue.toWTFString() and valueValue.toStringOrNull(). Added 2 new tests for detachment via toString() on the encoding argument.

claude · 2026-02-12T04:56:11Z

Code Review

Newest first

✅ 760c3 — No issues found

🟣 f003f — 1 issue found

Issue	File
🟣 `toWTFString()` on encoding arg can also trigger detachment after pointer is cached	`src/bun.js/bindings/JSBuffer.cpp`

claude · 2026-02-12T04:57:09Z

🟣 Pre-existing issue in `src/bun.js/bindings/JSBuffer.cpp:1537`

encodingValue.toWTFString() can execute arbitrary JavaScript via a user-defined toString() on the encoding argument, which could detach the ArrayBuffer after typedVector is cached on line 1522. This is the same class of UAF being fixed for toNumber(), but through a different argument. Not introduced by this PR — the vulnerability existed before — but worth addressing in a follow-up.

Conceptual exploit bypassing the current fix:

const buf = Buffer.alloc(64, 'A');
buf.indexOf("A", 0, { toString() { buf.buffer.transfer(); return "utf8"; } });

claude · 2026-02-12T04:57:25Z

No description provided.

claude · 2026-02-12T05:25:20Z

test

claude · 2026-02-12T05:25:37Z

No description provided.

claude · 2026-02-12T05:26:13Z

No description provided.

Jarred-Sumner

Do not fetch buffer state multiple times. Only once.

robobun

The concern about caching typedVector()/byteLength() before toWTFString()/toStringOrNull() was addressed in commit 760c32c. The current code:

Never caches typedVector early — it's only fetched through refetchBufferState after all JS-calling functions complete.
Re-fetches byteLength by reference via refetchBufferState before any memory access.
Calls refetchBufferState in all three paths (number, string, buffer) after the last JS-calling function and before passing pointers to the helper functions (indexOfString/indexOfBuffer/indexOfNumber).

Specifically for the string path: encodingValue.toWTFString() (line 1546) and valueValue.toStringOrNull() (line 1557) both run before refetchBufferState at line 1560, which checks isDetached() and re-fetches both typedVector and byteLength before calling indexOfString. Same pattern for the buffer path at line 1568.

…gered detachment The indexOf function (shared by Buffer.indexOf, Buffer.lastIndexOf, and Buffer.includes) cached the raw typedVector pointer and byteLength before calling toNumber() on the byteOffset argument. A user-supplied valueOf() callback could call ArrayBuffer.prototype.transfer() to detach the underlying ArrayBuffer, freeing the original memory. The stale pointer was then used to scan freed memory. Fix: defer fetching typedVector until after the toNumber() call and add a detachment check. If the buffer was detached, throw a TypeError. Co-Authored-By: Claude <noreply@anthropic.com>

…n indexOf Re-fetch the buffer pointer and length right before each use site (indexOfNumber, indexOfString, indexOfBuffer) rather than once after toNumber(). This covers additional JS execution points: the encodingValue.toWTFString() and valueValue.toStringOrNull() calls could also trigger detachment via toString() callbacks. Co-Authored-By: Claude <noreply@anthropic.com>

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/node/buffer-indexOf-detach.test.ts`:
- Around line 1-2: The test file currently imports describe and test from
"node:test"; change that import to pull describe and test from "bun:test"
instead (i.e., replace the import line that references "node:test" with one that
references "bun:test") while keeping the existing assert import unchanged so the
test harness uses bun:test as required by the repo policy.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6442e36 and 268745d.

📒 Files selected for processing (2)

src/bun.js/bindings/JSBuffer.cpp
test/js/node/buffer-indexOf-detach.test.ts

robobun

This is a false positive — the file already imports describe, expect, and test from "bun:test" on line 1:

import { describe, expect, test } from "bun:test";

There is no "node:test" import. No changes needed.

robobun

Needle detachment is now handled in commit 268745d. When the needle buffer is detached, it is treated as an empty buffer (matching Node.js behavior — returns the computed byteOffset for a zero-length match). The check is at lines 1589-1595 in JSBuffer.cpp, before calling indexOfBuffer. Four new test cases cover this scenario (indexOf, lastIndexOf, encoding toString, nonzero byteOffset).

…gered detachment (oven-sh#26927) ## Summary - Fix a heap read-after-free vulnerability in `Buffer.indexOf`, `Buffer.lastIndexOf`, and `Buffer.includes` where the raw `typedVector` pointer was cached before calling `toNumber()` on the `byteOffset` argument. A user-supplied `valueOf()` callback could call `ArrayBuffer.prototype.transfer()` to detach the underlying `ArrayBuffer`, freeing the original memory, causing these methods to scan freed heap data. - The fix defers fetching the `typedVector` pointer until after `toNumber()` completes, adds a detachment check, and throws a `TypeError` if the buffer was detached. ## Test plan - [x] New test file `test/js/node/buffer-indexOf-detach.test.ts` with 7 test cases: - `indexOf` throws `TypeError` when buffer detached via `valueOf` - `lastIndexOf` throws `TypeError` when buffer detached via `valueOf` - `includes` throws `TypeError` when buffer detached via `valueOf` - `indexOf` with string value throws `TypeError` when buffer detached - `indexOf` with Buffer value throws `TypeError` when buffer detached - Normal `indexOf`/`lastIndexOf`/`includes` functionality still works - `indexOf` with non-detaching `valueOf` still works correctly - [x] All 7 tests pass with `bun bd test` - [x] The 5 detachment tests fail with `USE_SYSTEM_BUN=1` (confirming they test the fix) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>

…via valueOf Buffer#copy captured source/target byteLength before calling toInteger on targetStart/sourceStart/sourceEnd, and Buffer#fill captured typedVector+length before calling toInt32/toString on value (and toString on the encoding arg). Each of those coercions invokes user-supplied valueOf / Symbol.toPrimitive / toString, which may call ArrayBuffer.prototype.transfer (detach -> vector() returns nullptr -> segfault in memmove/memset) or resize a resizable ArrayBuffer (pointer stays valid but logical length shrinks -> memmove reads past the post-resize boundary that JS normally enforces on the TypedArray API, leaking buffer contents through the copy target). Fix: re-fetch isDetached()/byteLength() (and read vector()/typedVector()) just before the memmove / memset in both functions, and clamp the copy window against the current logical length. On detach, silently no-op (return 0 from copy, return the same buffer from fill) to match Node.js. Same bug class previously patched for indexOf/lastIndexOf/includes in #26927. Fixes #29730.

…via valueOf (#29731) Fixes #29730 ## Repro ```js // 1. DoS via transfer in copy valueOf const s = Buffer.alloc(1024), t = Buffer.alloc(1024); s.copy(t, 0, { valueOf() { s.buffer.transfer(0); return 0; } }); // → SEGV at 0x0 in __sanitizer_internal_memmove // 2. OOB read via resize in copy valueOf const rab = new ArrayBuffer(1024, { maxByteLength: 1024 }); const src = Buffer.from(rab); src.fill(0xAB); const dst = Buffer.alloc(1024); src.copy(dst, { valueOf() { rab.resize(10); return 0; } }, 0, 1024); // → 1014 bytes past the post-resize logical end copied into dst // 3. DoS via transfer in fill valueOf const b = Buffer.alloc(100); b.fill({ valueOf() { b.buffer.transfer(0); return 0x42; } }); // → SEGV at 0x0 in memset ``` ## Cause `jsBufferPrototypeFunction_copyBody` captured `source->byteLength()` and `target->byteLength()` on entry, then called `toInteger()` on each of the three numeric args. `toInteger` calls `toNumber`, which invokes `valueOf`/`Symbol.toPrimitive`. That callback could: - `ArrayBuffer.prototype.transfer(0)` → detach; `vector()` returns nullptr → segfault inside `memmove`. - `ArrayBuffer.prototype.resize(smaller)` → pointer stays mapped but the logical length shrinks; the captured `sourceLength` wins the bounds check and `memmove` reads past the post-resize end that JS normally hides. `jsBufferPrototypeFunction_fillBody` had the same shape: `typedVector()` was read after `value.toInt32()` / `value.toWTFString()` / `parseEncoding()` could have detached or resized the backing store. Same bug class was patched for `indexOf`/`lastIndexOf`/`includes` in #26927; the fix was never extended to `copy`/`fill`. ## Fix - **copy**: keep the pre-coercion lengths for the user-facing `ERR_OUT_OF_RANGE` bounds checks (so reporting matches the user's mental model), but right before `memmove` re-check `isDetached()`, re-read the current `byteLength()` of both buffers, and clamp `nb` against those current lengths. Detached → return 0 (matches Node.js). - **fill**: add a `clampRangeAfterSideEffect()` helper that runs after each JS-visible coercion and clamps `[offset, end)` to the post-coercion `byteLength()`, short-circuiting to `return this` on detach. Call it after `parseEncoding`, after `value.toWTFString()` in the string branch, and after `value.toInt32()` in the number branch. The `typedVector()` read is pushed down to immediately before `memset`/`Bun__Buffer_fill`. ## Verification - `bun bd test test/js/node/buffer-copy-fill-detach.test.ts` — 17/17 pass (debug + ASAN). - `USE_SYSTEM_BUN=1 bun test …` on the new file — crashes on first test (confirms the test exercises the fix). - `bun bd` against the original 3 PoC scripts — all three survive and produce the same output as Node.js. - No regression in `test-buffer-{copy,fill,indexof,includes}.js` or `buffer-indexOf-detach.test.ts`. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>

…gered detachment (oven-sh#26927) ## Summary - Fix a heap read-after-free vulnerability in `Buffer.indexOf`, `Buffer.lastIndexOf`, and `Buffer.includes` where the raw `typedVector` pointer was cached before calling `toNumber()` on the `byteOffset` argument. A user-supplied `valueOf()` callback could call `ArrayBuffer.prototype.transfer()` to detach the underlying `ArrayBuffer`, freeing the original memory, causing these methods to scan freed heap data. - The fix defers fetching the `typedVector` pointer until after `toNumber()` completes, adds a detachment check, and throws a `TypeError` if the buffer was detached. ## Test plan - [x] New test file `test/js/node/buffer-indexOf-detach.test.ts` with 7 test cases: - `indexOf` throws `TypeError` when buffer detached via `valueOf` - `lastIndexOf` throws `TypeError` when buffer detached via `valueOf` - `includes` throws `TypeError` when buffer detached via `valueOf` - `indexOf` with string value throws `TypeError` when buffer detached - `indexOf` with Buffer value throws `TypeError` when buffer detached - Normal `indexOf`/`lastIndexOf`/`includes` functionality still works - `indexOf` with non-detaching `valueOf` still works correctly - [x] All 7 tests pass with `bun bd test` - [x] The 5 detachment tests fail with `USE_SYSTEM_BUN=1` (confirming they test the fix) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>

…via valueOf (oven-sh#29731) Fixes oven-sh#29730 ## Repro ```js // 1. DoS via transfer in copy valueOf const s = Buffer.alloc(1024), t = Buffer.alloc(1024); s.copy(t, 0, { valueOf() { s.buffer.transfer(0); return 0; } }); // → SEGV at 0x0 in __sanitizer_internal_memmove // 2. OOB read via resize in copy valueOf const rab = new ArrayBuffer(1024, { maxByteLength: 1024 }); const src = Buffer.from(rab); src.fill(0xAB); const dst = Buffer.alloc(1024); src.copy(dst, { valueOf() { rab.resize(10); return 0; } }, 0, 1024); // → 1014 bytes past the post-resize logical end copied into dst // 3. DoS via transfer in fill valueOf const b = Buffer.alloc(100); b.fill({ valueOf() { b.buffer.transfer(0); return 0x42; } }); // → SEGV at 0x0 in memset ``` ## Cause `jsBufferPrototypeFunction_copyBody` captured `source->byteLength()` and `target->byteLength()` on entry, then called `toInteger()` on each of the three numeric args. `toInteger` calls `toNumber`, which invokes `valueOf`/`Symbol.toPrimitive`. That callback could: - `ArrayBuffer.prototype.transfer(0)` → detach; `vector()` returns nullptr → segfault inside `memmove`. - `ArrayBuffer.prototype.resize(smaller)` → pointer stays mapped but the logical length shrinks; the captured `sourceLength` wins the bounds check and `memmove` reads past the post-resize end that JS normally hides. `jsBufferPrototypeFunction_fillBody` had the same shape: `typedVector()` was read after `value.toInt32()` / `value.toWTFString()` / `parseEncoding()` could have detached or resized the backing store. Same bug class was patched for `indexOf`/`lastIndexOf`/`includes` in oven-sh#26927; the fix was never extended to `copy`/`fill`. ## Fix - **copy**: keep the pre-coercion lengths for the user-facing `ERR_OUT_OF_RANGE` bounds checks (so reporting matches the user's mental model), but right before `memmove` re-check `isDetached()`, re-read the current `byteLength()` of both buffers, and clamp `nb` against those current lengths. Detached → return 0 (matches Node.js). - **fill**: add a `clampRangeAfterSideEffect()` helper that runs after each JS-visible coercion and clamps `[offset, end)` to the post-coercion `byteLength()`, short-circuiting to `return this` on detach. Call it after `parseEncoding`, after `value.toWTFString()` in the string branch, and after `value.toInt32()` in the number branch. The `typedVector()` read is pushed down to immediately before `memset`/`Bun__Buffer_fill`. ## Verification - `bun bd test test/js/node/buffer-copy-fill-detach.test.ts` — 17/17 pass (debug + ASAN). - `USE_SYSTEM_BUN=1 bun test …` on the new file — crashes on first test (confirms the test exercises the fix). - `bun bd` against the original 3 PoC scripts — all three survive and produce the same output as Node.js. - No regression in `test-buffer-{copy,fill,indexof,includes}.js` or `buffer-indexOf-detach.test.ts`. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>

github-actions Bot added the claude label Feb 12, 2026

coderabbitai Bot reviewed Feb 12, 2026

View reviewed changes

Comment thread src/bun.js/bindings/JSBuffer.cpp

Jarred-Sumner requested changes Feb 12, 2026

View reviewed changes

robobun commented Mar 1, 2026

View reviewed changes

Claude Bot and others added 2 commits March 1, 2026 11:18

robobun force-pushed the claude/fix-buffer-indexof-uaf branch from 760c32c to 6442e36 Compare March 1, 2026 11:25

claude Bot reviewed Mar 1, 2026

View reviewed changes

Comment thread src/bun.js/bindings/JSBuffer.cpp

Address comments + make test run in node

268745d

coderabbitai Bot reviewed Mar 2, 2026

View reviewed changes

Comment thread test/js/node/buffer-indexOf-detach.test.ts

robobun commented Mar 2, 2026

View reviewed changes

Jarred-Sumner merged commit 616501d into main Mar 2, 2026
34 of 45 checks passed

Jarred-Sumner deleted the claude/fix-buffer-indexof-uaf branch March 2, 2026 07:44

robobun commented Mar 2, 2026

View reviewed changes

robobun mentioned this pull request Mar 18, 2026

Bun Crashed in ECS #21347

Closed

robobun mentioned this pull request Apr 26, 2026

fix(buffer): prevent TOCTOU crash/OOB in Buffer#copy and Buffer#fill via valueOf #29731

Merged

VenkatKwest mentioned this pull request May 9, 2026

TOCTOU Vulnerability in Buffer.prototype.write #30417

Open

Conversation

robobun commented Feb 12, 2026

Summary

Test plan

Uh oh!

claude Bot commented Feb 12, 2026

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robobun commented Feb 12, 2026

Uh oh!

claude Bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Uh oh!

claude Bot commented Feb 12, 2026

🟣 Pre-existing issue in src/bun.js/bindings/JSBuffer.cpp:1537

Uh oh!

claude Bot commented Feb 12, 2026

Uh oh!

claude Bot commented Feb 12, 2026

Uh oh!

claude Bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Feb 12, 2026

Uh oh!

Jarred-Sumner left a comment

Choose a reason for hiding this comment

Uh oh!

robobun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robobun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robobun left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Feb 12, 2026 •

edited

Loading

coderabbitai Bot commented Feb 12, 2026 •

edited

Loading

claude Bot commented Feb 12, 2026 •

edited

Loading

🟣 Pre-existing issue in `src/bun.js/bindings/JSBuffer.cpp:1537`

claude Bot commented Feb 12, 2026 •

edited

Loading