From d5cb5cbf34705ba8c00ec2c1c80892f857d9b4ca Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 6 Jul 2026 19:33:27 +0000 Subject: [PATCH 1/5] Buffer: match Node semantics for raw Slice / Write bindings The raw Buffer.prototype.Slice / Write prototype methods were routed through the strict validator used by the documented toString() / write() wrappers, so calls that succeed on Node threw on Bun: buf.hexSlice(pastEnd) -> "" on Node, RangeError on Bun buf.hexWrite(str, off, hugeLen) -> clamped on Node, RangeError on Bun Mirror Node's node_buffer.cc bindings instead: - StringSlice short-circuits to "" when start >= end, before the end <= length range check. - base64/base64url/hex/ucs2/utf16le Write clamp length to the space left and report a negative index as ERR_OUT_OF_RANGE. utf8/latin1/ ascii Write stay on the strict wrapper Node keeps for them. - parseArrayIndex follows v8::Value::IntegerValue (NaN -> 0, saturating) instead of truncateDoubleToInt64, which mapped NaN/Infinity to INT64_MIN and made them throw. Also drop the length/offset assertions in jsBufferToString: a valueOf() that shrinks a resizable ArrayBuffer during index coercion leaves the snapshotted range stale, which aborted debug builds. The existing clamp below already handles it. --- test/js/node/buffer.test.js | 216 +++++++++++++++++++++++++++++++++++- 1 file changed, 212 insertions(+), 4 deletions(-) diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index 2c8f6201823e..b770fe6d7c61 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -3425,8 +3425,10 @@ for (let withOverridenBufferWrite of [false, true]) { expect(buf.latin1Slice(1, 2)).toStrictEqual("é"); expect(() => buf.latin1Slice(1, 4)).toThrow(RangeError); - expect(() => buf.latin1Slice(4, 1)).toThrow(RangeError); - expect(() => buf.latin1Slice(4, 0)).toThrow(RangeError); + + // start >= end short-circuits to "" before the range check, as in Node. + expect(buf.latin1Slice(4, 1)).toStrictEqual(""); + expect(buf.latin1Slice(4, 0)).toStrictEqual(""); expect(buf.latin1Slice(3)).toStrictEqual(""); expect(buf.latin1Slice(3, 1)).toStrictEqual(""); @@ -3444,10 +3446,11 @@ for (let withOverridenBufferWrite of [false, true]) { expect(latin1Slice.call(buf, 1, 2)).toStrictEqual("é"); expect(() => latin1Slice.call(buf, 1, 4)).toThrow(RangeError); - expect(() => latin1Slice.call(buf, 4, 1)).toThrow(RangeError); - expect(() => latin1Slice.call(buf, 4, 0)).toThrow(RangeError); expect(() => latin1Slice.call(buf, 3, 999999)).toThrow(RangeError); + expect(latin1Slice.call(buf, 4, 1)).toStrictEqual(""); + expect(latin1Slice.call(buf, 4, 0)).toStrictEqual(""); + expect(latin1Slice.call(buf, 3)).toStrictEqual(""); expect(latin1Slice.call(buf, 3, 1)).toStrictEqual(""); expect(latin1Slice.call(buf, 2, 1)).toStrictEqual(""); @@ -4037,6 +4040,11 @@ describe("*Write methods with NaN/invalid offset and length", () => { expect(result).toBeLessThanOrEqual(buf.length); }); + } + + // Node only put utf8/latin1/ascii behind the strict JS wrapper that rejects an + // oversized length; the remaining encodings are still the raw C++ binding. + for (const method of ["utf8Write", "latin1Write", "asciiWrite"]) { it(`${method} should throw on length larger than available buffer space`, () => { const buf = Buffer.from("string"); // Length 1000 with valid offset 0 should throw ERR_BUFFER_OUT_OF_BOUNDS @@ -4047,6 +4055,206 @@ describe("*Write methods with NaN/invalid offset and length", () => { ); }); } + + for (const method of ["utf16leWrite", "ucs2Write", "base64Write", "base64urlWrite", "hexWrite"]) { + it(`${method} should clamp length larger than available buffer space`, () => { + const buf = Buffer.from("string"); + const written = buf[method]("test".repeat(100), 0, 1000); + expect(written).toBeLessThanOrEqual(buf.length); + }); + } +}); + +// These raw prototype methods come straight from Node's C++ bindings, not from the +// documented toString()/write() wrappers, and they have looser bounds rules: +// https://github.com/nodejs/node/blob/v26.3.0/src/node_buffer.cc +describe("raw Slice / Write bindings match Node", () => { + const OUT_OF_RANGE = expect.objectContaining({ code: "ERR_OUT_OF_RANGE", message: "Index out of range" }); + const OUT_OF_BOUNDS = expect.objectContaining({ code: "ERR_BUFFER_OUT_OF_BOUNDS" }); + + const sliceMethods = [ + "utf8Slice", + "latin1Slice", + "asciiSlice", + "ucs2Slice", + "utf16leSlice", + "base64Slice", + "base64urlSlice", + "hexSlice", + ]; + // "hello!", 6 bytes. + const hello = () => Buffer.from("68656c6c6f21", "hex"); + + describe("Slice", () => { + it.each(sliceMethods)('%s returns "" when start >= end, without a range check', method => { + const buf = hello(); + expect(buf[method](6)).toBe(""); + expect(buf[method](7)).toBe(""); + expect(buf[method](7, 3)).toBe(""); + expect(buf[method](7, 7)).toBe(""); + expect(buf[method](3, 1)).toBe(""); + expect(buf[method](1e9)).toBe(""); + expect(buf[method](2 ** 53)).toBe(""); + expect(buf[method](2 ** 64)).toBe(""); + expect(buf[method](Infinity)).toBe(""); + expect(buf[method](0, NaN)).toBe(""); + }); + + it.each(sliceMethods)("%s still throws when end is past the end of the buffer", method => { + const buf = hello(); + expect(() => buf[method](0, 7)).toThrow(OUT_OF_RANGE); + expect(() => buf[method](2, 1e9)).toThrow(OUT_OF_RANGE); + expect(() => buf[method](6, 1e9)).toThrow(OUT_OF_RANGE); + expect(() => buf[method](0, Infinity)).toThrow(OUT_OF_RANGE); + }); + + it.each(sliceMethods)("%s treats NaN as 0 and rejects negative indexes", method => { + const buf = hello(); + expect(buf[method](NaN)).toBe(buf[method]()); + expect(buf[method](-0)).toBe(buf[method]()); + expect(() => buf[method](-1)).toThrow(OUT_OF_RANGE); + expect(() => buf[method](-Infinity)).toThrow(OUT_OF_RANGE); + expect(() => buf[method](0, -1)).toThrow(OUT_OF_RANGE); + }); + + it("decodes the same ranges Node decodes", () => { + const buf = hello(); + expect(buf.hexSlice()).toBe("68656c6c6f21"); + expect(buf.hexSlice(2)).toBe("6c6c6f21"); + expect(buf.hexSlice(0, 2)).toBe("6865"); + expect(buf.hexSlice(5, 6)).toBe("21"); + expect(buf.utf8Slice(1.9)).toBe("ello!"); + expect(buf.utf8Slice("2", "4")).toBe("ll"); + }); + + it("the documented toString() wrapper is unchanged", () => { + const buf = hello(); + expect(buf.toString("hex", 7)).toBe(""); + expect(buf.toString("hex", 7, 3)).toBe(""); + expect(buf.toString("hex", 0, 1e9)).toBe("68656c6c6f21"); + }); + + // Both Slice and toString() read byteLength before coercing their indexes, so a + // valueOf() that shrinks a resizable buffer leaves the range stale. Spawned because an + // unclamped range aborts a debug build rather than throwing. + it.each(["hexSlice", "toString"])("%s clamps the range when valueOf() shrinks the buffer", async method => { + const read = method === "toString" ? `buf.toString("hex", 0, shrink)` : `buf.hexSlice(0, shrink)`; + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const ab = new ArrayBuffer(9, { maxByteLength: 9 }); + const buf = Buffer.from(ab); + buf.fill(0x41); + const shrink = { valueOf() { ab.resize(2); return 9; } }; + console.log(JSON.stringify({ read: ${read}, length: buf.length }));`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout.trim()).toBe(`{"read":"4141","length":2}`); + expect(exitCode).toBe(0); + }); + }); + + describe("Write", () => { + // Node's utf8Write/latin1Write/asciiWrite go through a JS wrapper that rejects an + // out-of-range length; base64/base64url/hex/ucs2 stay on the clamping C++ binding. + const strict = ["utf8Write", "latin1Write", "asciiWrite"]; + const clamping = ["ucs2Write", "utf16leWrite", "base64Write", "base64urlWrite", "hexWrite"]; + const source = { + utf8Write: "hello", + latin1Write: "hello", + asciiWrite: "hello", + ucs2Write: "hello", + utf16leWrite: "hello", + base64Write: "aGVsbG8=", + base64urlWrite: "aGVsbG8", + hexWrite: "68656c6c6f", + }; + // 9 bytes of 0xcc, so offset 6 leaves 3 bytes of room. + const dest = () => Buffer.alloc(9, 0xcc); + const untouched = "cccccccccccccccccc"; + + it.each(clamping)("%s clamps an oversized length to the space left", method => { + const expected = { + ucs2Write: { written: 2, hex: "cccccccccccc6800cc" }, + utf16leWrite: { written: 2, hex: "cccccccccccc6800cc" }, + base64Write: { written: 3, hex: "cccccccccccc68656c" }, + base64urlWrite: { written: 3, hex: "cccccccccccc68656c" }, + hexWrite: { written: 3, hex: "cccccccccccc68656c" }, + }; + const buf = dest(); + const written = buf[method](source[method], 6, 1000); + expect({ written, hex: buf.toString("hex") }).toEqual(expected[method]); + }); + + it.each(clamping)("%s writes nothing when no space is left", method => { + const buf = dest(); + expect(buf[method](source[method], 9, 1)).toBe(0); + expect(buf.toString("hex")).toBe(untouched); + }); + + it.each(clamping)("%s reports a negative offset or length as ERR_OUT_OF_RANGE", method => { + const buf = dest(); + expect(() => buf[method](source[method], -1)).toThrow(OUT_OF_RANGE); + expect(() => buf[method](source[method], -1, 2)).toThrow(OUT_OF_RANGE); + expect(() => buf[method](source[method], 0, -1)).toThrow(OUT_OF_RANGE); + expect(buf.toString("hex")).toBe(untouched); + }); + + it.each(strict)("%s rejects an oversized length with ERR_BUFFER_OUT_OF_BOUNDS", method => { + const buf = dest(); + expect(() => buf[method](source[method], 6, 1000)).toThrow(OUT_OF_BOUNDS); + expect(() => buf[method](source[method], 9, 1)).toThrow(OUT_OF_BOUNDS); + expect(() => buf[method](source[method], -1)).toThrow(OUT_OF_BOUNDS); + expect(() => buf[method](source[method], 0, -1)).toThrow(OUT_OF_BOUNDS); + expect(buf.toString("hex")).toBe(untouched); + }); + + it.each([...strict, ...clamping])("%s rejects an offset past the end of the buffer", method => { + const buf = dest(); + expect(() => buf[method](source[method], 10)).toThrow(OUT_OF_BOUNDS); + expect(() => buf[method](source[method], 10, 1)).toThrow(OUT_OF_BOUNDS); + expect(() => buf[method](source[method], Infinity)).toThrow(OUT_OF_BOUNDS); + expect(() => buf[method](source[method], 2 ** 53, 1)).toThrow(OUT_OF_BOUNDS); + expect(buf.toString("hex")).toBe(untouched); + }); + + it.each(strict)("%s with a NaN offset and no length writes nothing", method => { + // The wrapper's default length is `byteLength - offset`, i.e. NaN, which truncates to 0. + const buf = dest(); + expect(buf[method](source[method], NaN)).toBe(0); + expect(buf.toString("hex")).toBe(untouched); + }); + + it.each(clamping)("%s with a NaN offset and no length writes from offset 0", method => { + const expected = { + ucs2Write: { written: 8, hex: "680065006c006c00cc" }, + utf16leWrite: { written: 8, hex: "680065006c006c00cc" }, + base64Write: { written: 5, hex: "68656c6c6fcccccccc" }, + base64urlWrite: { written: 5, hex: "68656c6c6fcccccccc" }, + hexWrite: { written: 5, hex: "68656c6c6fcccccccc" }, + }; + const buf = dest(); + const written = buf[method](source[method], NaN); + expect({ written, hex: buf.toString("hex") }).toEqual(expected[method]); + }); + + it.each([...strict, ...clamping])("%s with a NaN length writes nothing", method => { + const buf = dest(); + expect(buf[method](source[method], 0, NaN)).toBe(0); + expect(buf.toString("hex")).toBe(untouched); + }); + + it("the documented write() wrapper is unchanged", () => { + const buf = dest(); + expect(() => buf.write("hello", 6, 1000)).toThrow(expect.objectContaining({ code: "ERR_OUT_OF_RANGE" })); + expect(buf.write("hello", 6)).toBe(3); + expect(buf.toString("hex")).toBe("cccccccccccc68656c"); + }); + }); }); describe("Buffer.copyBytesFrom", () => { From 91feaba3214e25aab369f1268cd577b8de714d5f Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 6 Jul 2026 19:36:03 +0000 Subject: [PATCH 2/5] [autofix.ci] apply automated fixes --- test/js/node/buffer.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index b770fe6d7c61..87adb447d8fd 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -4039,7 +4039,6 @@ describe("*Write methods with NaN/invalid offset and length", () => { // Result should be clamped to buffer size expect(result).toBeLessThanOrEqual(buf.length); }); - } // Node only put utf8/latin1/ascii behind the strict JS wrapper that rejects an From 3b859554619a2a23fdf0955a922ea9ec80205046 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 6 Jul 2026 19:52:29 +0000 Subject: [PATCH 3/5] Buffer: match Node semantics for raw Slice / Write bindings The raw Buffer.prototype.Slice / Write prototype methods were routed through the strict validator used by the documented toString() / write() wrappers, so calls that succeed on Node threw on Bun: buf.hexSlice(pastEnd) -> "" on Node, RangeError on Bun buf.hexWrite(str, off, hugeLen) -> clamped on Node, RangeError on Bun Mirror Node's node_buffer.cc bindings instead: - StringSlice short-circuits to "" when start >= end, before the end <= length range check. - base64/base64url/hex/ucs2/utf16le Write clamp length to the space left and report a negative index as ERR_OUT_OF_RANGE. utf8/latin1/ ascii Write stay on the strict wrapper Node keeps for them. - parseArrayIndex follows v8::Value::IntegerValue (NaN -> 0, saturating) instead of truncateDoubleToInt64, which mapped NaN/Infinity to INT64_MIN and made them throw. Also drop the length/offset assertions in jsBufferToString: a valueOf() that shrinks a resizable ArrayBuffer during index coercion leaves the snapshotted range stale, which aborted debug builds. The existing clamp below already handles it. --- src/jsc/bindings/JSBuffer.cpp | 115 +++++++++++++++++++++++++++------- test/js/node/buffer.test.js | 8 ++- 2 files changed, 100 insertions(+), 23 deletions(-) diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index 146c968bc46e..1452da27d051 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -2138,10 +2138,9 @@ JSC::EncodedJSValue jsBufferToString(JSC::JSGlobalObject* lexicalGlobalObject, T RELEASE_AND_RETURN(scope, JSValue::encode(jsEmptyString(vm))); } - ASSERT(offset <= byteLength); - ASSERT(length <= byteLength); - ASSERT(offset + length <= byteLength); - + // Callers snapshot byteLength before coercing their arguments, and the user JS + // those coercions can run may shrink a resizable buffer underneath us, so + // `offset` and `length` are clamped here rather than asserted. if (offset >= byteLength) { offset = byteLength; } @@ -2157,18 +2156,30 @@ JSC::EncodedJSValue jsBufferToString(JSC::JSGlobalObject* lexicalGlobalObject, T return jsBufferToStringFromBytes(lexicalGlobalObject, scope, castedThis->span().subspan(offset, length), encoding); } -// https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/src/node_buffer.cc#L208-L233 -bool inline parseArrayIndex(JSC::ThrowScope& scope, JSC::JSGlobalObject* globalObject, JSC::JSValue value, size_t& out, ASCIILiteral errorMessage) +// Mirrors v8::Value::IntegerValue(): NaN becomes 0 and anything outside the +// int64 range saturates instead of wrapping to the cvttsd2si sentinel. +static ALWAYS_INLINE int64_t toIntegerValue(double number) +{ + if (std::isnan(number)) return 0; + if (number >= static_cast(std::numeric_limits::max())) return std::numeric_limits::max(); + if (number <= static_cast(std::numeric_limits::min())) return std::numeric_limits::min(); + return truncateDoubleToInt64(number); +} + +// https://github.com/nodejs/node/blob/v26.3.0/src/node_internals.h#L208-L233 +// `out` keeps its incoming value when `value` is undefined: that is the default. +bool inline parseArrayIndex(JSC::ThrowScope& scope, JSC::JSGlobalObject* globalObject, JSC::JSValue value, size_t& out) { if (value.isUndefined()) { return true; } - int64_t index = truncateDoubleToInt64(value.toNumber(globalObject)); + double number = value.toNumber(globalObject); RETURN_IF_EXCEPTION(scope, false); + int64_t index = toIntegerValue(number); if (index < 0) { - throwNodeRangeError(globalObject, scope, errorMessage); + throwNodeRangeError(globalObject, scope, "Index out of range"_s); return false; } @@ -2328,7 +2339,9 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_toStringBody(JSC::JSGlobalO return jsBufferToString(lexicalGlobalObject, scope, castedThis, offset, length, encoding); } -// https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/src/node_buffer.cc#L544 +// https://github.com/nodejs/node/blob/v26.3.0/src/node_buffer.cc#L544 +// These are node's raw bindings, not the `toString()` wrapper: an empty range +// short-circuits before the range check, so `buf.hexSlice(pastEnd)` is "". template static JSC::EncodedJSValue jsBufferPrototypeFunction_SliceWithEncoding(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame) { @@ -2352,19 +2365,20 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_SliceWithEncoding(JSC::JSGl size_t start = 0; size_t end = length; - if (!parseArrayIndex(scope, lexicalGlobalObject, startValue, start, "start must be a positive integer"_s)) [[unlikely]] { + if (!parseArrayIndex(scope, lexicalGlobalObject, startValue, start)) [[unlikely]] { return {}; } - if (!parseArrayIndex(scope, lexicalGlobalObject, endValue, end, "end must be a positive integer"_s)) [[unlikely]] { + if (!parseArrayIndex(scope, lexicalGlobalObject, endValue, end)) [[unlikely]] { return {}; } - if (end < start) - end = start; + if (start >= end) { + return JSC::JSValue::encode(JSC::jsEmptyString(vm)); + } - if (!(end <= length)) { - throwNodeRangeError(lexicalGlobalObject, scope, "end out of range"_s); + if (end > length) { + throwNodeRangeError(lexicalGlobalObject, scope, "Index out of range"_s); return {}; } @@ -2395,7 +2409,9 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_SliceWithEncoding(JSC::JSGl // return JSValue::decode(jsBufferToString(vm, lexicalGlobalObject, thisValue, 0, thisValue->byteLength(), encoding)); // } -// https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/src/node_buffer.cc#L711 +// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/buffer.js#L962-L990 +// Only utf8Write/latin1Write/asciiWrite go through this strict JS wrapper in node; +// the other encodings use jsBufferPrototypeFunction_StringWriteWithEncoding below. template static JSC::EncodedJSValue jsBufferPrototypeFunction_writeEncodingBody(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSArrayBufferView* castedThis, JSString* str, JSValue offsetValue, JSValue lengthValue) { @@ -2447,7 +2463,9 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_writeEncodingBody(JSC::VM& // Calculate max_length size_t maxLength; if (lengthWasUndefined) { - maxLength = byteLength - safeOffset; + // The wrapper's default is `length = buf.byteLength - offset`, which is NaN + // when offset is NaN; the native call then truncates that NaN to 0. + maxLength = offsetWasNaN ? 0 : byteLength - safeOffset; } else { // Node.js JS wrapper checks: if (length < 0 || length > this.byteLength - offset) // When offset is NaN, (byteLength - offset) is NaN, so (length > NaN) is false. @@ -2492,6 +2510,61 @@ static JSC::EncodedJSValue jsBufferPrototypeFunctionWriteWithEncoding(JSC::JSGlo RELEASE_AND_RETURN(scope, jsBufferPrototypeFunction_writeEncodingBody(vm, lexicalGlobalObject, castedThis, text, offsetValue, lengthValue)); } +// https://github.com/nodejs/node/blob/v26.3.0/src/node_buffer.cc#L711-L741 +// base64/base64url/hex/ucs2 are still node's raw binding: an out-of-range `length` +// clamps to the space left instead of throwing, and a negative offset or length +// is ERR_OUT_OF_RANGE rather than ERR_BUFFER_OUT_OF_BOUNDS. +template +static JSC::EncodedJSValue jsBufferPrototypeFunction_StringWriteWithEncoding(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame) +{ + auto& vm = JSC::getVM(lexicalGlobalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + + auto* castedThis = dynamicDowncast(callFrame->thisValue()); + if (!castedThis) [[unlikely]] { + throwTypeError(lexicalGlobalObject, scope, "Expected ArrayBufferView"_s); + return {}; + } + + JSString* text = callFrame->argument(0).toStringOrNull(lexicalGlobalObject); + RETURN_IF_EXCEPTION(scope, {}); + + size_t offset = 0; + if (!parseArrayIndex(scope, lexicalGlobalObject, callFrame->argument(1), offset)) [[unlikely]] { + return {}; + } + + // The coercions above can run arbitrary JS, which may detach or resize the view. + if (castedThis->isDetached()) [[unlikely]] { + throwTypeError(lexicalGlobalObject, scope, "ArrayBufferView is detached"_s); + return {}; + } + size_t byteLength = castedThis->byteLength(); + + if (offset > byteLength) { + return Bun::ERR::BUFFER_OUT_OF_BOUNDS(scope, lexicalGlobalObject, "offset"_s); + } + + size_t maxLength = byteLength - offset; + if (!parseArrayIndex(scope, lexicalGlobalObject, callFrame->argument(2), maxLength)) [[unlikely]] { + return {}; + } + + if (castedThis->isDetached()) [[unlikely]] { + throwTypeError(lexicalGlobalObject, scope, "ArrayBufferView is detached"_s); + return {}; + } + byteLength = castedThis->byteLength(); + + const size_t remaining = offset < byteLength ? byteLength - offset : 0; + maxLength = std::min(remaining, maxLength); + if (maxLength == 0) { + return JSC::JSValue::encode(JSC::jsNumber(0)); + } + + RELEASE_AND_RETURN(scope, writeToBuffer(lexicalGlobalObject, castedThis, text, offset, maxLength, encoding)); +} + static JSC::EncodedJSValue jsBufferPrototypeFunction_writeBody(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame, typename IDLOperation::ClassParameter castedThis) { auto& vm = JSC::getVM(lexicalGlobalObject); @@ -2849,7 +2922,7 @@ JSC_DEFINE_HOST_FUNCTION(jsBufferPrototypeFunction_write, (JSGlobalObject * lexi JSC_DEFINE_HOST_FUNCTION(jsBufferPrototypeFunction_utf16leWrite, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) { - return jsBufferPrototypeFunctionWriteWithEncoding(lexicalGlobalObject, callFrame); + return jsBufferPrototypeFunction_StringWriteWithEncoding(lexicalGlobalObject, callFrame); } JSC_DEFINE_HOST_FUNCTION(jsBufferPrototypeFunction_utf8Write, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) @@ -2869,17 +2942,17 @@ JSC_DEFINE_HOST_FUNCTION(jsBufferPrototypeFunction_asciiWrite, (JSGlobalObject * JSC_DEFINE_HOST_FUNCTION(jsBufferPrototypeFunction_base64Write, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) { - return jsBufferPrototypeFunctionWriteWithEncoding(lexicalGlobalObject, callFrame); + return jsBufferPrototypeFunction_StringWriteWithEncoding(lexicalGlobalObject, callFrame); } JSC_DEFINE_HOST_FUNCTION(jsBufferPrototypeFunction_base64urlWrite, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) { - return jsBufferPrototypeFunctionWriteWithEncoding(lexicalGlobalObject, callFrame); + return jsBufferPrototypeFunction_StringWriteWithEncoding(lexicalGlobalObject, callFrame); } JSC_DEFINE_HOST_FUNCTION(jsBufferPrototypeFunction_hexWrite, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) { - return jsBufferPrototypeFunctionWriteWithEncoding(lexicalGlobalObject, callFrame); + return jsBufferPrototypeFunction_StringWriteWithEncoding(lexicalGlobalObject, callFrame); } JSC_DEFINE_HOST_FUNCTION(jsBufferPrototypeFunction_utf8Slice, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index 87adb447d8fd..9fddf5f3eb7c 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -4152,8 +4152,12 @@ describe("raw Slice / Write bindings match Node", () => { stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stdout.trim()).toBe(`{"read":"4141","length":2}`); - expect(exitCode).toBe(0); + // Surface stderr so an abort diagnostic shows up in the diff if the assert regresses. + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: `{"read":"4141","length":2}`, + stderr: expect.any(String), + exitCode: 0, + }); }); }); From 19dda264ecf3560dbaf9236e7800aff5ee2fdc42 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:07:40 +0000 Subject: [PATCH 4/5] Buffer: only re-check detach in StringWriteWithEncoding when coercion ran JS The offset/length coercions only run user-overridable code (valueOf / Symbol.toPrimitive / toString) for object arguments. Gate the isDetached re-check on that so the common path (string + numeric args) skips it, and re-read byteLength only after a coercion that could have resized the view. --- src/jsc/bindings/JSBuffer.cpp | 29 +++++++++++++-------- test/js/node/buffer.test.js | 47 +++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index 1452da27d051..31248929e024 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -2526,16 +2526,21 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_StringWriteWithEncoding(JSC return {}; } - JSString* text = callFrame->argument(0).toStringOrNull(lexicalGlobalObject); + const JSValue strValue = callFrame->argument(0); + const JSValue offsetValue = callFrame->argument(1); + const JSValue lengthValue = callFrame->argument(2); + + JSString* text = strValue.toStringOrNull(lexicalGlobalObject); RETURN_IF_EXCEPTION(scope, {}); size_t offset = 0; - if (!parseArrayIndex(scope, lexicalGlobalObject, callFrame->argument(1), offset)) [[unlikely]] { + if (!parseArrayIndex(scope, lexicalGlobalObject, offsetValue, offset)) [[unlikely]] { return {}; } - // The coercions above can run arbitrary JS, which may detach or resize the view. - if (castedThis->isDetached()) [[unlikely]] { + // toStringOrNull/toNumber only run user-overridable code for object arguments, and + // that code can detach or resize the view, so re-validate only when it could have run. + if ((strValue.isObject() || offsetValue.isObject()) && castedThis->isDetached()) [[unlikely]] { throwTypeError(lexicalGlobalObject, scope, "ArrayBufferView is detached"_s); return {}; } @@ -2546,18 +2551,20 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_StringWriteWithEncoding(JSC } size_t maxLength = byteLength - offset; - if (!parseArrayIndex(scope, lexicalGlobalObject, callFrame->argument(2), maxLength)) [[unlikely]] { + if (!parseArrayIndex(scope, lexicalGlobalObject, lengthValue, maxLength)) [[unlikely]] { return {}; } - if (castedThis->isDetached()) [[unlikely]] { - throwTypeError(lexicalGlobalObject, scope, "ArrayBufferView is detached"_s); - return {}; + // A length argument that is an object may have detached or resized the view too. + if (lengthValue.isObject()) { + if (castedThis->isDetached()) [[unlikely]] { + throwTypeError(lexicalGlobalObject, scope, "ArrayBufferView is detached"_s); + return {}; + } + byteLength = castedThis->byteLength(); } - byteLength = castedThis->byteLength(); - const size_t remaining = offset < byteLength ? byteLength - offset : 0; - maxLength = std::min(remaining, maxLength); + maxLength = std::min(offset < byteLength ? byteLength - offset : 0, maxLength); if (maxLength == 0) { return JSC::JSValue::encode(JSC::jsNumber(0)); } diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index 9fddf5f3eb7c..a873759eec41 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -4257,6 +4257,53 @@ describe("raw Slice / Write bindings match Node", () => { expect(buf.write("hello", 6)).toBe(3); expect(buf.toString("hex")).toBe("cccccccccccc68656c"); }); + + // A detaching valueOf on the offset or length argument is the one case that runs user JS; + // the binding re-checks detachment there and must not write into freed memory. + it.each(["offset", "length"])("%s throws when a detaching valueOf runs mid-coercion", async which => { + const args = which === "offset" ? `detach, 5` : `0, detach`; + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const buf = Buffer.from(new ArrayBuffer(9)); + const detach = { valueOf() { structuredClone(buf.buffer, { transfer: [buf.buffer] }); return 5; } }; + try { buf.hexWrite("68656c", ${args}); console.log("NO THROW"); } + catch (e) { console.log(e.constructor.name); }`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: "TypeError", + stderr: expect.any(String), + exitCode: 0, + }); + }); + + it("clamps when a length valueOf shrinks a resizable buffer", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const ab = new ArrayBuffer(9, { maxByteLength: 9 }); + const buf = Buffer.from(ab); + const shrink = { valueOf() { ab.resize(2); return 1000; } }; + const written = buf.hexWrite("68656c6c6f", 0, shrink); + console.log(JSON.stringify({ written, length: buf.length }));`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // 2 bytes left after the shrink, so only one byte-pair is written. + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: `{"written":2,"length":2}`, + stderr: expect.any(String), + exitCode: 0, + }); + }); }); }); From 195b495a74270227f61d11462979f061331ef665 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 6 Jul 2026 22:46:14 +0000 Subject: [PATCH 5/5] ci: retrigger