diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index 01a8e12a08a..5f15fa403a7 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -1,4 +1,4 @@ -use core::ffi::{c_uint, c_void}; +use core::ffi::c_void; use core::ptr; use crate as jsc; @@ -155,9 +155,10 @@ impl ArrayBuffer { self.value.unpin_array_buffer(); } - // require('buffer').kMaxLength. + // The largest byte length JavaScriptCore backs an ArrayBuffer with + // (`MAX_ARRAY_BUFFER_SIZE`), exposed to JS as `require('buffer').kMaxLength`. // keep in sync with Bun::Buffer::kMaxLength - pub const MAX_SIZE: c_uint = c_uint::MAX; + pub const MAX_SIZE: usize = 1 << 32; // 4 MB or so is pretty good for mmap() const MMAP_THRESHOLD: usize = 1024 * 1024 * 4; @@ -424,8 +425,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() @@ -446,8 +447,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() diff --git a/src/runtime/ffi/FFIObject.rs b/src/runtime/ffi/FFIObject.rs index 01d678418ce..f734881aa8f 100644 --- a/src/runtime/ffi/FFIObject.rs +++ b/src/runtime/ffi/FFIObject.rs @@ -80,14 +80,18 @@ pub(crate) fn new_cstring( byte_offset: Option, length_value: Option, ) -> JsResult { - match get_ptr_slice(global_this, value, byte_offset, length_value) { - ValueOrError::Err(err) => Ok(err), - ValueOrError::Slice(ptr, len) => { - // SAFETY: ptr/len point to FFI-owned memory whose lifetime the caller guarantees. - let bytes = unsafe { core::slice::from_raw_parts(ptr, len) }; - jsc::bun_string_jsc::create_utf8_for_js(global_this, bytes) - } - } + // A C string's JS representation is capped by `WTF::String::MaxLength` in code + // units, which a UTF-8 byte count does not map onto; only the address is bounded. + let (ptr, len) = get_ptr_slice( + global_this, + value, + byte_offset, + length_value, + MAX_ADDRESSABLE_MEMORY, + )?; + // SAFETY: ptr/len point to FFI-owned memory whose lifetime the caller guarantees. + let bytes = unsafe { core::slice::from_raw_parts(ptr, len) }; + jsc::bun_string_jsc::create_utf8_for_js(global_this, bytes) } // DOMJIT fast-path descriptor + slow-path host fn, represented here as a const @@ -484,35 +488,30 @@ fn ptr_(global_this: &JSGlobalObject, value: JSValue, byte_offset: Option, byte_length: Option, -) -> ValueOrError { + max_byte_length: usize, +) -> JsResult<(*mut u8, usize)> { if !value.is_number() || value.as_number() < 0.0 || value.as_number() > usize::MAX as f64 { - return ValueOrError::Err( - global_this.to_invalid_arguments(format_args!("ptr must be a number.")), - ); + return Err(global_this.throw_invalid_arguments(format_args!("ptr must be a number."))); } let num = value.as_ptr_address(); if num == 0 { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( + return Err(global_this.throw_invalid_arguments(format_args!( "ptr cannot be zero, that would segfault Bun :(" ))); } @@ -529,69 +528,81 @@ fn get_ptr_slice( } if addr == 0 { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( + return Err(global_this.throw_invalid_arguments(format_args!( "ptr cannot be zero, that would segfault Bun :(" ))); } if !byte_off.as_number().is_finite() { - return ValueOrError::Err( - global_this.to_invalid_arguments(format_args!("ptr must be a finite number.")), - ); + return Err(global_this + .throw_invalid_arguments(format_args!("ptr must be a finite number."))); } - } else if !byte_off.is_empty_or_undefined_or_null() { - // do nothing + } else if byte_off.is_empty_or_undefined_or_null() { + // an omitted byteOffset leaves `addr` alone } else { - return ValueOrError::Err( - global_this.to_invalid_arguments(format_args!("Expected number for byteOffset")), + return Err( + global_this.throw_invalid_arguments(format_args!("Expected number for byteOffset")) ); } } if addr == 0xDEADBEEF || addr == 0xaaaaaaaa || addr == 0xAAAAAAAA { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( + return Err(global_this.throw_invalid_arguments(format_args!( "ptr to invalid memory, that would segfault Bun :(" ))); } - if let Some(value_length) = byte_length { - if !value_length.is_empty_or_undefined_or_null() { + let explicit_length = byte_length.filter(|len| !len.is_empty_or_undefined_or_null()); + let length_i = match explicit_length { + Some(value_length) => { if !value_length.is_number() { - return ValueOrError::Err( - global_this.to_invalid_arguments(format_args!("length must be a number.")), + return Err( + global_this.throw_invalid_arguments(format_args!("length must be a number.")) ); } if value_length.as_number() == 0.0 { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( + return Err(global_this.throw_invalid_arguments(format_args!( "length must be > 0. This usually means a bug in your code." ))); } let length_i = value_length.to_int64(); if length_i < 0 { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( + return Err(global_this.throw_invalid_arguments(format_args!( "length must be > 0. This usually means a bug in your code." ))); } - if length_i > i64::try_from(MAX_ADDRESSABLE_MEMORY).expect("int cast") { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( - "length exceeds max addressable memory. This usually means a bug in your code." - ))); - } - - let length = usize::try_from(length_i).expect("int cast"); - return ValueOrError::Slice(addr as *mut u8, length); + length_i + } + // Scan for the NUL terminator. The scanned length is bounded too: a C + // string longer than the result object can represent aborts inside JSC. + None => { + // SAFETY: caller asserts `addr` points at a NUL-terminated C string. + let len = unsafe { bun_core::ffi::cstr(addr as *const core::ffi::c_char) } + .to_bytes() + .len(); + i64::try_from(len).expect("int cast") } + }; + + let max = i64::try_from(max_byte_length).expect("int cast"); + if length_i > max { + return Err(global_this.throw_range_error( + length_i, + jsc::RangeErrorOptions { + field_name: b"byteLength", + max, + ..Default::default() + }, + )); } - // Scan for the NUL terminator. - // SAFETY: caller asserts `addr` points at a NUL-terminated C string. - let len = unsafe { bun_core::ffi::cstr(addr as *const core::ffi::c_char) } - .to_bytes() - .len(); - ValueOrError::Slice(addr as *mut u8, len) + Ok(( + addr as *mut u8, + usize::try_from(length_i).expect("int cast"), + )) } fn get_cptr(value: JSValue) -> Option { @@ -619,56 +630,59 @@ pub(crate) fn to_array_buffer( finalization_ctx_or_ptr: Option, finalization_callback: Option, ) -> JsResult { - match get_ptr_slice(global_this, value, byte_offset, value_length) { - ValueOrError::Err(erro) => Ok(erro), - ValueOrError::Slice(ptr, len) => { - let mut callback: jsc::JSTypedArrayBytesDeallocator = None; - let mut ctx: Option<*mut c_void> = None; - if let Some(callback_value) = finalization_callback { - if let Some(callback_ptr) = get_cptr(callback_value) { - // SAFETY: user-supplied raw fn pointer address. - callback = unsafe { deallocator_from_addr(callback_ptr) }; - - if let Some(ctx_value) = finalization_ctx_or_ptr { - if let Some(ctx_ptr) = get_cptr(ctx_value) { - ctx = Some(ctx_ptr as *mut c_void); - } else if !ctx_value.is_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected user data to be a C pointer (number or BigInt)" - ))); - } - } - } else if !callback_value.is_empty_or_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected callback to be a C pointer (number or BigInt)" + let (ptr, len) = get_ptr_slice( + global_this, + value, + byte_offset, + value_length, + ArrayBuffer::MAX_SIZE, + )?; + + let mut callback: jsc::JSTypedArrayBytesDeallocator = None; + let mut ctx: Option<*mut c_void> = None; + if let Some(callback_value) = finalization_callback { + if let Some(callback_ptr) = get_cptr(callback_value) { + // SAFETY: user-supplied raw fn pointer address. + callback = unsafe { deallocator_from_addr(callback_ptr) }; + + if let Some(ctx_value) = finalization_ctx_or_ptr { + if let Some(ctx_ptr) = get_cptr(ctx_value) { + ctx = Some(ctx_ptr as *mut c_void); + } else if !ctx_value.is_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected user data to be a C pointer (number or BigInt)" ))); } - } else if let Some(callback_value) = finalization_ctx_or_ptr { - if let Some(callback_ptr) = get_cptr(callback_value) { - // SAFETY: user-supplied raw fn pointer address. - callback = unsafe { deallocator_from_addr(callback_ptr) }; - } else if !callback_value.is_empty_or_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected callback to be a C pointer (number or BigInt)" - ))); - } - } - - // SAFETY: ptr/len came from get_ptr_slice; FFI-owned memory. The - // `bun:ffi` user asserts the pointer stays valid for the object's - // lifetime and that their finalization callback/ctx pair, if - // provided, is sound to invoke once at GC — `toArrayBuffer(ptr, - // ...)` is an inherently trusting FFI API. - unsafe { - let slice = core::slice::from_raw_parts_mut(ptr, len); - ArrayBuffer::from_bytes(slice, jsc::JSType::ArrayBuffer).to_js_with_context( - global_this, - ctx.unwrap_or(core::ptr::null_mut()), - callback, - ) } + } else if !callback_value.is_empty_or_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected callback to be a C pointer (number or BigInt)" + ))); + } + } else if let Some(callback_value) = finalization_ctx_or_ptr { + if let Some(callback_ptr) = get_cptr(callback_value) { + // SAFETY: user-supplied raw fn pointer address. + callback = unsafe { deallocator_from_addr(callback_ptr) }; + } else if !callback_value.is_empty_or_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected callback to be a C pointer (number or BigInt)" + ))); } } + + // SAFETY: ptr/len came from get_ptr_slice; FFI-owned memory. The + // `bun:ffi` user asserts the pointer stays valid for the object's + // lifetime and that their finalization callback/ctx pair, if + // provided, is sound to invoke once at GC — `toArrayBuffer(ptr, + // ...)` is an inherently trusting FFI API. + unsafe { + let slice = core::slice::from_raw_parts_mut(ptr, len); + ArrayBuffer::from_bytes(slice, jsc::JSType::ArrayBuffer).to_js_with_context( + global_this, + ctx.unwrap_or(core::ptr::null_mut()), + callback, + ) + } } pub(crate) fn to_buffer( @@ -679,57 +693,60 @@ pub(crate) fn to_buffer( finalization_ctx_or_ptr: Option, finalization_callback: Option, ) -> JsResult { - match get_ptr_slice(global_this, value, byte_offset, value_length) { - ValueOrError::Err(err) => Ok(err), - ValueOrError::Slice(ptr, len) => { - let mut callback: jsc::JSTypedArrayBytesDeallocator = None; - let mut ctx: Option<*mut c_void> = None; - if let Some(callback_value) = finalization_callback { - if let Some(callback_ptr) = get_cptr(callback_value) { - // SAFETY: user-supplied raw fn pointer address. - callback = unsafe { deallocator_from_addr(callback_ptr) }; - - if let Some(ctx_value) = finalization_ctx_or_ptr { - if let Some(ctx_ptr) = get_cptr(ctx_value) { - ctx = Some(ctx_ptr as *mut c_void); - } else if !ctx_value.is_empty_or_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected user data to be a C pointer (number or BigInt)" - ))); - } - } - } else if !callback_value.is_empty_or_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected callback to be a C pointer (number or BigInt)" - ))); - } - } else if let Some(callback_value) = finalization_ctx_or_ptr { - if let Some(callback_ptr) = get_cptr(callback_value) { - // SAFETY: user-supplied raw fn pointer address. - callback = unsafe { deallocator_from_addr(callback_ptr) }; - } else if !callback_value.is_empty_or_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected callback to be a C pointer (number or BigInt)" + let (ptr, len) = get_ptr_slice( + global_this, + value, + byte_offset, + value_length, + ArrayBuffer::MAX_SIZE, + )?; + + let mut callback: jsc::JSTypedArrayBytesDeallocator = None; + let mut ctx: Option<*mut c_void> = None; + if let Some(callback_value) = finalization_callback { + if let Some(callback_ptr) = get_cptr(callback_value) { + // SAFETY: user-supplied raw fn pointer address. + callback = unsafe { deallocator_from_addr(callback_ptr) }; + + if let Some(ctx_value) = finalization_ctx_or_ptr { + if let Some(ctx_ptr) = get_cptr(ctx_value) { + ctx = Some(ctx_ptr as *mut c_void); + } else if !ctx_value.is_empty_or_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected user data to be a C pointer (number or BigInt)" ))); } } - - // SAFETY: ptr/len came from get_ptr_slice; FFI-owned memory. - let slice = unsafe { core::slice::from_raw_parts_mut(ptr, len) }; - if callback.is_some() || ctx.is_some() { - return Ok(create_buffer_with_ctx( - global_this, - slice, - ctx.unwrap_or(core::ptr::null_mut()), - callback, - )); - } - - // `JSValue::create_buffer` installs `MarkedArrayBuffer_deallocator` so - // the slice is `mi_free`d on GC (including the free-foreign-memory footgun). - Ok(JSValue::create_buffer(global_this, slice)) + } else if !callback_value.is_empty_or_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected callback to be a C pointer (number or BigInt)" + ))); } + } else if let Some(callback_value) = finalization_ctx_or_ptr { + if let Some(callback_ptr) = get_cptr(callback_value) { + // SAFETY: user-supplied raw fn pointer address. + callback = unsafe { deallocator_from_addr(callback_ptr) }; + } else if !callback_value.is_empty_or_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected callback to be a C pointer (number or BigInt)" + ))); + } + } + + // SAFETY: ptr/len came from get_ptr_slice; FFI-owned memory. + let slice = unsafe { core::slice::from_raw_parts_mut(ptr, len) }; + if callback.is_some() || ctx.is_some() { + return Ok(create_buffer_with_ctx( + global_this, + slice, + ctx.unwrap_or(core::ptr::null_mut()), + callback, + )); } + + // `JSValue::create_buffer` installs `MarkedArrayBuffer_deallocator` so + // the slice is `mi_free`d on GC (including the free-foreign-memory footgun). + Ok(JSValue::create_buffer(global_this, slice)) } pub(crate) fn getter(global_object: &JSGlobalObject, _: &JSObject) -> JSValue { diff --git a/test/js/bun/ffi/ffi.test.js b/test/js/bun/ffi/ffi.test.js index 2546e1d1fcf..6f82fcceeda 100644 --- a/test/js/bun/ffi/ffi.test.js +++ b/test/js/bun/ffi/ffi.test.js @@ -707,6 +707,105 @@ it.skipIf(!isWindows || isFFIUnavailable)("dlopen accepts non-ASCII library path }); }); +// `expect(fn).toThrow()` is satisfied by a *returned* Error, so spell the +// difference out: these used to hand the caller an Error object as the result. +it("toArrayBuffer and toBuffer throw argument errors instead of returning them", () => { + const address = ptr(new Uint8Array(8)); + const outcome = view => { + try { + const result = view(); + return `returned ${result instanceof Error ? result.constructor.name : typeof result}`; + } catch (error) { + return `threw ${error.constructor.name}: ${error.message}`; + } + }; + + expect([ + outcome(() => toArrayBuffer(0)), + outcome(() => toBuffer(0)), + outcome(() => toArrayBuffer(address, 0, 0)), + outcome(() => toBuffer(address, 0, -1)), + outcome(() => toArrayBuffer(address, 0, 8, "not a pointer")), + outcome(() => toBuffer(address, 0, 8, "not a pointer")), + ]).toEqual([ + "threw TypeError: ptr cannot be zero, that would segfault Bun :(", + "threw TypeError: ptr cannot be zero, that would segfault Bun :(", + "threw TypeError: length must be > 0. This usually means a bug in your code.", + "threw TypeError: length must be > 0. This usually means a bug in your code.", + "threw TypeError: Expected callback to be a C pointer (number or BigInt)", + "threw TypeError: Expected callback to be a C pointer (number or BigInt)", + ]); +}); + +// An omitted byteOffset used to be the error case and a non-number the +// silently-ignored one, so CString#arrayBuffer (byteOffset defaults to +// undefined) cached a TypeError as its ArrayBuffer. +it("toArrayBuffer accepts an omitted byteOffset and rejects a non-number one", () => { + const bytes = new Uint8Array(16); + bytes.set(Buffer.from("hi\0")); + const address = ptr(bytes); + + expect(toArrayBuffer(address, undefined, 8).byteLength).toBe(8); + expect(toArrayBuffer(address, null, 8).byteLength).toBe(8); + expect(() => toArrayBuffer(address, "garbage", 8)).toThrow("Expected number for byteOffset"); + expect(() => toArrayBuffer(address, {}, 8)).toThrow("Expected number for byteOffset"); + + const arrayBuffer = new CString(address).arrayBuffer; + expect(arrayBuffer).toBeInstanceOf(ArrayBuffer); + expect(new TextDecoder().decode(arrayBuffer)).toBe("hi"); + + // the view aliases `bytes`, which also keeps it reachable across the decode + new Uint8Array(arrayBuffer)[0] = "H".charCodeAt(0); + expect(bytes[0]).toBe("H".charCodeAt(0)); +}); + +// A byteLength JSC cannot back an ArrayBuffer with used to abort the process: +// toArrayBuffer panicked on an int cast, toBuffer tripped a JSC RELEASE_ASSERT. +it("toArrayBuffer and toBuffer reject a byteLength past the max ArrayBuffer size", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `import { ptr, toArrayBuffer, toBuffer } from "bun:ffi"; + const backing = new Uint8Array(64); + const address = ptr(backing); + for (const [name, view] of [["toArrayBuffer", toArrayBuffer], ["toBuffer", toBuffer]]) { + for (const byteLength of [2 ** 32 + 1, 2 ** 33, Number.MAX_SAFE_INTEGER]) { + try { + view(address, 0, byteLength); + console.log(name, byteLength, "did not throw"); + } catch (e) { + console.log(name, e.constructor.name, e.code, e.message); + } + } + } + // exactly MAX_ARRAY_BUFFER_SIZE is what new ArrayBuffer(2 ** 32) accepts. + // Only toArrayBuffer holds a view here: toBuffer without a finalizer installs + // a deallocator that frees a pointer it does not own, which VM teardown runs. + console.log("at the limit", toArrayBuffer(address, 0, 2 ** 32).byteLength, backing.length);`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + // stderr is drained, not asserted: ASAN and debug builds emit benign warnings. + const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const outOfRange = "is out of range. It must be <= 4294967296. Received"; + expect({ stdout: stdout.split("\n"), exitCode }).toEqual({ + stdout: [ + `toArrayBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 4294967297`, + `toArrayBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 8589934592`, + `toArrayBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 9007199254740991`, + `toBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 4294967297`, + `toBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 8589934592`, + `toBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 9007199254740991`, + "at the limit 4294967296 64", + "", + ], + exitCode: 0, + }); +}); + it('suffix does not start with a "."', () => { expect(suffix).not.toMatch(/^\./); });