Feature - Check syscall outputs do not overlap (#28599)

* Extends is_nonoverlapping() to be able to deal with two different lengths.

* Uses is_nonoverlapping() for syscall output parameters.

* Feature gates the new throws of SyscallError::CopyOverlapping.

* Adds tests which trigger SyscallError::CopyOverlapping.
This commit is contained in:
Alexander Meißner 2022-10-27 19:11:18 +02:00 committed by GitHub
parent 7b70aef33c
commit a43098a428
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 221 additions and 32 deletions

View File

@ -26,7 +26,7 @@ declare_syscall!(
.feature_set
.is_active(&check_physical_overlapping::id());
if !is_nonoverlapping(src_addr, dst_addr, n) {
if !is_nonoverlapping(src_addr, n, dst_addr, n) {
return Err(SyscallError::CopyOverlapping.into());
}
@ -47,7 +47,7 @@ declare_syscall!(
)?
.as_ptr();
if do_check_physical_overlapping
&& !is_nonoverlapping(src_ptr as usize, dst_ptr as usize, n as usize)
&& !is_nonoverlapping(src_ptr as usize, n as usize, dst_ptr as usize, n as usize)
{
unsafe {
std::ptr::copy(src_ptr, dst_ptr, n as usize);

View File

@ -30,7 +30,8 @@ use {
entrypoint::{BPF_ALIGN_OF_U128, MAX_PERMITTED_DATA_INCREASE, SUCCESS},
feature_set::FeatureSet,
feature_set::{
self, blake3_syscall_enabled, check_physical_overlapping, curve25519_syscall_enabled,
self, blake3_syscall_enabled, check_physical_overlapping,
check_syscall_outputs_do_not_overlap, curve25519_syscall_enabled,
disable_cpi_setting_executable_and_rent_epoch, disable_fees_sysvar,
enable_early_verification_of_account_modifications, libsecp256k1_0_5_upgrade_enabled,
limit_secp256k1_recovery_id, stop_sibling_instruction_search_at_parent,
@ -649,10 +650,21 @@ declare_syscall!(
let address = translate_slice_mut::<u8>(
memory_mapping,
address_addr,
32,
std::mem::size_of::<Pubkey>() as u64,
invoke_context.get_check_aligned(),
invoke_context.get_check_size(),
)?;
if !is_nonoverlapping(
bump_seed_ref as *const _ as usize,
std::mem::size_of_val(bump_seed_ref),
address.as_ptr() as usize,
std::mem::size_of::<Pubkey>(),
) && invoke_context
.feature_set
.is_active(&check_syscall_outputs_do_not_overlap::id())
{
return Err(SyscallError::CopyOverlapping.into());
}
*bump_seed_ref = bump_seed[0];
address.copy_from_slice(new_address.as_ref());
return Ok(0);
@ -1432,6 +1444,18 @@ declare_syscall!(
invoke_context.get_check_aligned(),
)?;
if !is_nonoverlapping(
to_slice.as_ptr() as usize,
length as usize,
program_id_result as *const _ as usize,
std::mem::size_of::<Pubkey>(),
) && invoke_context
.feature_set
.is_active(&check_syscall_outputs_do_not_overlap::id())
{
return Err(SyscallError::CopyOverlapping.into());
}
*program_id_result = *program_id;
}
@ -1490,17 +1514,14 @@ declare_syscall!(
}
if let Some(instruction_context) = found_instruction_context {
let ProcessedSiblingInstruction {
data_len,
accounts_len,
} = translate_type_mut::<ProcessedSiblingInstruction>(
let result_header = translate_type_mut::<ProcessedSiblingInstruction>(
memory_mapping,
meta_addr,
invoke_context.get_check_aligned(),
)?;
if *data_len == (instruction_context.get_instruction_data().len() as u64)
&& *accounts_len
if result_header.data_len == (instruction_context.get_instruction_data().len() as u64)
&& result_header.accounts_len
== (instruction_context.get_number_of_instruction_accounts() as u64)
{
let program_id = translate_type_mut::<Pubkey>(
@ -1511,18 +1532,58 @@ declare_syscall!(
let data = translate_slice_mut::<u8>(
memory_mapping,
data_addr,
*data_len as u64,
result_header.data_len as u64,
invoke_context.get_check_aligned(),
invoke_context.get_check_size(),
)?;
let accounts = translate_slice_mut::<AccountMeta>(
memory_mapping,
accounts_addr,
*accounts_len as u64,
result_header.accounts_len as u64,
invoke_context.get_check_aligned(),
invoke_context.get_check_size(),
)?;
if (!is_nonoverlapping(
result_header as *const _ as usize,
std::mem::size_of::<ProcessedSiblingInstruction>(),
program_id as *const _ as usize,
std::mem::size_of::<Pubkey>(),
) || !is_nonoverlapping(
result_header as *const _ as usize,
std::mem::size_of::<ProcessedSiblingInstruction>(),
accounts.as_ptr() as usize,
std::mem::size_of::<AccountMeta>()
.saturating_mul(result_header.accounts_len as usize),
) || !is_nonoverlapping(
result_header as *const _ as usize,
std::mem::size_of::<ProcessedSiblingInstruction>(),
data.as_ptr() as usize,
result_header.data_len as usize,
) || !is_nonoverlapping(
program_id as *const _ as usize,
std::mem::size_of::<Pubkey>(),
data.as_ptr() as usize,
result_header.data_len as usize,
) || !is_nonoverlapping(
program_id as *const _ as usize,
std::mem::size_of::<Pubkey>(),
accounts.as_ptr() as usize,
std::mem::size_of::<AccountMeta>()
.saturating_mul(result_header.accounts_len as usize),
) || !is_nonoverlapping(
data.as_ptr() as usize,
result_header.data_len as usize,
accounts.as_ptr() as usize,
std::mem::size_of::<AccountMeta>()
.saturating_mul(result_header.accounts_len as usize),
)) && invoke_context
.feature_set
.is_active(&check_syscall_outputs_do_not_overlap::id())
{
return Err(SyscallError::CopyOverlapping.into());
}
*program_id = *instruction_context
.get_last_program_key(invoke_context.transaction_context)
.map_err(SyscallError::InstructionError)?;
@ -1548,8 +1609,9 @@ declare_syscall!(
.map_err(SyscallError::InstructionError)?;
accounts.clone_from_slice(account_metas.as_slice());
}
*data_len = instruction_context.get_instruction_data().len() as u64;
*accounts_len = instruction_context.get_number_of_instruction_accounts() as u64;
result_header.data_len = instruction_context.get_instruction_data().len() as u64;
result_header.accounts_len =
instruction_context.get_number_of_instruction_accounts() as u64;
return Ok(true as u64);
}
Ok(false as u64)
@ -1603,7 +1665,7 @@ declare_syscall!(
let instruction_context = transaction_context
.get_current_instruction_context()
.map_err(SyscallError::InstructionError)?;
let updates = translate_slice_mut::<AccountPropertyUpdate>(
let updates = translate_slice::<AccountPropertyUpdate>(
memory_mapping,
updates_addr,
updates_count,
@ -1621,7 +1683,7 @@ declare_syscall!(
unsafe { std::mem::transmute::<_, TransactionContextAttribute>(update.attribute) };
match attribute {
TransactionContextAttribute::TransactionAccountOwner => {
let owner_pubkey = translate_type_mut::<Pubkey>(
let owner_pubkey = translate_type::<Pubkey>(
memory_mapping,
update.value,
invoke_context.get_check_aligned(),
@ -3620,6 +3682,7 @@ mod tests {
invoke_context: &'a mut InvokeContext<'b>,
seeds: &[&[u8]],
program_id: &Pubkey,
overlap_outputs: bool,
syscall: SyscallFunction<&'a mut InvokeContext<'b>>,
) -> Result<(Pubkey, u8), EbpfError> {
const SEEDS_VA: u64 = 0x100000000;
@ -3687,7 +3750,11 @@ mod tests {
seeds.len() as u64,
PROGRAM_ID_VA,
ADDRESS_VA,
BUMP_SEED_VA,
if overlap_outputs {
ADDRESS_VA
} else {
BUMP_SEED_VA
},
&mut memory_mapping,
&mut result,
);
@ -3703,6 +3770,7 @@ mod tests {
invoke_context,
seeds,
address,
false,
SyscallCreateProgramAddress::call,
)?;
Ok(address)
@ -3717,10 +3785,85 @@ mod tests {
invoke_context,
seeds,
address,
false,
SyscallTryFindProgramAddress::call,
)
}
#[test]
fn test_set_and_get_return_data() {
const SRC_VA: u64 = 0x100000000;
const DST_VA: u64 = 0x200000000;
const PROGRAM_ID_VA: u64 = 0x300000000;
let data = vec![42; 24];
let mut data_buffer = vec![0; 16];
let mut id_buffer = vec![0; 32];
let config = Config::default();
let mut memory_mapping = MemoryMapping::new(
vec![
MemoryRegion::new_readonly(&data, SRC_VA),
MemoryRegion::new_writable(&mut data_buffer, DST_VA),
MemoryRegion::new_writable(&mut id_buffer, PROGRAM_ID_VA),
],
&config,
)
.unwrap();
prepare_mockup!(
invoke_context,
transaction_context,
program_id,
bpf_loader::id(),
);
let mut result = ProgramResult::Ok(0);
SyscallSetReturnData::call(
&mut invoke_context,
SRC_VA,
data.len() as u64,
0,
0,
0,
&mut memory_mapping,
&mut result,
);
assert_eq!(result.unwrap(), 0);
let mut result = ProgramResult::Ok(0);
SyscallGetReturnData::call(
&mut invoke_context,
DST_VA,
data_buffer.len() as u64,
PROGRAM_ID_VA,
0,
0,
&mut memory_mapping,
&mut result,
);
assert_eq!(result.unwrap() as usize, data.len());
assert_eq!(data.get(0..data_buffer.len()).unwrap(), data_buffer);
assert_eq!(id_buffer, program_id.to_bytes());
let mut result = ProgramResult::Ok(0);
SyscallGetReturnData::call(
&mut invoke_context,
PROGRAM_ID_VA,
data_buffer.len() as u64,
PROGRAM_ID_VA,
0,
0,
&mut memory_mapping,
&mut result,
);
assert!(matches!(
result,
ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::<BpfError>().unwrap() == &BpfError::SyscallError(
SyscallError::CopyOverlapping
),
));
}
#[test]
fn test_syscall_sol_get_processed_sibling_instruction() {
let transaction_accounts = (0..9)
@ -3859,6 +4002,28 @@ mod tests {
&mut result,
);
assert_eq!(result.unwrap(), 0);
invoke_context
.get_compute_meter()
.borrow_mut()
.mock_set_remaining(syscall_base_cost);
let mut result = ProgramResult::Ok(0);
SyscallGetProcessedSiblingInstruction::call(
&mut invoke_context,
0,
VM_BASE_ADDRESS.saturating_add(META_OFFSET as u64),
VM_BASE_ADDRESS.saturating_add(META_OFFSET as u64),
VM_BASE_ADDRESS.saturating_add(META_OFFSET as u64),
VM_BASE_ADDRESS.saturating_add(META_OFFSET as u64),
&mut memory_mapping,
&mut result,
);
assert!(matches!(
result,
ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::<BpfError>().unwrap() == &BpfError::SyscallError(
SyscallError::CopyOverlapping
),
));
}
#[test]
@ -4219,6 +4384,19 @@ mod tests {
SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded)
),
));
assert!(matches!(
call_program_address_common(
&mut invoke_context,
seeds,
&address,
true,
SyscallTryFindProgramAddress::call,
),
Err(EbpfError::UserError(error)) if error.downcast_ref::<BpfError>().unwrap() == &BpfError::SyscallError(
SyscallError::CopyOverlapping
),
));
}
#[test]

View File

@ -57,7 +57,7 @@ pub trait SyscallStubs: Sync + Send {
unsafe fn sol_memcpy(&self, dst: *mut u8, src: *const u8, n: usize) {
// cannot be overlapping
assert!(
is_nonoverlapping(src as usize, dst as usize, n),
is_nonoverlapping(src as usize, n, dst as usize, n),
"memcpy does not support overlapping regions"
);
std::ptr::copy_nonoverlapping(src, dst, n as usize);
@ -207,18 +207,20 @@ pub(crate) fn sol_set_account_properties(updates: &[AccountPropertyUpdate]) {
/// Check that two regions do not overlap.
///
/// Adapted from libcore, hidden to share with bpf_loader without being part of
/// the API surface.
/// Hidden to share with bpf_loader without being part of the API surface.
#[doc(hidden)]
pub fn is_nonoverlapping<N>(src: N, dst: N, count: N) -> bool
pub fn is_nonoverlapping<N>(src: N, src_len: N, dst: N, dst_len: N) -> bool
where
N: Ord + std::ops::Sub<Output = N>,
<N as 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,
// If the absolute distance between the ptrs is at least as big as the size of the other,
// they do not overlap.
diff >= count
if src > dst {
src - dst >= dst_len
} else {
dst - src >= src_len
}
}
#[cfg(test)]
@ -227,12 +229,16 @@ mod tests {
#[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));
for dst in 0..8 {
assert!(is_nonoverlapping(10, 3, dst, 3));
}
for dst in 8..13 {
assert!(!is_nonoverlapping(10, 3, dst, 3));
}
for dst in 13..20 {
assert!(is_nonoverlapping(10, 3, dst, 3));
}
assert!(is_nonoverlapping::<u8>(255, 3, 254, 1));
assert!(!is_nonoverlapping::<u8>(255, 2, 254, 3));
}
}

View File

@ -530,6 +530,10 @@ pub mod limit_max_instruction_trace_length {
solana_sdk::declare_id!("GQALDaC48fEhZGWRj9iL5Q889emJKcj3aCvHF7VCbbF4");
}
pub mod check_syscall_outputs_do_not_overlap {
solana_sdk::declare_id!("3uRVPBpyEJRo1emLCrq38eLRFGcu6uKSpUXqGvU8T7SZ");
}
lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -656,7 +660,8 @@ lazy_static! {
(epoch_accounts_hash::id(), "enable epoch accounts hash calculation #27539"),
(remove_deprecated_request_unit_ix::id(), "remove support for RequestUnitsDeprecated instruction #27500"),
(increase_tx_account_lock_limit::id(), "increase tx account lock limit to 128 #27241"),
(limit_max_instruction_trace_length::id(), "limit max instruction trace length"),
(limit_max_instruction_trace_length::id(), "limit max instruction trace length #27939"),
(check_syscall_outputs_do_not_overlap::id(), "check syscall outputs do_not overlap #28600"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()