From bf8017c6c27642128d0d58b1cda50edb9fcd7a1e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:33:04 +0000 Subject: [PATCH 1/5] crypto,util: re-snapshot input buffers after later-arg coercion Bun.CryptoHasher.hash, Bun.SHA*.hash, Bun.sha, Bun.randomUUIDv5 and Bun.indexOfLine all captured an ArrayBuffer-backed input slice and then coerced a later argument that can run JS (a boxed String's toString or an object's valueOf). That JS can transfer(0) the input's backing ArrayBuffer and spray a same-size allocation, leaving the captured ptr/len pointing at a recycled foreign block. The digest/scan then runs over freed memory. For the hash entry points, add StringOrBuffer::refresh_buffer and re-snapshot the input's ArrayBuffer from its JSValue after the output argument has been coerced, so a detached input is observed as length 0. For randomUUIDv5 and indexOfLine, decode the later argument (namespace / offset) before snapshotting the buffer; the namespace result is copied into a local [u8; 16] and offset is a plain integer, so the reverse ordering is not exposed to the same hazard. --- src/runtime/api/BunObject.rs | 15 ++++--- src/runtime/crypto/CryptoHasher.rs | 12 ++++- src/runtime/node/types.rs | 24 ++++++++++ src/runtime/webcore/Crypto.rs | 45 ++++++++++--------- test/js/bun/util/bun-cryptohasher.test.ts | 54 +++++++++++++++++++++++ test/js/bun/util/index-of-line.test.ts | 20 +++++++++ test/js/bun/util/randomUUIDv5.test.ts | 20 +++++++++ 7 files changed, 162 insertions(+), 28 deletions(-) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index e3fbc234c67c..5c9aff32db6c 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -230,7 +230,7 @@ mod static_adapters { // re-enters the VM). let _a0_guard = a0.protected(); let _a1_guard = a1.protected(); - let Some(input) = BlobOrStringOrBuffer::from_js(g, a0)? else { + let Some(mut input) = BlobOrStringOrBuffer::from_js(g, a0)? else { return Err(g.throw_invalid_arguments(format_args!( "expected string, buffer, TypedArray, or Blob", ))); @@ -240,6 +240,9 @@ mod static_adapters { } else { StringOrBuffer::from_js(g, a1)? }; + // `StringOrBuffer::from_js` above may call a boxed String's `toString`, + // which can detach `input`'s backing ArrayBuffer (use-after-free). + input.refresh_buffer(g); Crypto::SHA512_256::hash_(g, &input, output) } } @@ -1517,16 +1520,18 @@ pub(crate) fn index_of_line( return Ok(JSValue::js_number_from_int32(-1)); } - let Some(buffer) = arguments[0].as_array_buffer(global_this) else { - return Ok(JSValue::js_number_from_int32(-1)); - }; - + // Coerce `offset` before snapshotting the buffer: `coerce_to_int64` can run + // `valueOf`/`toString`, which may detach `arguments[0]` (use-after-free). let mut offset: usize = 0; if arguments.len() > 1 { let offset_value = arguments[1].coerce_to_int64(global_this)?; offset = offset_value.max(0) as usize; } + let Some(buffer) = arguments[0].as_array_buffer(global_this) else { + return Ok(JSValue::js_number_from_int32(-1)); + }; + let bytes = buffer.byte_slice(); let mut current_offset = offset; let end = bytes.len() as u32; diff --git a/src/runtime/crypto/CryptoHasher.rs b/src/runtime/crypto/CryptoHasher.rs index eb19f059a3f7..70f0ba4985bb 100644 --- a/src/runtime/crypto/CryptoHasher.rs +++ b/src/runtime/crypto/CryptoHasher.rs @@ -265,7 +265,7 @@ impl CryptoHasher { }; // Node.BlobOrStringOrBuffer - let input = { + let mut input = { let Some(arg) = next_eat() else { return Err( global.throw_invalid_arguments(format_args!("expected blob, string or buffer")) @@ -296,6 +296,10 @@ impl CryptoHasher { None => None, }; + // `StringOrBuffer::from_js` above may call a boxed String's `toString`, + // which can detach `input`'s backing ArrayBuffer (use-after-free). + input.refresh_buffer(global); + Self::hash_(global, algorithm, &input, output) } @@ -1236,7 +1240,7 @@ impl StaticCryptoHasher { }; // Node.BlobOrStringOrBuffer - let input = { + let mut input = { let Some(arg) = next_eat() else { return Err( global.throw_invalid_arguments(format_args!("expected blob, string or buffer")) @@ -1267,6 +1271,10 @@ impl StaticCryptoHasher { None => None, }; + // `StringOrBuffer::from_js` above may call a boxed String's `toString`, + // which can detach `input`'s backing ArrayBuffer (use-after-free). + input.refresh_buffer(global); + Self::hash_(global, &input, output) } diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index 475ef78c71b9..dc347288ed32 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -85,6 +85,14 @@ impl BlobOrStringOrBuffer { } } + /// See [`StringOrBuffer::refresh_buffer`]. + #[inline] + pub fn refresh_buffer(&mut self, global: &JSGlobalObject) { + if let Self::StringOrBuffer(sob) = self { + sob.refresh_buffer(global); + } + } + pub fn protect(&self) { match self { Self::StringOrBuffer(sob) => sob.protect(), @@ -259,6 +267,22 @@ impl StringOrBuffer { Self::Buffer(str) => str.slice(), } } + + /// Re-snapshot the `Buffer` variant's backing `ArrayBuffer` from its + /// underlying `JSValue`. Call after coercing a later argument whose + /// `toString`/`valueOf` may have detached the backing store. + #[inline] + pub fn refresh_buffer(&mut self, global: &JSGlobalObject) { + if let Self::Buffer(buf) = self { + if let Some(fresh) = buf.buffer.value.as_array_buffer(global) { + buf.buffer = fresh; + } else { + buf.buffer.ptr = core::ptr::null_mut(); + buf.buffer.len = 0; + buf.buffer.byte_len = 0; + } + } + } } impl Drop for StringOrBuffer { diff --git a/src/runtime/webcore/Crypto.rs b/src/runtime/webcore/Crypto.rs index 2687adbabe14..d7ea2fb67330 100644 --- a/src/runtime/webcore/Crypto.rs +++ b/src/runtime/webcore/Crypto.rs @@ -351,27 +351,9 @@ pub(crate) fn bun_random_uuid_v5( let name_value = arguments.ptr[0]; let namespace_value = arguments.ptr[1]; - // `bun_core::ZigStringSlice` is a borrow-or-own UTF-8 slice. - let name: bun_core::ZigStringSlice = 'brk: { - if name_value.is_string() { - let name_str = bun_core::OwnedString::new(name_value.to_bun_string(global)?); - let result = name_str.to_utf8(); - - break 'brk result; - } else if let Some(array_buffer) = name_value.as_array_buffer(global) { - let bytes: &[u8] = array_buffer.byte_slice(); - break 'brk bun_core::ZigStringSlice::from_utf8_never_free(bytes); - } else { - return Err(global - .err( - bun_jsc::ErrorCode::INVALID_ARG_TYPE, - format_args!("The \"name\" argument must be of type string or BufferSource"), - ) - .throw()); - } - }; - // `defer name.deinit()` — Utf8Slice's Drop handles cleanup. - + // Decode `namespace` first: its `to_bun_string` can call a boxed String's + // `toString`, which may detach `name`'s backing ArrayBuffer. `namespace` + // is copied to a local `[u8; 16]`, so it's safe against the reverse. let namespace: [u8; 16] = 'brk: { if namespace_value.is_string() { let namespace_str = bun_core::OwnedString::new(namespace_value.to_bun_string(global)?); @@ -420,6 +402,27 @@ pub(crate) fn bun_random_uuid_v5( .throw()); }; + // `bun_core::ZigStringSlice` is a borrow-or-own UTF-8 slice. + let name: bun_core::ZigStringSlice = 'brk: { + if name_value.is_string() { + let name_str = bun_core::OwnedString::new(name_value.to_bun_string(global)?); + let result = name_str.to_utf8(); + + break 'brk result; + } else if let Some(array_buffer) = name_value.as_array_buffer(global) { + let bytes: &[u8] = array_buffer.byte_slice(); + break 'brk bun_core::ZigStringSlice::from_utf8_never_free(bytes); + } else { + return Err(global + .err( + bun_jsc::ErrorCode::INVALID_ARG_TYPE, + format_args!("The \"name\" argument must be of type string or BufferSource"), + ) + .throw()); + } + }; + // `defer name.deinit()` — Utf8Slice's Drop handles cleanup. + let uuid = UUID5::init(&namespace, name.slice()); if encoding == Encoding::Hex { diff --git a/test/js/bun/util/bun-cryptohasher.test.ts b/test/js/bun/util/bun-cryptohasher.test.ts index 8c6ab9ca468f..cc0ef9072b5f 100644 --- a/test/js/bun/util/bun-cryptohasher.test.ts +++ b/test/js/bun/util/bun-cryptohasher.test.ts @@ -1,6 +1,60 @@ import { describe, expect, test } from "bun:test"; import { withoutAggressiveGC } from "harness"; +describe("input buffer detached by output argument's toString", () => { + // A boxed String's `toString` runs during output-encoding coercion. If the + // input buffer's slice was snapshotted before that, detaching it here leaves + // the hasher reading freed memory. After the fix, the input is re-snapshotted + // as detached (length 0), so the result is the digest of the empty input. + const N = 1 << 16; + const keep: Uint8Array[] = []; + const mk = () => Buffer.from(new ArrayBuffer(N)).fill(0x41); + const evilEncoding = (victim: Buffer, s: string) => + Object.assign(new String(s), { + toString() { + victim.buffer.transfer(0); + for (let i = 0; i < 8; i++) keep.push(new Uint8Array(N).fill(0x5a)); + return s; + }, + }) as string; + + test("Bun.CryptoHasher.hash", () => { + const b = mk(); + const got = Bun.CryptoHasher.hash("sha256", b, evilEncoding(b, "hex")); + expect(b.byteLength).toBe(0); + expect(got).toBe(Bun.CryptoHasher.hash("sha256", new Uint8Array(0), "hex")); + }); + + test("Bun.SHA256.hash", () => { + const b = mk(); + const got = Bun.SHA256.hash(b, evilEncoding(b, "hex")); + expect(b.byteLength).toBe(0); + expect(got).toBe(Bun.SHA256.hash(new Uint8Array(0), "hex")); + }); + + test("Bun.sha", () => { + const b = mk(); + const got = Bun.sha(b, evilEncoding(b, "hex")); + expect(b.byteLength).toBe(0); + expect(got).toBe(Bun.sha(new Uint8Array(0), "hex")); + }); + + test("Bun.CryptoHasher.hash (output=Uint8Array, input=String object)", () => { + // Reverse direction stays safe: input is a boxed String whose toString + // detaches the output buffer before it is snapshotted. The output buffer + // is captured as detached (length 0) and a length check throws. + const out = Buffer.from(new ArrayBuffer(32)); + const evilInput = Object.assign(new String("A"), { + toString() { + out.buffer.transfer(0); + return "A"; + }, + }); + expect(() => Bun.CryptoHasher.hash("sha256", evilInput as unknown as string, out)).toThrow(); + expect(out.byteLength).toBe(0); + }); +}); + test("Bun.file in CryptoHasher is not supported yet", () => { expect(() => Bun.SHA1.hash(Bun.file(import.meta.path))).toThrow(); expect(() => Bun.CryptoHasher.hash("sha1", Bun.file(import.meta.path))).toThrow(); diff --git a/test/js/bun/util/index-of-line.test.ts b/test/js/bun/util/index-of-line.test.ts index b8bae60339af..d8928f4c1a71 100644 --- a/test/js/bun/util/index-of-line.test.ts +++ b/test/js/bun/util/index-of-line.test.ts @@ -85,6 +85,26 @@ test("indexOfLine is linear on large input with a non-ASCII byte", async () => { expect(proc.signalCode).toBeNull(); }, 60_000); +test("indexOfLine coerces offset before snapshotting the buffer", () => { + // `offset.valueOf()` can detach the buffer. Before the fix, the stale + // snapshot was scanned (reading freed memory); now the buffer is captured + // after coercion and seen as detached (length 0) so the result is -1. + const N = 1 << 16; + const keep: Uint8Array[] = []; + const b = Buffer.from(new ArrayBuffer(N)).fill(0x41); + b[100] = 0x0a; + const evilOffset = { + valueOf() { + b.buffer.transfer(0); + for (let i = 0; i < 8; i++) keep.push(new Uint8Array(N).fill(0x0a)); + return 0; + }, + }; + const got = indexOfLine(b, evilOffset as unknown as number); + expect(b.byteLength).toBe(0); + expect(got).toBe(-1); +}); + test("indexOfLine skips multi-byte sequences correctly", () => { // ascii prefix, multi-byte char, ascii, newline const buf = Buffer.from("abé d\n"); diff --git a/test/js/bun/util/randomUUIDv5.test.ts b/test/js/bun/util/randomUUIDv5.test.ts index 769a98399786..780549f6478f 100644 --- a/test/js/bun/util/randomUUIDv5.test.ts +++ b/test/js/bun/util/randomUUIDv5.test.ts @@ -367,6 +367,26 @@ describe("randomUUIDv5", () => { expect(result).toEqual(uuid.v5("test", uuid.v5.DNS)); }); + test("name buffer detached by namespace argument's toString", () => { + // A boxed String namespace's `toString` runs during coercion. Before the + // fix, the name buffer's slice was snapshotted first, so detaching it here + // left UUID5 reading freed memory. Now namespace is decoded first and the + // name buffer is captured as detached (length 0). + const N = 1 << 16; + const keep: Uint8Array[] = []; + const b = Buffer.from(new ArrayBuffer(N)).fill(0x41); + const evilNamespace = Object.assign(new String(dnsNamespace), { + toString() { + b.buffer.transfer(0); + for (let i = 0; i < 8; i++) keep.push(new Uint8Array(N).fill(0x5a)); + return dnsNamespace; + }, + }) as string; + const got = Bun.randomUUIDv5(b, evilNamespace); + expect(b.byteLength).toBe(0); + expect(got).toBe(Bun.randomUUIDv5(new Uint8Array(0), dnsNamespace)); + }); + test("consistent across multiple calls", () => { const results: string[] = []; for (let i = 0; i < 100; i++) { From 3cbb45d4b0707c0908b9739843f96da58579a57d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:49:51 +0000 Subject: [PATCH 2/5] ci: retrigger From 8dda462960d28d6b7545723d08ab3bb474c89865 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 21 Jul 2026 16:01:46 +0000 Subject: [PATCH 3/5] crypto: also re-snapshot password buffer in Bun.password.verifySync Same pattern as the hash entry points: password's ArrayBuffer is snapshotted via StringOrBuffer::from_js, then the hash argument is coerced (a boxed String's toString can detach the password buffer), then password.slice() is read. Re-snapshot password after the hash argument has been decoded. Also tighten the new tests: trim comments to the invariant being asserted and check the specific error message in the reverse-direction case. --- src/runtime/crypto/PasswordObject.rs | 6 +++++- test/js/bun/util/bun-cryptohasher.test.ts | 15 +++++++-------- test/js/bun/util/index-of-line.test.ts | 5 ++--- test/js/bun/util/password.test.ts | 19 +++++++++++++++++++ test/js/bun/util/randomUUIDv5.test.ts | 6 ++---- 5 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/runtime/crypto/PasswordObject.rs b/src/runtime/crypto/PasswordObject.rs index 9409a40ba3e0..59df84ebd4c9 100644 --- a/src/runtime/crypto/PasswordObject.rs +++ b/src/runtime/crypto/PasswordObject.rs @@ -929,7 +929,7 @@ pub(crate) fn js_password_object_verify_sync( }; } - let Some(password) = StringOrBuffer::from_js(global_object, arguments[0])? else { + let Some(mut password) = StringOrBuffer::from_js(global_object, arguments[0])? else { return Err(global_object.throw_invalid_argument_type( "verify", "password", @@ -946,6 +946,10 @@ pub(crate) fn js_password_object_verify_sync( )); }; + // `StringOrBuffer::from_js` above may call a boxed String's `toString`, + // which can detach `password`'s backing ArrayBuffer (use-after-free). + password.refresh_buffer(global_object); + // defer password.deinit() / hash_.deinit() — Drop at scope exit. if hash_.slice().is_empty() { diff --git a/test/js/bun/util/bun-cryptohasher.test.ts b/test/js/bun/util/bun-cryptohasher.test.ts index cc0ef9072b5f..e7f1700a2e5f 100644 --- a/test/js/bun/util/bun-cryptohasher.test.ts +++ b/test/js/bun/util/bun-cryptohasher.test.ts @@ -2,10 +2,8 @@ import { describe, expect, test } from "bun:test"; import { withoutAggressiveGC } from "harness"; describe("input buffer detached by output argument's toString", () => { - // A boxed String's `toString` runs during output-encoding coercion. If the - // input buffer's slice was snapshotted before that, detaching it here leaves - // the hasher reading freed memory. After the fix, the input is re-snapshotted - // as detached (length 0), so the result is the digest of the empty input. + // Detaching the input during output-arg coercion must be observed as + // length 0, so the result is the digest of the empty input. const N = 1 << 16; const keep: Uint8Array[] = []; const mk = () => Buffer.from(new ArrayBuffer(N)).fill(0x41); @@ -40,9 +38,8 @@ describe("input buffer detached by output argument's toString", () => { }); test("Bun.CryptoHasher.hash (output=Uint8Array, input=String object)", () => { - // Reverse direction stays safe: input is a boxed String whose toString - // detaches the output buffer before it is snapshotted. The output buffer - // is captured as detached (length 0) and a length check throws. + // Reverse direction: input's toString detaches the output buffer, which + // is then captured at length 0 and rejected by the length check. const out = Buffer.from(new ArrayBuffer(32)); const evilInput = Object.assign(new String("A"), { toString() { @@ -50,7 +47,9 @@ describe("input buffer detached by output argument's toString", () => { return "A"; }, }); - expect(() => Bun.CryptoHasher.hash("sha256", evilInput as unknown as string, out)).toThrow(); + expect(() => Bun.CryptoHasher.hash("sha256", evilInput as unknown as string, out)).toThrow( + /TypedArray must be at least 32 bytes/, + ); expect(out.byteLength).toBe(0); }); }); diff --git a/test/js/bun/util/index-of-line.test.ts b/test/js/bun/util/index-of-line.test.ts index d8928f4c1a71..aa307b4d3434 100644 --- a/test/js/bun/util/index-of-line.test.ts +++ b/test/js/bun/util/index-of-line.test.ts @@ -86,9 +86,8 @@ test("indexOfLine is linear on large input with a non-ASCII byte", async () => { }, 60_000); test("indexOfLine coerces offset before snapshotting the buffer", () => { - // `offset.valueOf()` can detach the buffer. Before the fix, the stale - // snapshot was scanned (reading freed memory); now the buffer is captured - // after coercion and seen as detached (length 0) so the result is -1. + // Detaching the buffer during offset coercion must be observed as + // length 0, so the result is -1. const N = 1 << 16; const keep: Uint8Array[] = []; const b = Buffer.from(new ArrayBuffer(N)).fill(0x41); diff --git a/test/js/bun/util/password.test.ts b/test/js/bun/util/password.test.ts index 0aa020fa71c4..7e71d8f3560d 100644 --- a/test/js/bun/util/password.test.ts +++ b/test/js/bun/util/password.test.ts @@ -424,3 +424,22 @@ test("verify rejects encoded argon2 hashes with cost parameters above the suppor expect(() => password.verifySync("correct horse", hugeParallelism)).toThrow("WeakParameters"); await expect(password.verify("correct horse", hugeParallelism)).rejects.toThrow("WeakParameters"); }); + +test("verifySync: password buffer detached by hash argument's toString", () => { + // Detaching the password buffer during hash coercion must be observed as + // length 0, so verification short-circuits to false. + const N = 1 << 16; + const keep: Uint8Array[] = []; + const hashed = password.hashSync(Buffer.alloc(N, 0x5a), "bcrypt"); + const b = Buffer.from(new ArrayBuffer(N)).fill(0x41); + const evilHash = Object.assign(new String(hashed), { + toString() { + b.buffer.transfer(0); + for (let i = 0; i < 8; i++) keep.push(new Uint8Array(N).fill(0x5a)); + return hashed; + }, + }) as string; + const got = password.verifySync(b, evilHash); + expect(b.byteLength).toBe(0); + expect(got).toBe(false); +}); diff --git a/test/js/bun/util/randomUUIDv5.test.ts b/test/js/bun/util/randomUUIDv5.test.ts index 780549f6478f..18fd12731ccc 100644 --- a/test/js/bun/util/randomUUIDv5.test.ts +++ b/test/js/bun/util/randomUUIDv5.test.ts @@ -368,10 +368,8 @@ describe("randomUUIDv5", () => { }); test("name buffer detached by namespace argument's toString", () => { - // A boxed String namespace's `toString` runs during coercion. Before the - // fix, the name buffer's slice was snapshotted first, so detaching it here - // left UUID5 reading freed memory. Now namespace is decoded first and the - // name buffer is captured as detached (length 0). + // Detaching the name buffer during namespace coercion must be observed + // as length 0, so the result is UUIDv5 of the empty name. const N = 1 << 16; const keep: Uint8Array[] = []; const b = Buffer.from(new ArrayBuffer(N)).fill(0x41); From 03d272c85d125d8428836265d41c63fff78e22ca Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 01:05:25 +0000 Subject: [PATCH 4/5] crypto,util: coerce later arguments before capturing the input buffer Replace the post-coercion re-snapshot with a decode order that runs all user JS before any ArrayBuffer slice is captured: * Bun.CryptoHasher.hash / Bun.SHA*.hash / Bun.sha: decode the output argument first, then the input via BlobOrStringOrBuffer::from_js_no_string_object so the input decode never calls user toString and the already-captured output buffer stays valid. * Bun.password.verifySync: decode the hash argument first, then the password with allow_string_object = false. * Bun.randomUUIDv5 / Bun.indexOfLine were already reordered in the previous commits. BlobOrStringOrBuffer::from_js_maybe_file_maybe_async now takes allow_string_object and from_js_no_string_object is added as a convenience wrapper. Boxed String inputs to the hash/verifySync entry points are now rejected (matching Node's ERR_INVALID_ARG_TYPE for crypto.Hash#update). --- src/runtime/api/BunObject.rs | 17 +-- src/runtime/crypto/CryptoHasher.rs | 121 +++++++++------------- src/runtime/crypto/PasswordObject.rs | 20 ++-- src/runtime/node/types.rs | 46 ++++---- test/js/bun/util/bun-cryptohasher.test.ts | 10 +- 5 files changed, 94 insertions(+), 120 deletions(-) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 5c9aff32db6c..fb3d2ea4753d 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -230,19 +230,20 @@ mod static_adapters { // re-enters the VM). let _a0_guard = a0.protected(); let _a1_guard = a1.protected(); - let Some(mut input) = BlobOrStringOrBuffer::from_js(g, a0)? else { - return Err(g.throw_invalid_arguments(format_args!( - "expected string, buffer, TypedArray, or Blob", - ))); - }; + // Coerce the output argument first: `StringOrBuffer::from_js` can call a + // boxed String's `toString`, which may detach the input's ArrayBuffer. let output = if a1.is_undefined_or_null() { None } else { StringOrBuffer::from_js(g, a1)? }; - // `StringOrBuffer::from_js` above may call a boxed String's `toString`, - // which can detach `input`'s backing ArrayBuffer (use-after-free). - input.refresh_buffer(g); + // `from_js_no_string_object` never runs user JS, so `output`'s buffer + // (captured above) stays valid. + let Some(input) = BlobOrStringOrBuffer::from_js_no_string_object(g, a0)? else { + return Err(g.throw_invalid_arguments(format_args!( + "expected string, buffer, TypedArray, or Blob", + ))); + }; Crypto::SHA512_256::hash_(g, &input, output) } } diff --git a/src/runtime/crypto/CryptoHasher.rs b/src/runtime/crypto/CryptoHasher.rs index 70f0ba4985bb..24a89a9edc98 100644 --- a/src/runtime/crypto/CryptoHasher.rs +++ b/src/runtime/crypto/CryptoHasher.rs @@ -242,47 +242,30 @@ impl CryptoHasher { /// Hand-expanded static-method argument decode for the parameter list /// `(algorithm string, input, optional output buffer/encoding)`. pub fn hash(global: &JSGlobalObject, callframe: &CallFrame) -> JsResult { - let arguments = callframe.arguments_old::<3>(); - let mut i = 0usize; - let mut next_eat = || { - if i < arguments.len { - let v = arguments.ptr[i]; - i += 1; - Some(v) - } else { - None - } - }; + let arguments = callframe.arguments_undef::<3>(); let algorithm = { - let Some(string_value) = next_eat() else { - return Err(global.throw_invalid_arguments(format_args!("Missing argument"))); - }; + let string_value = arguments.ptr[0]; if string_value.is_undefined_or_null() { + if arguments.len == 0 { + return Err(global.throw_invalid_arguments(format_args!("Missing argument"))); + } return Err(global.throw_invalid_arguments(format_args!("Expected string"))); } string_value.get_zig_string(global)? }; - // Node.BlobOrStringOrBuffer - let mut input = { - let Some(arg) = next_eat() else { - return Err( - global.throw_invalid_arguments(format_args!("expected blob, string or buffer")) - ); - }; - match BlobOrStringOrBuffer::from_js(global, arg)? { - Some(b) => b, - None => { - return Err(global - .throw_invalid_arguments(format_args!("expected blob, string or buffer"))); - } - } - }; + if arguments.len < 2 { + return Err( + global.throw_invalid_arguments(format_args!("expected blob, string or buffer")) + ); + } - // ?Node.StringOrBuffer (static-method arm: only `undefined` → None) - let output: Option = match next_eat() { - Some(arg) => match StringOrBuffer::from_js(global, arg)? { + // Coerce the output argument first: `StringOrBuffer::from_js` can call a + // boxed String's `toString`, which may detach the input's ArrayBuffer. + let output: Option = if arguments.len > 2 { + let arg = arguments.ptr[2]; + match StringOrBuffer::from_js(global, arg)? { Some(v) => Some(v), None => { if arg.is_undefined() { @@ -292,13 +275,20 @@ impl CryptoHasher { .throw_invalid_arguments(format_args!("expected string or buffer"))); } } - }, - None => None, + } + } else { + None }; - // `StringOrBuffer::from_js` above may call a boxed String's `toString`, - // which can detach `input`'s backing ArrayBuffer (use-after-free). - input.refresh_buffer(global); + // `from_js_no_string_object` never runs user JS, so `output`'s buffer + // (captured above) stays valid. + let input = match BlobOrStringOrBuffer::from_js_no_string_object(global, arguments.ptr[1])? { + Some(b) => b, + None => { + return Err(global + .throw_invalid_arguments(format_args!("expected blob, string or buffer"))); + } + }; Self::hash_(global, algorithm, &input, output) } @@ -1227,37 +1217,19 @@ impl StaticCryptoHasher { /// Hand-expanded `wrapStaticMethod` decode for the parameter list /// `(*JSGlobalObject, Node.BlobOrStringOrBuffer, ?Node.StringOrBuffer)`. pub fn hash(global: &JSGlobalObject, callframe: &CallFrame) -> JsResult { - let arguments = callframe.arguments_old::<2>(); - let mut i = 0usize; - let mut next_eat = || { - if i < arguments.len { - let v = arguments.ptr[i]; - i += 1; - Some(v) - } else { - None - } - }; + let arguments = callframe.arguments_undef::<2>(); - // Node.BlobOrStringOrBuffer - let mut input = { - let Some(arg) = next_eat() else { - return Err( - global.throw_invalid_arguments(format_args!("expected blob, string or buffer")) - ); - }; - match BlobOrStringOrBuffer::from_js(global, arg)? { - Some(b) => b, - None => { - return Err(global - .throw_invalid_arguments(format_args!("expected blob, string or buffer"))); - } - } - }; + if arguments.len == 0 { + return Err( + global.throw_invalid_arguments(format_args!("expected blob, string or buffer")) + ); + } - // ?Node.StringOrBuffer (static-method arm: only `undefined` → None) - let output: Option = match next_eat() { - Some(arg) => match StringOrBuffer::from_js(global, arg)? { + // Coerce the output argument first: `StringOrBuffer::from_js` can call a + // boxed String's `toString`, which may detach the input's ArrayBuffer. + let output: Option = if arguments.len > 1 { + let arg = arguments.ptr[1]; + match StringOrBuffer::from_js(global, arg)? { Some(v) => Some(v), None => { if arg.is_undefined() { @@ -1267,13 +1239,20 @@ impl StaticCryptoHasher { .throw_invalid_arguments(format_args!("expected string or buffer"))); } } - }, - None => None, + } + } else { + None }; - // `StringOrBuffer::from_js` above may call a boxed String's `toString`, - // which can detach `input`'s backing ArrayBuffer (use-after-free). - input.refresh_buffer(global); + // `from_js_no_string_object` never runs user JS, so `output`'s buffer + // (captured above) stays valid. + let input = match BlobOrStringOrBuffer::from_js_no_string_object(global, arguments.ptr[0])? { + Some(b) => b, + None => { + return Err(global + .throw_invalid_arguments(format_args!("expected blob, string or buffer"))); + } + }; Self::hash_(global, &input, output) } diff --git a/src/runtime/crypto/PasswordObject.rs b/src/runtime/crypto/PasswordObject.rs index 59df84ebd4c9..90bfa106196e 100644 --- a/src/runtime/crypto/PasswordObject.rs +++ b/src/runtime/crypto/PasswordObject.rs @@ -929,27 +929,29 @@ pub(crate) fn js_password_object_verify_sync( }; } - let Some(mut password) = StringOrBuffer::from_js(global_object, arguments[0])? else { + // Coerce the hash argument first: `StringOrBuffer::from_js` can call a + // boxed String's `toString`, which may detach the password's ArrayBuffer. + let Some(hash_) = StringOrBuffer::from_js(global_object, arguments[1])? else { return Err(global_object.throw_invalid_argument_type( "verify", - "password", + "hash", "string or TypedArray", )); }; - let Some(hash_) = StringOrBuffer::from_js(global_object, arguments[1])? else { - drop(password); + // `allow_string_object = false`: the password decode never runs user JS, so + // `hash_`'s buffer (captured above) stays valid. + let Some(password) = + StringOrBuffer::from_js_maybe_async(global_object, arguments[0], false, false)? + else { + drop(hash_); return Err(global_object.throw_invalid_argument_type( "verify", - "hash", + "password", "string or TypedArray", )); }; - // `StringOrBuffer::from_js` above may call a boxed String's `toString`, - // which can detach `password`'s backing ArrayBuffer (use-after-free). - password.refresh_buffer(global_object); - // defer password.deinit() / hash_.deinit() — Drop at scope exit. if hash_.slice().is_empty() { diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index dc347288ed32..9f9bd446eb65 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -85,14 +85,6 @@ impl BlobOrStringOrBuffer { } } - /// See [`StringOrBuffer::refresh_buffer`]. - #[inline] - pub fn refresh_buffer(&mut self, global: &JSGlobalObject) { - if let Self::StringOrBuffer(sob) = self { - sob.refresh_buffer(global); - } - } - pub fn protect(&self) { match self { Self::StringOrBuffer(sob) => sob.protect(), @@ -109,9 +101,15 @@ impl BlobOrStringOrBuffer { value: JSValue, allow_file: bool, is_async: bool, + allow_string_object: bool, ) -> JsResult> { // Check StringOrBuffer first because it's more common and cheaper. - let str = match StringOrBuffer::from_js_maybe_async(global, value, is_async, true)? { + let str = match StringOrBuffer::from_js_maybe_async( + global, + value, + is_async, + allow_string_object, + )? { Some(s) => s, None => { // `as_class_ref` is the safe shared-borrow downcast (centralised @@ -149,7 +147,7 @@ impl BlobOrStringOrBuffer { value: JSValue, allow_file: bool, ) -> JsResult> { - Self::from_js_maybe_file_maybe_async(global, value, allow_file, false) + Self::from_js_maybe_file_maybe_async(global, value, allow_file, false, true) } pub fn from_js( @@ -159,11 +157,21 @@ impl BlobOrStringOrBuffer { Self::from_js_maybe_file(global, value, true) } + /// [`from_js`] with `allow_string_object = false`: boxed `String` inputs are + /// rejected, so this never calls user `toString` and is safe to call after + /// an earlier argument's ArrayBuffer slice has been captured. + pub fn from_js_no_string_object( + global: &JSGlobalObject, + value: JSValue, + ) -> JsResult> { + Self::from_js_maybe_file_maybe_async(global, value, true, false, false) + } + pub fn from_js_async( global: &JSGlobalObject, value: JSValue, ) -> JsResult> { - Self::from_js_maybe_file_maybe_async(global, value, true, true) + Self::from_js_maybe_file_maybe_async(global, value, true, true, true) } pub fn from_js_with_encoding_value( @@ -267,22 +275,6 @@ impl StringOrBuffer { Self::Buffer(str) => str.slice(), } } - - /// Re-snapshot the `Buffer` variant's backing `ArrayBuffer` from its - /// underlying `JSValue`. Call after coercing a later argument whose - /// `toString`/`valueOf` may have detached the backing store. - #[inline] - pub fn refresh_buffer(&mut self, global: &JSGlobalObject) { - if let Self::Buffer(buf) = self { - if let Some(fresh) = buf.buffer.value.as_array_buffer(global) { - buf.buffer = fresh; - } else { - buf.buffer.ptr = core::ptr::null_mut(); - buf.buffer.len = 0; - buf.buffer.byte_len = 0; - } - } - } } impl Drop for StringOrBuffer { diff --git a/test/js/bun/util/bun-cryptohasher.test.ts b/test/js/bun/util/bun-cryptohasher.test.ts index e7f1700a2e5f..4eaa71eb634c 100644 --- a/test/js/bun/util/bun-cryptohasher.test.ts +++ b/test/js/bun/util/bun-cryptohasher.test.ts @@ -37,9 +37,9 @@ describe("input buffer detached by output argument's toString", () => { expect(got).toBe(Bun.sha(new Uint8Array(0), "hex")); }); - test("Bun.CryptoHasher.hash (output=Uint8Array, input=String object)", () => { - // Reverse direction: input's toString detaches the output buffer, which - // is then captured at length 0 and rejected by the length check. + test("Bun.CryptoHasher.hash rejects boxed String as input", () => { + // Boxed `String` inputs are rejected so the input decode never runs user + // JS, keeping the already-captured output buffer valid. const out = Buffer.from(new ArrayBuffer(32)); const evilInput = Object.assign(new String("A"), { toString() { @@ -48,9 +48,9 @@ describe("input buffer detached by output argument's toString", () => { }, }); expect(() => Bun.CryptoHasher.hash("sha256", evilInput as unknown as string, out)).toThrow( - /TypedArray must be at least 32 bytes/, + /expected blob, string or buffer/, ); - expect(out.byteLength).toBe(0); + expect(out.byteLength).toBe(32); }); }); From 3557f2a216c532d33461da839298d882a4ae8ce5 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 01:07:20 +0000 Subject: [PATCH 5/5] [autofix.ci] apply automated fixes --- src/runtime/crypto/CryptoHasher.rs | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/runtime/crypto/CryptoHasher.rs b/src/runtime/crypto/CryptoHasher.rs index 24a89a9edc98..fd3ac27d6244 100644 --- a/src/runtime/crypto/CryptoHasher.rs +++ b/src/runtime/crypto/CryptoHasher.rs @@ -282,13 +282,14 @@ impl CryptoHasher { // `from_js_no_string_object` never runs user JS, so `output`'s buffer // (captured above) stays valid. - let input = match BlobOrStringOrBuffer::from_js_no_string_object(global, arguments.ptr[1])? { - Some(b) => b, - None => { - return Err(global - .throw_invalid_arguments(format_args!("expected blob, string or buffer"))); - } - }; + let input = + match BlobOrStringOrBuffer::from_js_no_string_object(global, arguments.ptr[1])? { + Some(b) => b, + None => { + return Err(global + .throw_invalid_arguments(format_args!("expected blob, string or buffer"))); + } + }; Self::hash_(global, algorithm, &input, output) } @@ -1246,13 +1247,14 @@ impl StaticCryptoHasher { // `from_js_no_string_object` never runs user JS, so `output`'s buffer // (captured above) stays valid. - let input = match BlobOrStringOrBuffer::from_js_no_string_object(global, arguments.ptr[0])? { - Some(b) => b, - None => { - return Err(global - .throw_invalid_arguments(format_args!("expected blob, string or buffer"))); - } - }; + let input = + match BlobOrStringOrBuffer::from_js_no_string_object(global, arguments.ptr[0])? { + Some(b) => b, + None => { + return Err(global + .throw_invalid_arguments(format_args!("expected blob, string or buffer"))); + } + }; Self::hash_(global, &input, output) }