diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 4fe4f004b3..2db15ca4e8 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -22,10 +22,10 @@ use { entrypoint::{BPF_ALIGN_OF_U128, MAX_PERMITTED_DATA_INCREASE, SUCCESS}, feature_set::{ add_get_processed_sibling_instruction_syscall, blake3_syscall_enabled, - disable_fees_sysvar, do_support_realloc, libsecp256k1_0_5_upgrade_enabled, - prevent_calling_precompiles_as_programs, return_data_syscall_enabled, - secp256k1_recover_syscall_enabled, sol_log_data_syscall_enabled, - update_syscall_base_costs, + disable_fees_sysvar, do_support_realloc, fixed_memcpy_nonoverlapping_check, + libsecp256k1_0_5_upgrade_enabled, prevent_calling_precompiles_as_programs, + return_data_syscall_enabled, secp256k1_recover_syscall_enabled, + sol_log_data_syscall_enabled, update_syscall_base_costs, }, hash::{Hasher, HASH_BYTES}, instruction::{ @@ -37,6 +37,7 @@ use { native_loader, precompiles::is_precompile, program::MAX_RETURN_DATA, + program_stubs::is_nonoverlapping, pubkey::{Pubkey, PubkeyError, MAX_SEEDS, MAX_SEED_LEN}, secp256k1_recover::{ Secp256k1RecoverError, SECP256K1_PUBLIC_KEY_LENGTH, SECP256K1_SIGNATURE_LENGTH, @@ -1368,7 +1369,9 @@ impl<'a, 'b> SyscallObject for SyscallKeccak256<'a, 'b> { } } -fn check_overlapping(src_addr: u64, dst_addr: u64, n: u64) -> bool { +/// This function is incorrect due to arithmetic overflow and only exists for +/// backwards compatibility. Instead use program_stubs::is_nonoverlapping. +fn check_overlapping_do_not_use(src_addr: u64, dst_addr: u64, n: u64) -> bool { (src_addr <= dst_addr && src_addr + n > dst_addr) || (dst_addr <= src_addr && dst_addr + n > src_addr) } @@ -1406,9 +1409,27 @@ impl<'a, 'b> SyscallObject for SyscallMemcpy<'a, 'b> { memory_mapping: &MemoryMapping, result: &mut Result>, ) { - if check_overlapping(src_addr, dst_addr, n) { - *result = Err(SyscallError::CopyOverlapping.into()); - return; + let invoke_context = question_mark!( + self.invoke_context + .try_borrow() + .map_err(|_| SyscallError::InvokeContextBorrowFailed), + result + ); + let use_fixed_nonoverlapping_check = invoke_context + .feature_set + .is_active(&fixed_memcpy_nonoverlapping_check::id()); + + #[allow(clippy::collapsible_else_if)] + if use_fixed_nonoverlapping_check { + if !is_nonoverlapping(src_addr, dst_addr, n) { + *result = Err(SyscallError::CopyOverlapping.into()); + return; + } + } else { + if check_overlapping_do_not_use(src_addr, dst_addr, n) { + *result = Err(SyscallError::CopyOverlapping.into()); + return; + } } let invoke_context = question_mark!( @@ -4028,13 +4049,13 @@ mod tests { #[test] fn test_overlapping() { - assert!(!check_overlapping(10, 7, 3)); - assert!(check_overlapping(10, 8, 3)); - assert!(check_overlapping(10, 9, 3)); - assert!(check_overlapping(10, 10, 3)); - assert!(check_overlapping(10, 11, 3)); - assert!(check_overlapping(10, 12, 3)); - assert!(!check_overlapping(10, 13, 3)); + assert!(!check_overlapping_do_not_use(10, 7, 3)); + assert!(check_overlapping_do_not_use(10, 8, 3)); + assert!(check_overlapping_do_not_use(10, 9, 3)); + assert!(check_overlapping_do_not_use(10, 10, 3)); + assert!(check_overlapping_do_not_use(10, 11, 3)); + assert!(check_overlapping_do_not_use(10, 12, 3)); + assert!(!check_overlapping_do_not_use(10, 13, 3)); } fn call_program_address_common( diff --git a/sdk/program/src/program_stubs.rs b/sdk/program/src/program_stubs.rs index d55545198a..ca31ba04b1 100644 --- a/sdk/program/src/program_stubs.rs +++ b/sdk/program/src/program_stubs.rs @@ -54,7 +54,7 @@ pub trait SyscallStubs: Sync + Send { unsafe fn sol_memcpy(&self, dst: *mut u8, src: *const u8, n: usize) { // cannot be overlapping assert!( - !(dst as usize + n > src as usize && src as usize > dst as usize), + is_nonoverlapping(src as usize, dst as usize, n), "memcpy does not support overlapping regions" ); std::ptr::copy_nonoverlapping(src, dst, n as usize); @@ -193,3 +193,35 @@ pub(crate) fn sol_get_processed_sibling_instruction(index: usize) -> Option u64 { SYSCALL_STUBS.read().unwrap().sol_get_stack_height() } + +/// Check that two regions do not overlap. +/// +/// Adapted from libcore, hidden to share with bpf_loader without being part of +/// the API surface. +#[doc(hidden)] +pub fn is_nonoverlapping(src: N, dst: N, count: N) -> bool +where + N: Ord + std::ops::Sub, + ::Output: Ord, +{ + let diff = if src > dst { src - dst } else { dst - src }; + // If the absolute distance between the ptrs is at least as big as the size of the buffer, + // they do not overlap. + diff >= count +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_is_nonoverlapping() { + assert!(is_nonoverlapping(10, 7, 3)); + assert!(!is_nonoverlapping(10, 8, 3)); + assert!(!is_nonoverlapping(10, 9, 3)); + assert!(!is_nonoverlapping(10, 10, 3)); + assert!(!is_nonoverlapping(10, 11, 3)); + assert!(!is_nonoverlapping(10, 12, 3)); + assert!(is_nonoverlapping(10, 13, 3)); + } +} diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 5c1c791ae7..e62b1f3499 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -303,6 +303,10 @@ pub mod add_get_processed_sibling_instruction_syscall { solana_sdk::declare_id!("CFK1hRCNy8JJuAAY8Pb2GjLFNdCThS2qwZNe3izzBMgn"); } +pub mod fixed_memcpy_nonoverlapping_check { + solana_sdk::declare_id!("36PRUK2Dz6HWYdG9SpjeAsF5F3KxnFCakA2BZMbtMhSb"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -373,6 +377,7 @@ lazy_static! { (disable_bpf_deprecated_load_instructions::id(), "Disable ldabs* and ldind* BPF instructions"), (disable_bpf_unresolved_symbols_at_runtime::id(), "Disable reporting of unresolved BPF symbols at runtime"), (add_get_processed_sibling_instruction_syscall::id(), "add add_get_processed_sibling_instruction_syscall"), + (fixed_memcpy_nonoverlapping_check::id(), "use correct check for nonoverlapping regions in memcpy syscall"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()