diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index e0e55dd27493..73da49dcc076 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -386,8 +386,8 @@ impl ArrayBuffer { pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> ArrayBuffer { ArrayBuffer { - len: u32::try_from(bytes.len()).expect("int cast") as usize, - byte_len: u32::try_from(bytes.len()).expect("int cast") as usize, + len: bytes.len(), + byte_len: bytes.len(), typed_array_type, ptr: bytes.as_mut_ptr(), ..Default::default() @@ -408,8 +408,8 @@ impl ArrayBuffer { // this is an FFI hand-off, not a leak. let ptr = bun_core::heap::into_raw(bytes).cast::(); ArrayBuffer { - len: u32::try_from(len).expect("int cast") as usize, - byte_len: u32::try_from(len).expect("int cast") as usize, + len, + byte_len: len, typed_array_type, ptr, ..Default::default() @@ -876,6 +876,20 @@ impl MarkedArrayBuffer { }) } + /// For in-place writes: re-read from `value` when `self.buffer.ptr` is an + /// owned snapshot (see `StringOrBuffer::array_buffer_into`). + #[inline] + pub fn live_array_buffer(&self, global: &JSGlobalObject) -> ArrayBuffer { + if self.owns_buffer && self.buffer.value != JSValue::ZERO { + return self + .buffer + .value + .as_array_buffer(global) + .unwrap_or_default(); + } + self.buffer + } + pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> MarkedArrayBuffer { MarkedArrayBuffer { buffer: ArrayBuffer::from_bytes(bytes, typed_array_type), diff --git a/src/runtime/api/MarkdownObject.rs b/src/runtime/api/MarkdownObject.rs index 22855563fbd5..ceb5a9fcc62f 100644 --- a/src/runtime/api/MarkdownObject.rs +++ b/src/runtime/api/MarkdownObject.rs @@ -79,6 +79,9 @@ impl PinnedView { let Some(b) = buffer.buffer() else { return Ok(None); }; + if b.owns_buffer { + return Ok(None); + } match b.buffer.value.as_pinned_arraybuffer(global) { Some(pinned) => Ok(Some(Self(pinned))), None => Err(global.throw_out_of_memory()), diff --git a/src/runtime/crypto/CryptoHasher.rs b/src/runtime/crypto/CryptoHasher.rs index 469d943b4ebe..d67b2ff894f6 100644 --- a/src/runtime/crypto/CryptoHasher.rs +++ b/src/runtime/crypto/CryptoHasher.rs @@ -438,7 +438,7 @@ impl CryptoHasher { if let Some(string_or_buffer) = output { if let StringOrBuffer::Buffer(buffer) = &string_or_buffer { - let ab = buffer.buffer; + let ab = buffer.live_array_buffer(global); return Self::hash_to_bytes(global, &mut evp, input, Some(ab)); } // `inline else => |*str|` — every non-buffer arm yields a string-like @@ -687,7 +687,7 @@ impl CryptoHasher { ) -> JsResult { if let Some(string_or_buffer) = output { if let StringOrBuffer::Buffer(buffer) = &string_or_buffer { - let ab = buffer.buffer; + let ab = buffer.live_array_buffer(global); return this.digest_to_bytes(global, Some(ab)); } // `defer str.deinit()` — handled by Drop. @@ -927,7 +927,7 @@ impl CryptoHasherZig { ) -> JsResult { if let Some(string_or_buffer) = output { if let StringOrBuffer::Buffer(buffer) = &string_or_buffer { - let ab = buffer.buffer; + let ab = buffer.live_array_buffer(global); return Self::hash_by_name_inner_to_bytes::(global, input, Some(ab)); } let Some(encoding) = Encoding::from(string_or_buffer.slice()) else { @@ -1374,7 +1374,7 @@ impl StaticCryptoHasher { if let Some(string_or_buffer) = output { if let StringOrBuffer::Buffer(buffer) = &string_or_buffer { - let ab = buffer.buffer; + let ab = buffer.live_array_buffer(global); return Self::hash_to_bytes(global, input, Some(ab)); } let Some(encoding) = Encoding::from(string_or_buffer.slice()) else { @@ -1464,7 +1464,7 @@ impl StaticCryptoHasher { } if let Some(string_or_buffer) = output { if let StringOrBuffer::Buffer(buffer) = &string_or_buffer { - let ab = buffer.buffer; + let ab = buffer.live_array_buffer(global); return this.digest_to_bytes(global, Some(ab)); } let Some(encoding) = Encoding::from(string_or_buffer.slice()) else { diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 3ee2ace9e788..9e0ecbb65de9 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -3891,7 +3891,9 @@ pub mod args { } } } - if arguments.will_be_async && matches!(args.buffer, StringOrBuffer::Buffer(_)) { + if arguments.will_be_async + && matches!(&args.buffer, StringOrBuffer::Buffer(b) if !b.owns_buffer) + { if let Some(pinned) = bv.as_pinned_arraybuffer(ctx) { args.buffer = StringOrBuffer::Buffer(Buffer { buffer: pinned, diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index 3edab6e4ef9e..0c0d9cf97948 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -274,11 +274,28 @@ impl Drop for StringOrBuffer { Self::EncodedSlice(_encoded) => { // ZigStringSlice has Drop; cleanup is implicit. } - Self::Buffer(_) => {} + Self::Buffer(buffer) => { + buffer.destroy(); + } } } } +#[cold] +#[inline(never)] +fn snapshot_resizable(global: &JSGlobalObject, ab: &jsc::ArrayBuffer) -> Buffer { + let bytes = ab.byte_slice(); + let mut owned = if bytes.is_empty() { + Buffer::EMPTY + } else { + global.vm().report_extra_memory(bytes.len()); + bun_core::handle_oom(Buffer::from_string(bytes)) + }; + owned.buffer.value = ab.value; + owned.buffer.typed_array_type = ab.typed_array_type; + owned +} + impl bun_jsc::Unprotect for BlobOrStringOrBuffer { /// JS-side half of cleanup — owned /// payloads are released by `Drop` (which runs next when held in a @@ -356,7 +373,9 @@ impl StringOrBuffer { if buffer.buffer.value != JSValue::ZERO { return Ok(buffer.buffer.value); } - Ok(buffer.to_node_buffer(ctx)) + let js = buffer.to_node_buffer(ctx); + buffer.owns_buffer = false; + Ok(js) } } } @@ -371,6 +390,40 @@ impl StringOrBuffer { } } + /// `pin()` guards `transfer()` but not `ArrayBuffer.prototype.resize()`, + /// so resizable non-shared inputs are snapshotted (growable SAB only grows + /// in-place; captured extent stays readable). `snapshot_volatile = false` + /// opts out for callers that run no more user JS before reading. + #[inline] + fn array_buffer_into( + out: &mut Self, + global: &JSGlobalObject, + value: JSValue, + is_async: bool, + snapshot_volatile: bool, + ) { + let ab = value.as_array_buffer(global).unwrap_or_default(); + let buffer = if snapshot_volatile && ab.resizable && !ab.shared { + snapshot_resizable(global, &ab) + } else if is_async { + Buffer::from_js_pinned(global, value).unwrap_or(Buffer { + buffer: ab, + owns_buffer: false, + pinned: false, + }) + } else { + Buffer { + buffer: ab, + owns_buffer: false, + pinned: false, + } + }; + if is_async { + buffer.buffer.value.protect(); + } + *out = Self::Buffer(buffer); + } + /// Out-param core of [`from_js_maybe_async`]. Writes the decoded payload /// directly into `*out` and returns /// `Ok(true)` on success, `Ok(false)` if `value` is not a string/buffer @@ -437,18 +490,7 @@ impl StringOrBuffer { | JSType::BigInt64Array | JSType::BigUint64Array | JSType::DataView => { - let buffer = if is_async { - Buffer::from_js_pinned(global, value) - .unwrap_or_else(|| Buffer::from_array_buffer(global, value)) - } else { - Buffer::from_array_buffer(global, value) - }; - - if is_async { - buffer.buffer.value.protect(); - } - - *out = Self::Buffer(buffer); + Self::array_buffer_into(out, global, value, is_async, true); Ok(true) } _ => Ok(false), @@ -484,7 +526,8 @@ impl StringOrBuffer { Self::from_js_with_encoding_maybe_async(global, value, encoding, false, true) } - /// Out-param convenience wrapper — see [`from_js_with_encoding_maybe_async_into`]. + /// Out-param wrapper for `NodeHTTPResponse`; it evaluates encoding/callback + /// before capture and spills resizable tails itself (`snapshot_volatile=false`). #[inline] pub fn from_js_with_encoding_into( out: &mut StringOrBuffer, @@ -492,7 +535,9 @@ impl StringOrBuffer { value: JSValue, encoding: Encoding, ) -> JsResult { - Self::from_js_with_encoding_maybe_async_into(out, global, value, encoding, false, true) + Self::from_js_with_encoding_maybe_async_into( + out, global, value, encoding, false, true, false, + ) } /// Out-param core of [`from_js_with_encoding_maybe_async`]. Writes into @@ -506,18 +551,10 @@ impl StringOrBuffer { encoding: Encoding, is_async: bool, allow_string_object: bool, + snapshot_volatile: bool, ) -> JsResult { if value.is_cell() && value.js_type().is_array_buffer_like() { - let buffer = if is_async { - Buffer::from_js_pinned(global, value) - .unwrap_or_else(|| Buffer::from_array_buffer(global, value)) - } else { - Buffer::from_array_buffer(global, value) - }; - if is_async { - buffer.buffer.value.protect(); - } - *out = Self::Buffer(buffer); + Self::array_buffer_into(out, global, value, is_async, snapshot_volatile); return Ok(true); } @@ -570,6 +607,7 @@ impl StringOrBuffer { encoding, is_async, allow_string_object, + true, )? { Ok(Some(out)) } else { diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index ec3d76a55648..2fb8241edd70 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -4032,15 +4032,8 @@ impl FormDataContext<'_> { let js_err = err.to_js(global_this); let _ = global_this.throw_value(js_err); } - Ok(mut result) => { + Ok(result) => { joiner.push_cloned(result.slice()); - // StringOrBuffer::Drop is a no-op for Buffer; release - // the readFile allocation explicitly. - if let crate::node::types::StringOrBuffer::Buffer(buf) = - &mut result - { - buf.destroy(); - } } } } diff --git a/src/runtime/webcore/fetch.rs b/src/runtime/webcore/fetch.rs index 526e44910f96..d44b99584111 100644 --- a/src/runtime/webcore/fetch.rs +++ b/src/runtime/webcore/fetch.rs @@ -1814,16 +1814,11 @@ fn fetch_impl( body.detach(); return Ok(rejected_value); } - Ok(mut result) => { + Ok(result) => { body.detach(); body = HTTPRequestBody::AnyBlob(blob::Any::from_owned_slice( result.slice().to_vec(), )); - // StringOrBuffer::Drop is a no-op for Buffer; release the - // readFile allocation now that the bytes are copied out. - if let crate::node::types::StringOrBuffer::Buffer(buf) = &mut result { - buf.destroy(); - } } } } diff --git a/test/js/bun/md/md-render-callback.test.ts b/test/js/bun/md/md-render-callback.test.ts index 7351d508360b..9d8547391459 100644 --- a/test/js/bun/md/md-render-callback.test.ts +++ b/test/js/bun/md/md-render-callback.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; const Markdown = Bun.markdown; @@ -426,4 +427,20 @@ describe("Bun.markdown buffer input", () => { input.buffer.transfer(); expect((input.buffer as ArrayBuffer).detached).toBe(true); }); + + test("a resizable input is read at call time even if an option getter resizes it to 0", async () => { + const script = ` + const bytes = new TextEncoder().encode("# Hello\\n\\nworld\\n" + Buffer.alloc(1 << 16, 0x20).toString()); + const input = new Uint8Array(new ArrayBuffer(bytes.length, { maxByteLength: bytes.length })); + input.set(bytes); + const fixed = Bun.markdown.html(Buffer.from(bytes), { autolinks: false }); + const out = Bun.markdown.html(input, { get autolinks() { input.buffer.resize(0); return false; } }); + console.log(out === fixed ? "OK" : "MISMATCH " + JSON.stringify(out.slice(0, 200))); + `; + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", script], env: bunEnv, stdout: "pipe", stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("OK"); + expect(exitCode).toBe(0); + }); }); diff --git a/test/js/node/crypto/scrypt.test.ts b/test/js/node/crypto/scrypt.test.ts index bb12b541db06..0cbac3a79b66 100644 --- a/test/js/node/crypto/scrypt.test.ts +++ b/test/js/node/crypto/scrypt.test.ts @@ -76,3 +76,84 @@ test("scrypt async does not leak callback/buffers when output allocation fails", expect(exitCode).toBe(0); }); + +// pin() guards transfer(), not ArrayBuffer.prototype.resize(); a later-arg getter calling resize(0) SIGSEGV'd the borrowed slice. +test("scrypt reads resizable ArrayBuffer inputs at capture time even if a later arg resizes them", async () => { + const script = ` + const crypto = require("node:crypto"); + const SIZE = 1 << 16; + const fixed = b => Buffer.alloc(SIZE, b); + const resizable = b => new Uint8Array(new ArrayBuffer(SIZE, { maxByteLength: SIZE })).fill(b); + + const fullPw = crypto.scryptSync(fixed(0x41), fixed(0x42).subarray(0, 16), 16, { N: 1024 }).toString("hex"); + const fullSalt = crypto.scryptSync(fixed(0x41).subarray(0, 16), fixed(0x42), 16, { N: 1024 }).toString("hex"); + + const out = {}; + + // sync: options.N getter resizes the captured password to 0. + { + const pw = resizable(0x41); + out.syncPw = crypto.scryptSync(pw, fixed(0x42).subarray(0, 16), 16, { + get N() { pw.buffer.resize(0); return 1024; }, + }).toString("hex") === fullPw; + } + // sync: options.N getter resizes the captured salt to 0. + { + const salt = resizable(0x42); + out.syncSalt = crypto.scryptSync(fixed(0x41).subarray(0, 16), salt, 16, { + get N() { salt.buffer.resize(0); return 1024; }, + }).toString("hex") === fullSalt; + } + // async: JS thread resizes the password after the job is queued. + { + const pw = resizable(0x41); + const p = new Promise((res, rej) => + crypto.scrypt(pw, fixed(0x42).subarray(0, 16), 16, { N: 1024 }, (e, k) => e ? rej(e) : res(k))); + pw.buffer.resize(0); + out.asyncPw = (await p).toString("hex") === fullPw; + } + // zero-length resizable input stays zero-length. + { + const pw = new Uint8Array(new ArrayBuffer(0, { maxByteLength: SIZE })); + out.syncEmpty = crypto.scryptSync(pw, fixed(0x42).subarray(0, 16), 16, { N: 1024 }).toString("hex") + === crypto.scryptSync(Buffer.alloc(0), fixed(0x42).subarray(0, 16), 16, { N: 1024 }).toString("hex"); + } + // growable SharedArrayBuffer is not snapshotted (grow-only, reading the + // captured extent stays valid) but still derives the same key. + { + const pw = new Uint8Array(new SharedArrayBuffer(SIZE, { maxByteLength: 2 * SIZE })).fill(0x41); + out.sab = crypto.scryptSync(pw, fixed(0x42).subarray(0, 16), 16, { N: 1024 }).toString("hex") === fullPw; + } + + // regression guard: passing a resizable output buffer to Bun.SHA256.hash + // still writes into the caller's buffer, not a private copy. + { + const dst = new Uint8Array(new ArrayBuffer(32, { maxByteLength: 64 })); + const ret = Bun.SHA256.hash(fixed(0x41).subarray(0, 5), dst); + const want = Bun.SHA256.hash(fixed(0x41).subarray(0, 5)); + out.hashOutput = ret === dst && Buffer.from(dst).equals(Buffer.from(want)); + } + + console.log(JSON.stringify(out)); + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", script], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + expect(JSON.parse(stdout.trim())).toEqual({ + syncPw: true, + syncSalt: true, + asyncPw: true, + syncEmpty: true, + sab: true, + hashOutput: true, + }); + expect(exitCode).toBe(0); +}); diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 16396f0dd211..275448f1b354 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -5640,6 +5640,34 @@ it("fs.write keeps the source buffer attached while the write is in flight", asy expect(readFileSync(file, "latin1")).toBe("DDDDDDDD"); }); +it("fs.write writes the bytes captured at call time when a resizable source is resized to 0 while in flight", async () => { + using dir = tempDir("fs-write-resizable", { + "run.js": ` + const fs = require("node:fs"); + const fd = fs.openSync("out.bin", "w"); + const buf = new Uint8Array(new ArrayBuffer(1 << 16, { maxByteLength: 1 << 16 })).fill(0x44); + fs.write(fd, buf, 0, buf.byteLength, 0, (err, written) => { + fs.closeSync(fd); + if (err) throw err; + const got = fs.readFileSync("out.bin"); + console.log(JSON.stringify({ written, len: got.length, allD: got.every(b => b === 0x44) })); + }); + buf.buffer.resize(0); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "run.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout.trim())).toEqual({ written: 1 << 16, len: 1 << 16, allD: true }); + expect(exitCode).toBe(0); +}); + it("fs.promises.writeFile keeps the source buffer attached while the write is in flight", async () => { using dir = tempDir("fs-writefile-pin", {}); const file = join(String(dir), "out.bin");