diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index e3fbc234c67c..fb3d2ea4753d 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -230,16 +230,20 @@ 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 { - 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)? }; + // `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) } } @@ -1517,16 +1521,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..fd3ac27d6244 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 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,10 +275,22 @@ impl CryptoHasher { .throw_invalid_arguments(format_args!("expected string or buffer"))); } } - }, - None => None, + } + } else { + None }; + // `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) } @@ -1223,37 +1218,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 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() { @@ -1263,10 +1240,22 @@ impl StaticCryptoHasher { .throw_invalid_arguments(format_args!("expected string or buffer"))); } } - }, - None => None, + } + } else { + None }; + // `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 9409a40ba3e0..90bfa106196e 100644 --- a/src/runtime/crypto/PasswordObject.rs +++ b/src/runtime/crypto/PasswordObject.rs @@ -929,19 +929,25 @@ pub(crate) fn js_password_object_verify_sync( }; } - let Some(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", )); }; diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index 475ef78c71b9..9f9bd446eb65 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -101,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 @@ -141,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( @@ -151,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( 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..4eaa71eb634c 100644 --- a/test/js/bun/util/bun-cryptohasher.test.ts +++ b/test/js/bun/util/bun-cryptohasher.test.ts @@ -1,6 +1,59 @@ import { describe, expect, test } from "bun:test"; import { withoutAggressiveGC } from "harness"; +describe("input buffer detached by output argument's toString", () => { + // 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); + 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 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() { + out.buffer.transfer(0); + return "A"; + }, + }); + expect(() => Bun.CryptoHasher.hash("sha256", evilInput as unknown as string, out)).toThrow( + /expected blob, string or buffer/, + ); + expect(out.byteLength).toBe(32); + }); +}); + 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..aa307b4d3434 100644 --- a/test/js/bun/util/index-of-line.test.ts +++ b/test/js/bun/util/index-of-line.test.ts @@ -85,6 +85,25 @@ 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", () => { + // 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); + 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/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 769a98399786..18fd12731ccc 100644 --- a/test/js/bun/util/randomUUIDv5.test.ts +++ b/test/js/bun/util/randomUUIDv5.test.ts @@ -367,6 +367,24 @@ describe("randomUUIDv5", () => { expect(result).toEqual(uuid.v5("test", uuid.v5.DNS)); }); + test("name buffer detached by namespace argument's toString", () => { + // 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); + 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++) {