Decode a private copy of a JS buffer in the UTF-8 decoders

RO
1mo ago
+278-39

Problem

  • Buffer.from(sab).toString('utf8') segfaults a release build while a worker rewrites the buffer: panic(main thread): Segmentation fault at address 0x0, 5 of 5 runs. So does an ordinary Buffer that an async fs.read is filling. node v26.3.0 survives both.
  • The decoder reads its input twice: one pass sizes the output, then convert_utf8_to_utf16le_with_errors writes it. A byte that changes in between makes the write overrun the reservation. ASAN reports heap-buffer-overflow in to_utf16_alloc (src/bun_core/string/immutable.rs:2580).

Fix

  • Copy a JS buffer's bytes before a UTF-8 decode: Bun::stableBytes for transcode, StringDecoder and the stream paths, Bun__encoding__toStringUnstable for Buffer.toString.
  • ASCII input is not copied again: the byte count alone sizes its Latin-1 string.
  • The decoders commit the count simdutf wrote. Input above the string limit is rejected before the copy.
  • Verified: test/js/node/buffer-shared-decode-race.test.ts fails on a release binary without the fix. 2225 tests pass across buffer, string_decoder, encoding and fetch body.

Background

  • Another thread can write any JS ArrayBuffer during a native read: a worker writes a shared one, a pool thread writes one that an async operation pinned (pread_inner, src/runtime/node/node_fs.rs:6054).
  • Considered reserving one unit per input byte, and node's validate-then-convert shape. Both still overflow (proof in Notes).

Downsides

  • A non-ASCII Buffer.toString('utf8') copies its input once more: 1 MiB for a 1 MiB buffer. ASCII unchanged.
  • StringDecoder, transcode and the stream paths copy their input on every call. An ASCII chunk is copied again by the string constructor.
  • Not measured: instruction counts and binary size. No perf, valgrind or bloaty here.
Notes

The two repros. Both segfault a release build (1.4.3-canary.1) and both print ok under node v26.3.0.

// 1. An async fs.read fills a plain, non-shared Buffer from a pool thread.
import fs from "node:fs";
import { writeFileSync } from "node:fs";
const N = 1 << 20;
writeFileSync("/tmp/fsrace.bin", Buffer.alloc(N, 0x61));
const fd = fs.openSync("/tmp/fsrace.bin", "r");
const buf = Buffer.alloc(N, 0x80);
function round(i) {
  buf.fill(0x80);
  fs.read(fd, buf, 0, N, 0, () => { if (i < 400) round(i + 1); else console.log("ok"); });
  for (let k = 0; k < 40; k++) buf.toString("utf8");
}
round(0);
// 2. A SharedArrayBuffer flipped between two states that are BOTH valid UTF-8.
import { Worker, isMainThread, workerData, parentPort } from "node:worker_threads";
const N = 4096;
if (!isMainThread) {
  const u8 = new Uint8Array(workerData);
  const two = new Uint8Array(N); for (let i = 0; i < N; i += 2) { two[i] = 0xc3; two[i + 1] = 0xa9; }
  const asc = new Uint8Array(N).fill(0x61);
  parentPort.postMessage("go");
  for (;;) { u8.set(two); u8.set(asc); }
} else {
  const sab = new SharedArrayBuffer(N);
  const buf = Buffer.from(sab);
  for (let i = 0; i < N; i += 2) { buf[i] = 0xc3; buf[i + 1] = 0xa9; }
  const w = new Worker(new URL(import.meta.url), { workerData: sab });
  w.on("message", () => {
    let n = 0;
    for (let i = 0; i < 2000; i++) n += buf.toString("utf8").length;
    console.log("ok", n > 0);
    process.exit(0);
  });
}

Under ASAN the first repro reports the decoder, the same frames as the SharedArrayBuffer case. The length pass reserved 1047424 units for the 1048576-byte input, because 1152 bytes were still 0x80. The convert pass then read ASCII there and wrote one unit per byte.

ERROR: AddressSanitizer: heap-buffer-overflow
WRITE of size 64 ... thread T0
  simdutf::convert_utf8_to_utf16le_with_errors
  bun_core::string::immutable::to_utf16_alloc
  Bun__encoding__toString
  WebCore::jsBufferToString
is located 0 bytes after 2094848-byte region
allocated by ... Vec<u16>::try_reserve_exact

Why a reservation sized from the input is not enough. One UTF-16 unit per input byte is the upper bound for a stable input only. On an error the icelake kernel calls scalar::utf8_to_utf16::rewind_and_convert_with_errors(pos, in + pos, size - pos, utf16_output) with prior_bytes = pos and the current output pointer (simdutf_impl.cpp.h:19902). The scalar routine sets how_far_back = prior_bytes, walks back to a leading byte, and converts those bytes again into the same pointer. A racing writer makes the continuation run as long as it likes, so the overshoot is bounded only by the input length. A harness over simdutf alone, with the output sized at the input byte count and a canary after it, wrote up to 256 units past in 7 of 3e6 iterations, and simdutf returned SUCCESS with count = N + 256. So the returned count cannot be checked either.

Why validating first is not enough. That is node's shape: validate_utf8, then convert_valid_utf8_to_utf16, which has no rewind. Repro 2 defeats it. Both states are valid UTF-8, so nothing rejects the input, and a length pass on C3 A9 repeated (2048 units) with a convert pass on 61 repeated (4096 units) writes twice the reservation. Release bun segfaults 4 of 5 runs. node survives through its layering, not its fast path: validate_ascii_with_errors catches the all-ASCII state and copies, and a torn read usually splits a C3 A9 pair and fails validate_utf8, which routes to V8's bounded decoder.

Why the ASCII check is safe on live bytes. first_non_ascii can be wrong the moment it returns, but the branch it picks is safe either way. The ASCII branch copies exactly len bytes into a Latin-1 string, and content cannot change that count. A byte that turns non-ASCII first is read as Latin-1, which is what node's ASCII fast path also produces. An earlier form of this check called back into the borrowing path, which tests the bytes again and reached the two-pass decoder. Repro 2 caught it.

What the copy does not do. Reading a racing buffer is a data race by the Rust abstract machine whichever way it is read. The copy does not remove the race. It bounds the damage to one unsynchronised memcpy whose result is then decoded in isolation. V8 and JSC do the same for SAB-backed views.

Error paths that moved. An unconditional copy in StringDecoder first turned the 2 GiB ERR_STRING_TOO_LONG case into an allocation failure, which string-decoder.test.js caught. The limit check now runs before the copy in write, end, text and the two stream chunk steps. text() read the view length through a uint32_t, so a 2**32-byte buffer measured as 0. It measures in size_t now and throws ERR_STRING_TOO_LONG, as write and end do. The existing 2 GiB test covers that case too.

Scope. Bun.JSONL.parse on a view goes through WTF::String::fromUTF8ReplacingInvalidSequences, which sizes by byte count. Bun.JSON5.parse, Bun.JSONC.parse and Response#json do not reproduce. Bun.TOML.parse and Bun.YAML.parse over a shared buffer are exposed one level up: they hand the view's bytes to a parser as source text, and the lexer reads them many times. That needs a private copy of the source buffer, not a copy at the decoder, so it is not in this PR. #29088 (dylan-conway) fixed this class before the Rust port moved the files. #33072 and #31339 landed the TextDecoder and HTTP parser parts.

The test. It is serial: each fixture pins a core with a worker that rewrites the buffer in a spin loop, and five at once starved each other into the per-test timeout. On a release binary without the fix at least 3 of the 5 modes crash per run. Under ASAN the overflow is caught on the first unit past the end. It has its own file, next to buffer-copy-fill-detach.test.ts and buffer-indexOf-detach.test.ts, because a 14 s serial stress block does not belong in buffer.test.js.

Suites run with the fix. buffer.test.js, buffer-utf16, buffer-shared-decode-race, string_decoder, all of test/js/web/encoding/, fetch/body.test.ts, fetch/wpt/textstream-wpt.test.ts.

Self-review. The first design copied only a SharedArrayBuffer. A review comment showed the async fs.read case, which that design cannot see, and the two experiments above removed the cheaper alternatives. Two more review findings were fixed: the ASCII path had cost an extra allocation, and an over-long stream chunk was copied before it was rejected.


[human-review] gate passed · iteration 2 · 13 files touched

fails on main (without fix)
ASAN without fix: 5 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/buffer-shared-decode-race.test.ts test/js/node/string_decoder/string-decoder.test.js
bun test v1.4.3 (367d939d9)

test/js/node/buffer-shared-decode-race.test.ts:
killed 1 dangling process
(fail) toString on a SharedArrayBuffer that another thread writes [5004.70ms]
  ^ this test timed out after 5000ms.

# Unhandled error between tests
-------------------------------
80 |       stdout: "pipe",
81 |       stderr: "pipe",
82 |     });
83 |     const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
84 |     // stderr carries the panic or the sanitizer report on a failing build.
85 |     expect(stdout, stderr).toBe("ok true\n");
                                ^
error: ============================================================
Bun Debug v1.4.3 (367d939d9) Linux x64
Linux Kernel v7.0.0 | glibc v2.41
CPU: sse42 popcnt avx avx2 avx512
Args: "/workspace/bun/build/debug/bun-debug" "fixture.mjs" "toString" "500"
Features: jsc workers_spawned 
Builtins: "bun:main" "node:buffer" "n
... (truncated)

release without fix: 6 FAILED
bun test v1.4.3-canary.1 (367d939d9)

test/js/node/buffer-shared-decode-race.test.ts:
80 |       stdout: "pipe",
81 |       stderr: "pipe",
82 |     });
83 |     const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
84 |     // stderr carries the panic or the sanitizer report on a failing build.
85 |     expect(stdout, stderr).toBe("ok true\n");
                                ^
error: ============================================================
Bun Canary v1.4.3-canary.1 (367d939d9) Linux x64
Linux Kernel v7.0.0 | glibc v2.41
CPU: sse42 popcnt avx avx2 avx512
Args: "/workspace/bun/build/release/bun" "fixture.mjs" "toString" "500"
Features: jsc workers_spawned 
Builtins: "bun:main" "node:buffer" "node:string_decoder" "node:worker_threads" 

Elapsed: 31ms | User: 32ms | Sys: 8ms
RSS: 43.45 MB | Peak: 44.50 MB | Commit: 59.77 MB | Faults: 0 | Machine: 34.36 GB

panic(main thread): Segmentation fault at address 0x0
oh no: Bun has crashed. This indicates a bug in Bun, not your code.

To send a redacted crash report to Bun's team,
please file a GitHub issue using the link below:

 https://bun.report/1.4.3/la2367d939g
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/buffer-shared-decode-race.test.ts test/js/node/string_decoder/string-decoder.test.js
bun test v1.4.3 (367d939d9)

test/js/node/buffer-shared-decode-race.test.ts:
(pass) toString on a SharedArrayBuffer that another thread writes [1624.61ms]
(pass) transcode on a SharedArrayBuffer that another thread writes [1405.13ms]
(pass) string_decoder on a SharedArrayBuffer that another thread writes [2040.76ms]
(pass) TextDecoderStream on a SharedArrayBuffer that another thread writes [1929.99ms]
(pass) textStream on a SharedArrayBuffer that another thread writes [2127.57ms]

test/js/node/string_decoder/string-decoder.test.js:
(pass) require('string_decoder') [4.55ms]
(pass) Bun.inspect(StringDecoder) [4.48ms]
(pass) FakeStringDecoderCall > StringDecoder-utf8 [284.51ms]
(pass) FakeStringDecoderCall > StringDecoder-ucs-2 [198.13ms]
(pass) FakeStringDecoderCall > StringDecoder-utf16le [4.72ms]
(pass) FakeStringDecoderCall > StringDecoder-utf8-additional [6.05ms]
(pass) FakeStringDecoderCall > StringDecoder-utf16le-additional [4.92ms]
(pa
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 1173ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/223] gen JSBuffer.lut.h
Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[2/223] gen generated_host_exports.rs
generated_host_exports.rs: 121 exports (host=5, lazy=10, generic=106, rust=0); 248 extern-C blocks audited
[3/223] gen cpp.rs (cppbind)
[4/223] gen JS modules (bundle-modules)
Preprocess modules (7046ms)
Bundle modules (88ms)
Postprocesss modules (174ms)
Bundle Functions (853ms)
Generate Code (68ms)

[8.24s] Bundled "src/js" for production
  2613 kb
  198 internal modules
  13 native modules
  50 internal functions across 16 files
[5/220] cc obj/packages/bun-usockets/src/node_quic_shim.c.o
[6/220] cc obj/packages/bun-usockets/src/udp.c.o
[7/220] cc obj/packages/bun-usockets/src/eventing/libuv.c.o
[8/220] cc obj/packages/bun-usockets/src/loop.c.o
[9/220] cc obj/packages/bun-usockets/src/eventing/epoll_kqueue.c.o
[10/220] cc obj/packages/bun-usockets/src/bsd.c.o
[11/220] cc obj/packages/bun-usockets/src/context.c.o
[12/220] cc obj/p
... (truncated)
diff hotspot
src/bun_core/string/immutable.rs                   |  9 ++-
 src/bun_core/string/immutable/unicode.rs           | 17 ++++-
 src/jsc/bindings/BunString.cpp                     |  1 +
 src/jsc/bindings/JSBuffer.cpp                      | 22 +++++-
 src/jsc/bindings/JSBuffer.h                        | 16 +++-
 src/jsc/bindings/JSStringDecoder.cpp               | 36 ++++++---
 src/jsc/bindings/JSStringDecoder.h                 | 10 +--
 .../webcore/streams/JSTextDecoderStream.cpp        | 33 +++++---
 .../webcore/streams/ReadableStreamOperations.cpp   |  8 ++
 src/jsc/modules/NodeBufferModule.cpp               |  5 +-
 src/runtime/webcore/encoding.rs                    | 41 ++++++++++
 test/js/node/buffer-shared-decode-race.test.ts     | 88 ++++++++++++++++++++++
 test/js/node/string_decoder/string-decoder.test.js | 31 ++++++++
 13 files changed, 278 insertions(+), 39 deletions(-)

gate history · 3 passed · 0 rejected · iteration 2

evidence per changed file
file                                                      reads  edits  tests
src/bun_core/string/immutable.rs                              3      2     63
src/bun_core/string/immutable/unicode.rs                      5      5     63
src/jsc/bindings/BunString.cpp                                0      0     63
src/jsc/bindings/JSBuffer.cpp                                 4      6     63
src/jsc/bindings/JSBuffer.h                                   2      3     63
src/jsc/bindings/JSStringDecoder.cpp                          3      5     63
src/jsc/bindings/JSStringDecoder.h                            0      0     63
src/jsc/bindings/webcore/streams/JSTextDecoderStream.cpp      0      0     63
…c/bindings/webcore/streams/ReadableStreamOperations.cpp      0      0     63
src/jsc/modules/NodeBufferModule.cpp                          2      5     63
src/runtime/webcore/encoding.rs                               2      2     63
test/js/node/buffer-shared-decode-race.test.ts                2      5     38
test/js/node/string_decoder/string-decoder.test.js            0      0     29