From 6679153ca17f8e96f7e3b57bc4c8e565774f421f Mon Sep 17 00:00:00 2001 From: Alessandro Decina Date: Tue, 5 Sep 2023 14:27:26 +0700 Subject: [PATCH] CPI: improve test coverage (#31986) * programs/sbf: add TEST_[FORBID|ALLOW]_WRITE_AFTER_OWNERSHIP_CHANGE* * programs/sbf: add tests for the AccessViolation -> InstructionError mapping * cpi: add more tests * programs/sbf: add tests for immutable AccountInfo pointers * programs/sbf: add tests for verification of SolAccountInfo pointers too * programs/sbf: add tests for ref_to_len_in_vm handling in CPI Add TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE_MOVING_DATA_POINTER and TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE that exercise the new logic. * cpi: tweak tests Remove some copy pasta and rename two tests to better describe what they're doing * cpi: add tests that check that CPI updates all accounts at once * direct mapping: test that writes to executable accounts trigger ExecutableDataModified * programs/sbf: add explicit tests for when an account's data allocation changes --- programs/sbf/Cargo.lock | 1 + programs/sbf/c/src/invoke/invoke.c | 73 ++ .../sbf/rust/deprecated_loader/src/lib.rs | 144 +++- programs/sbf/rust/invoke/Cargo.toml | 1 + programs/sbf/rust/invoke/src/instructions.rs | 18 + programs/sbf/rust/invoke/src/processor.rs | 614 ++++++++++++++++- programs/sbf/rust/invoked/src/instructions.rs | 1 + programs/sbf/rust/invoked/src/processor.rs | 8 + programs/sbf/rust/realloc/src/instructions.rs | 1 + programs/sbf/rust/realloc/src/processor.rs | 8 + programs/sbf/tests/programs.rs | 645 +++++++++++++++++- 11 files changed, 1483 insertions(+), 31 deletions(-) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 22a0e9f5c8..18a669098b 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5755,6 +5755,7 @@ version = "1.17.0" dependencies = [ "solana-program", "solana-sbf-rust-invoked", + "solana-sbf-rust-realloc", ] [[package]] diff --git a/programs/sbf/c/src/invoke/invoke.c b/programs/sbf/c/src/invoke/invoke.c index 4cb038cc7f..1ff4e6b69a 100644 --- a/programs/sbf/c/src/invoke/invoke.c +++ b/programs/sbf/c/src/invoke/invoke.c @@ -31,6 +31,12 @@ static const uint8_t TEST_RETURN_DATA_TOO_LARGE = 18; static const uint8_t TEST_DUPLICATE_PRIVILEGE_ESCALATION_SIGNER = 19; static const uint8_t TEST_DUPLICATE_PRIVILEGE_ESCALATION_WRITABLE = 20; static const uint8_t TEST_MAX_ACCOUNT_INFOS_EXCEEDED = 21; +// TEST_CPI_INVALID_* must match the definitions in +// https://github.com/solana-labs/solana/blob/master/programs/sbf/rust/invoke/src/instructions.rs +static const uint8_t TEST_CPI_INVALID_KEY_POINTER = 34; +static const uint8_t TEST_CPI_INVALID_OWNER_POINTER = 35; +static const uint8_t TEST_CPI_INVALID_LAMPORTS_POINTER = 36; +static const uint8_t TEST_CPI_INVALID_DATA_POINTER = 37; static const int MINT_INDEX = 0; static const int ARGUMENT_INDEX = 1; @@ -663,6 +669,73 @@ extern uint64_t entrypoint(const uint8_t *input) { sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts)); break; } + case TEST_CPI_INVALID_KEY_POINTER: + { + sol_log("Test TEST_CPI_INVALID_KEY_POINTER"); + SolAccountMeta arguments[] = { + {accounts[ARGUMENT_INDEX].key, false, false}, + {accounts[INVOKED_ARGUMENT_INDEX].key, false, false}, + }; + uint8_t data[] = {}; + SolPubkey key = *accounts[ARGUMENT_INDEX].key; + accounts[ARGUMENT_INDEX].key = &key; + + const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, SOL_ARRAY_SIZE(data)}; + sol_invoke(&instruction, accounts, 4); + break; + } + case TEST_CPI_INVALID_LAMPORTS_POINTER: + { + sol_log("Test TEST_CPI_INVALID_LAMPORTS_POINTER"); + SolAccountMeta arguments[] = { + {accounts[ARGUMENT_INDEX].key, false, false}, + {accounts[INVOKED_ARGUMENT_INDEX].key, false, false}, + }; + uint8_t data[] = {}; + uint64_t lamports = *accounts[ARGUMENT_INDEX].lamports; + accounts[ARGUMENT_INDEX].lamports = &lamports; + + const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, SOL_ARRAY_SIZE(data)}; + sol_invoke(&instruction, accounts, 4); + break; + } + case TEST_CPI_INVALID_OWNER_POINTER: + { + sol_log("Test TEST_CPI_INVALID_OWNER_POINTER"); + SolAccountMeta arguments[] = { + {accounts[ARGUMENT_INDEX].key, false, false}, + {accounts[INVOKED_ARGUMENT_INDEX].key, false, false}, + }; + uint8_t data[] = {}; + SolPubkey owner = *accounts[ARGUMENT_INDEX].owner; + accounts[ARGUMENT_INDEX].owner = &owner; + + const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, SOL_ARRAY_SIZE(data)}; + sol_invoke(&instruction, accounts, 4); + break; + } + case TEST_CPI_INVALID_DATA_POINTER: + { + sol_log("Test TEST_CPI_INVALID_DATA_POINTER"); + SolAccountMeta arguments[] = { + {accounts[ARGUMENT_INDEX].key, false, false}, + {accounts[INVOKED_ARGUMENT_INDEX].key, false, false}, + }; + uint8_t data[] = {}; + accounts[ARGUMENT_INDEX].data = data; + + const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, SOL_ARRAY_SIZE(data)}; + sol_invoke(&instruction, accounts, 4); + break; + } default: sol_panic(); diff --git a/programs/sbf/rust/deprecated_loader/src/lib.rs b/programs/sbf/rust/deprecated_loader/src/lib.rs index 772e0c0f59..a9b801b5e4 100644 --- a/programs/sbf/rust/deprecated_loader/src/lib.rs +++ b/programs/sbf/rust/deprecated_loader/src/lib.rs @@ -5,10 +5,21 @@ extern crate solana_program; use solana_program::{ - account_info::AccountInfo, bpf_loader, entrypoint_deprecated::ProgramResult, log::*, msg, + account_info::AccountInfo, + bpf_loader, + entrypoint_deprecated::ProgramResult, + instruction::{AccountMeta, Instruction}, + log::*, + msg, + program::invoke, pubkey::Pubkey, }; +pub const REALLOC: u8 = 1; +pub const REALLOC_EXTEND_FROM_SLICE: u8 = 12; +pub const TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS: u8 = 28; +pub const TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_NESTED: u8 = 29; + #[derive(Debug, PartialEq)] struct SStruct { x: u64, @@ -39,39 +50,124 @@ fn process_instruction( assert!(!bpf_loader::check_id(program_id)); - // Log the provided account keys and instruction input data. In the case of - // the no-op program, no account keys or input data are expected but real - // programs will have specific requirements so they can do their work. - msg!("Account keys and instruction input data:"); - sol_log_params(accounts, instruction_data); + // test_sol_alloc_free_no_longer_deployable calls this program with + // bpf_loader instead of bpf_loader_deprecated, so instruction_data isn't + // deserialized correctly and is empty. + match instruction_data.first() { + Some(&REALLOC) => { + let (bytes, _) = instruction_data[2..].split_at(std::mem::size_of::()); + let new_len = usize::from_le_bytes(bytes.try_into().unwrap()); + msg!("realloc to {}", new_len); + let account = &accounts[0]; + account.realloc(new_len, false)?; + assert_eq!(new_len, account.data_len()); + } + Some(&REALLOC_EXTEND_FROM_SLICE) => { + msg!("realloc extend from slice deprecated"); + let data = &instruction_data[1..]; + let account = &accounts[0]; + let prev_len = account.data_len(); + account.realloc(prev_len + data.len(), false)?; + account.data.borrow_mut()[prev_len..].copy_from_slice(data); + } + Some(&TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS) => { + msg!("DEPRECATED TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS"); + const ARGUMENT_INDEX: usize = 1; + const CALLEE_PROGRAM_INDEX: usize = 3; + let account = &accounts[ARGUMENT_INDEX]; + let callee_program_id = accounts[CALLEE_PROGRAM_INDEX].key; - { - // Test - use std methods, unwrap + let expected = { + let data = &instruction_data[1..]; + let prev_len = account.data_len(); + // when direct mapping is off, this will accidentally clobber + // whatever comes after the data slice (owner, executable, rent + // epoch etc). When direct mapping is on, you get an + // InvalidRealloc error. + account.realloc(prev_len + data.len(), false)?; + account.data.borrow_mut()[prev_len..].copy_from_slice(data); + account.data.borrow().to_vec() + }; - // valid bytes, in a stack-allocated array - let sparkle_heart = [240, 159, 146, 150]; - let result_str = std::str::from_utf8(&sparkle_heart).unwrap(); - assert_eq!(4, result_str.len()); - assert_eq!("💖", result_str); - msg!(result_str); - } + let mut instruction_data = vec![TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_NESTED]; + instruction_data.extend_from_slice(&expected); + invoke( + &create_instruction( + *callee_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (callee_program_id, false, false), + ], + instruction_data, + ), + accounts, + ) + .unwrap(); + } + Some(&TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_NESTED) => { + msg!("DEPRECATED LOADER TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_NESTED"); + const ARGUMENT_INDEX: usize = 0; + let account = &accounts[ARGUMENT_INDEX]; + assert_eq!(*account.data.borrow(), &instruction_data[1..]); + } + _ => { + { + // Log the provided account keys and instruction input data. In the case of + // the no-op program, no account keys or input data are expected but real + // programs will have specific requirements so they can do their work. + msg!("Account keys and instruction input data:"); + sol_log_params(accounts, instruction_data); - { - // Test - struct return + // Test - use std methods, unwrap - let s = return_sstruct(); - assert_eq!(s.x + s.y + s.z, 6); - } + // valid bytes, in a stack-allocated array + let sparkle_heart = [240, 159, 146, 150]; + let result_str = std::str::from_utf8(&sparkle_heart).unwrap(); + assert_eq!(4, result_str.len()); + assert_eq!("💖", result_str); + msg!(result_str); + } - { - // Test - arch config - #[cfg(not(target_os = "solana"))] - panic!(); + { + // Test - struct return + + let s = return_sstruct(); + assert_eq!(s.x + s.y + s.z, 6); + } + + { + // Test - arch config + #[cfg(not(target_os = "solana"))] + panic!(); + } + } } Ok(()) } +pub fn create_instruction( + program_id: Pubkey, + arguments: &[(&Pubkey, bool, bool)], + data: Vec, +) -> Instruction { + let accounts = arguments + .iter() + .map(|(key, is_writable, is_signer)| { + if *is_writable { + AccountMeta::new(**key, *is_signer) + } else { + AccountMeta::new_readonly(**key, *is_signer) + } + }) + .collect(); + Instruction { + program_id, + accounts, + data, + } +} + #[cfg(test)] mod test { use super::*; diff --git a/programs/sbf/rust/invoke/Cargo.toml b/programs/sbf/rust/invoke/Cargo.toml index 616beab7a4..66b07e500d 100644 --- a/programs/sbf/rust/invoke/Cargo.toml +++ b/programs/sbf/rust/invoke/Cargo.toml @@ -16,6 +16,7 @@ program = [] [dependencies] solana-program = { workspace = true } solana-sbf-rust-invoked = { workspace = true } +solana-sbf-rust-realloc = { workspace = true } [lib] crate-type = ["lib", "cdylib"] diff --git a/programs/sbf/rust/invoke/src/instructions.rs b/programs/sbf/rust/invoke/src/instructions.rs index db8be12dea..b335fb52f5 100644 --- a/programs/sbf/rust/invoke/src/instructions.rs +++ b/programs/sbf/rust/invoke/src/instructions.rs @@ -21,6 +21,24 @@ pub const TEST_RETURN_DATA_TOO_LARGE: u8 = 18; pub const TEST_DUPLICATE_PRIVILEGE_ESCALATION_SIGNER: u8 = 19; pub const TEST_DUPLICATE_PRIVILEGE_ESCALATION_WRITABLE: u8 = 20; pub const TEST_MAX_ACCOUNT_INFOS_EXCEEDED: u8 = 21; +pub const TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLEE: u8 = 22; +pub const TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLEE_NESTED: u8 = 23; +pub const TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLER: u8 = 24; +pub const TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE_MOVING_DATA_POINTER: u8 = 25; +pub const TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE: u8 = 26; +pub const TEST_ALLOW_WRITE_AFTER_OWNERSHIP_CHANGE_TO_CALLER: u8 = 27; +pub const TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS: u8 = 28; +pub const TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_NESTED: u8 = 29; +pub const TEST_CPI_ACCOUNT_UPDATE_CALLEE_GROWS: u8 = 30; +pub const TEST_CPI_ACCOUNT_UPDATE_CALLEE_SHRINKS_SMALLER_THAN_ORIGINAL_LEN: u8 = 31; +pub const TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS: u8 = 32; +pub const TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS_NESTED: u8 = 33; +pub const TEST_CPI_INVALID_KEY_POINTER: u8 = 34; +pub const TEST_CPI_INVALID_OWNER_POINTER: u8 = 35; +pub const TEST_CPI_INVALID_LAMPORTS_POINTER: u8 = 36; +pub const TEST_CPI_INVALID_DATA_POINTER: u8 = 37; +pub const TEST_CPI_CHANGE_ACCOUNT_DATA_MEMORY_ALLOCATION: u8 = 38; +pub const TEST_WRITE_ACCOUNT: u8 = 39; pub const MINT_INDEX: usize = 0; pub const ARGUMENT_INDEX: usize = 1; diff --git a/programs/sbf/rust/invoke/src/processor.rs b/programs/sbf/rust/invoke/src/processor.rs index dce6492ece..2e1ee8cac9 100644 --- a/programs/sbf/rust/invoke/src/processor.rs +++ b/programs/sbf/rust/invoke/src/processor.rs @@ -2,11 +2,13 @@ #![cfg(feature = "program")] #![allow(unreachable_code)] +#![allow(clippy::integer_arithmetic)] use { crate::instructions::*, solana_program::{ account_info::AccountInfo, + bpf_loader_deprecated, entrypoint::{ProgramResult, MAX_PERMITTED_DATA_INCREASE}, instruction::Instruction, msg, @@ -16,9 +18,11 @@ use { syscalls::{ MAX_CPI_ACCOUNT_INFOS, MAX_CPI_INSTRUCTION_ACCOUNTS, MAX_CPI_INSTRUCTION_DATA_LEN, }, - system_instruction, + system_instruction, system_program, }, solana_sbf_rust_invoked::instructions::*, + solana_sbf_rust_realloc::instructions::*, + std::{cell::RefCell, mem, rc::Rc, slice}, }; fn do_nested_invokes(num_nested_invokes: u64, accounts: &[AccountInfo]) -> ProgramResult { @@ -688,8 +692,614 @@ fn process_instruction( invoked_instruction.accounts[1].is_writable = true; invoke(&invoked_instruction, accounts)?; } - _ => panic!(), + TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLEE => { + msg!("TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLEE"); + invoke( + &create_instruction( + *program_id, + &[ + (program_id, false, false), + (accounts[ARGUMENT_INDEX].key, true, false), + ], + vec![ + TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLEE_NESTED, + 42, + 42, + 42, + ], + ), + accounts, + ) + .unwrap(); + let account = &accounts[ARGUMENT_INDEX]; + // this should cause the tx to fail since the callee changed ownership + unsafe { + *account + .data + .borrow_mut() + .get_unchecked_mut(instruction_data[1] as usize) = 42 + }; + } + TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLEE_NESTED => { + msg!("TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLEE_NESTED"); + let account = &accounts[ARGUMENT_INDEX]; + account.data.borrow_mut().fill(0); + account.assign(&system_program::id()); + } + TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLER => { + msg!("TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLER"); + let account = &accounts[ARGUMENT_INDEX]; + let invoked_program_id = accounts[INVOKED_PROGRAM_INDEX].key; + account.data.borrow_mut().fill(0); + account.assign(invoked_program_id); + invoke( + &create_instruction( + *invoked_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (invoked_program_id, false, false), + ], + vec![RETURN_OK], + ), + accounts, + ) + .unwrap(); + // this should cause the tx to failsince invoked_program_id now owns + // the account + unsafe { + *account + .data + .borrow_mut() + .get_unchecked_mut(instruction_data[1] as usize) = 42 + }; + } + TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE_MOVING_DATA_POINTER => { + msg!("TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE_MOVING_DATA_POINTER"); + const INVOKE_PROGRAM_INDEX: usize = 3; + const REALLOC_PROGRAM_INDEX: usize = 4; + let account = &accounts[ARGUMENT_INDEX]; + let realloc_program_id = accounts[REALLOC_PROGRAM_INDEX].key; + let invoke_program_id = accounts[INVOKE_PROGRAM_INDEX].key; + account.realloc(0, false).unwrap(); + account.assign(realloc_program_id); + + // Place a RcBox> in the account data. This + // allows us to test having CallerAccount::ref_to_len_in_vm in an + // account region. + let rc_box_addr = + account.data.borrow_mut().as_mut_ptr() as *mut RcBox>; + let rc_box_size = mem::size_of::>>(); + unsafe { + std::ptr::write( + rc_box_addr, + RcBox { + strong: 1, + weak: 0, + // We're testing what happens if we make CPI update the + // slice length after we put the slice in the account + // address range. To do so, we need to move the data + // pointer past the RcBox or CPI will clobber the length + // change when it copies the callee's account data back + // into the caller's account data + // https://github.com/solana-labs/solana/blob/fa28958bd69054d1c2348e0a731011e93d44d7af/programs/bpf_loader/src/syscalls/cpi.rs#L1487 + value: RefCell::new(slice::from_raw_parts_mut( + account.data.borrow_mut().as_mut_ptr().add(rc_box_size), + 0, + )), + }, + ); + } + + // CPI now will update the serialized length in the wrong place, + // since we moved the account data slice. To hit the corner case we + // want to hit, we'll need to update the serialized length manually + // or during deserialize_parameters() we'll get + // AccountDataSizeChanged + let serialized_len_ptr = + unsafe { account.data.borrow_mut().as_mut_ptr().offset(-8) as *mut u64 }; + unsafe { + std::ptr::write( + &account.data as *const _ as usize as *mut Rc>, + Rc::from_raw(((rc_box_addr as usize) + mem::size_of::() * 2) as *mut _), + ); + } + + let mut instruction_data = vec![REALLOC, 0]; + instruction_data.extend_from_slice(&rc_box_size.to_le_bytes()); + + // check that the account is empty before we CPI + assert_eq!(account.data_len(), 0); + + invoke( + &create_instruction( + *realloc_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (realloc_program_id, false, false), + (invoke_program_id, false, false), + ], + instruction_data.to_vec(), + ), + accounts, + ) + .unwrap(); + + // verify that CPI did update `ref_to_len_in_vm` + assert_eq!(account.data_len(), rc_box_size); + + // update the serialized length so we don't error out early with AccountDataSizeChanged + unsafe { *serialized_len_ptr = rc_box_size as u64 }; + + // hack to avoid dropping the RcBox, which is supposed to be on the + // heap but we put it into account data. If we don't do this, + // dropping the Rc will cause + // global_deallocator.dealloc(rc_box_addr) which is invalid and + // happens to write a poison value into the account. + unsafe { + std::ptr::write( + &account.data as *const _ as usize as *mut Rc>, + Rc::new(RefCell::new(&mut [])), + ); + } + } + TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE => { + msg!("TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE"); + const INVOKE_PROGRAM_INDEX: usize = 3; + const REALLOC_PROGRAM_INDEX: usize = 4; + let account = &accounts[ARGUMENT_INDEX]; + let target_account_index = instruction_data[1] as usize; + let target_account = &accounts[target_account_index]; + let realloc_program_id = accounts[REALLOC_PROGRAM_INDEX].key; + let invoke_program_id = accounts[INVOKE_PROGRAM_INDEX].key; + account.realloc(0, false).unwrap(); + account.assign(realloc_program_id); + target_account.realloc(0, false).unwrap(); + target_account.assign(realloc_program_id); + + let rc_box_addr = + target_account.data.borrow_mut().as_mut_ptr() as *mut RcBox>; + let rc_box_size = mem::size_of::>>(); + unsafe { + std::ptr::write( + rc_box_addr, + RcBox { + strong: 1, + weak: 0, + // The difference with + // TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE_MOVING_DATA_POINTER + // is that we don't move the data pointer past the + // RcBox. This is needed to avoid the "Invalid account + // info pointer" check when direct mapping is enabled. + // This also means we don't need to update the + // serialized len like we do in the other test. + value: RefCell::new(slice::from_raw_parts_mut( + account.data.borrow_mut().as_mut_ptr(), + 0, + )), + }, + ); + } + + let serialized_len_ptr = + unsafe { account.data.borrow_mut().as_mut_ptr().offset(-8) as *mut u64 }; + // Place a RcBox> in the account data. This + // allows us to test having CallerAccount::ref_to_len_in_vm in an + // account region. + unsafe { + std::ptr::write( + &account.data as *const _ as usize as *mut Rc>, + Rc::from_raw(((rc_box_addr as usize) + mem::size_of::() * 2) as *mut _), + ); + } + + let mut instruction_data = vec![REALLOC, 0]; + instruction_data.extend_from_slice(&rc_box_size.to_le_bytes()); + + // check that the account is empty before we CPI + assert_eq!(account.data_len(), 0); + + invoke( + &create_instruction( + *realloc_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (target_account.key, true, false), + (realloc_program_id, false, false), + (invoke_program_id, false, false), + ], + instruction_data.to_vec(), + ), + accounts, + ) + .unwrap(); + + unsafe { *serialized_len_ptr = rc_box_size as u64 }; + // hack to avoid dropping the RcBox, which is supposed to be on the + // heap but we put it into account data. If we don't do this, + // dropping the Rc will cause + // global_deallocator.dealloc(rc_box_addr) which is invalid and + // happens to write a poison value into the account. + unsafe { + std::ptr::write( + &account.data as *const _ as usize as *mut Rc>, + Rc::new(RefCell::new(&mut [])), + ); + } + } + TEST_ALLOW_WRITE_AFTER_OWNERSHIP_CHANGE_TO_CALLER => { + msg!("TEST_ALLOW_WRITE_AFTER_OWNERSHIP_CHANGE_TO_CALLER"); + const INVOKE_PROGRAM_INDEX: usize = 3; + let account = &accounts[ARGUMENT_INDEX]; + let invoked_program_id = accounts[INVOKED_PROGRAM_INDEX].key; + let invoke_program_id = accounts[INVOKE_PROGRAM_INDEX].key; + invoke( + &create_instruction( + *invoked_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (invoked_program_id, false, false), + (invoke_program_id, false, false), + ], + vec![ASSIGN_ACCOUNT_TO_CALLER], + ), + accounts, + ) + .unwrap(); + // this should succeed since the callee gave us ownership of the + // account + unsafe { + *account + .data + .borrow_mut() + .get_unchecked_mut(instruction_data[1] as usize) = 42 + }; + } + TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS => { + msg!("TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS"); + const CALLEE_PROGRAM_INDEX: usize = 3; + let account = &accounts[ARGUMENT_INDEX]; + let callee_program_id = accounts[CALLEE_PROGRAM_INDEX].key; + + let expected = { + let data = &instruction_data[1..]; + let prev_len = account.data_len(); + account.realloc(prev_len + data.len(), false)?; + account.data.borrow_mut()[prev_len..].copy_from_slice(data); + account.data.borrow().to_vec() + }; + + let mut instruction_data = vec![TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_NESTED]; + instruction_data.extend_from_slice(&expected); + invoke( + &create_instruction( + *callee_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (callee_program_id, false, false), + ], + instruction_data, + ), + accounts, + ) + .unwrap(); + } + TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_NESTED => { + msg!("TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_NESTED"); + const ARGUMENT_INDEX: usize = 0; + let account = &accounts[ARGUMENT_INDEX]; + assert_eq!(*account.data.borrow(), &instruction_data[1..]); + } + TEST_CPI_ACCOUNT_UPDATE_CALLEE_GROWS => { + msg!("TEST_CPI_ACCOUNT_UPDATE_CALLEE_GROWS"); + const REALLOC_PROGRAM_INDEX: usize = 2; + const INVOKE_PROGRAM_INDEX: usize = 3; + let account = &accounts[ARGUMENT_INDEX]; + let realloc_program_id = accounts[REALLOC_PROGRAM_INDEX].key; + let realloc_program_owner = accounts[REALLOC_PROGRAM_INDEX].owner; + let invoke_program_id = accounts[INVOKE_PROGRAM_INDEX].key; + let mut instruction_data = instruction_data.to_vec(); + let mut expected = account.data.borrow().to_vec(); + expected.extend_from_slice(&instruction_data[1..]); + instruction_data[0] = REALLOC_EXTEND_FROM_SLICE; + invoke( + &create_instruction( + *realloc_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (realloc_program_id, false, false), + (invoke_program_id, false, false), + ], + instruction_data.to_vec(), + ), + accounts, + ) + .unwrap(); + + if !bpf_loader_deprecated::check_id(realloc_program_owner) { + assert_eq!(&*account.data.borrow(), &expected); + } + } + TEST_CPI_ACCOUNT_UPDATE_CALLEE_SHRINKS_SMALLER_THAN_ORIGINAL_LEN => { + msg!("TEST_CPI_ACCOUNT_UPDATE_CALLEE_SHRINKS_SMALLER_THAN_ORIGINAL_LEN"); + const REALLOC_PROGRAM_INDEX: usize = 2; + const INVOKE_PROGRAM_INDEX: usize = 3; + let account = &accounts[ARGUMENT_INDEX]; + let realloc_program_id = accounts[REALLOC_PROGRAM_INDEX].key; + let realloc_program_owner = accounts[REALLOC_PROGRAM_INDEX].owner; + let invoke_program_id = accounts[INVOKE_PROGRAM_INDEX].key; + let new_len = usize::from_le_bytes(instruction_data[1..9].try_into().unwrap()); + let prev_len = account.data_len(); + let expected = account.data.borrow()[..new_len].to_vec(); + let mut instruction_data = vec![REALLOC, 0]; + instruction_data.extend_from_slice(&new_len.to_le_bytes()); + invoke( + &create_instruction( + *realloc_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (realloc_program_id, false, false), + (invoke_program_id, false, false), + ], + instruction_data, + ), + accounts, + ) + .unwrap(); + + // deserialize_parameters_unaligned predates realloc support, and + // hardcodes the account data length to the original length. + if !bpf_loader_deprecated::check_id(realloc_program_owner) { + assert_eq!(&*account.data.borrow(), &expected); + assert_eq!( + unsafe { + slice::from_raw_parts( + account.data.borrow().as_ptr().add(new_len), + prev_len - new_len, + ) + }, + &vec![0; prev_len - new_len] + ); + } + } + TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS => { + msg!("TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS"); + const INVOKE_PROGRAM_INDEX: usize = 3; + const SENTINEL: u8 = 42; + let account = &accounts[ARGUMENT_INDEX]; + let invoke_program_id = accounts[INVOKE_PROGRAM_INDEX].key; + + let prev_data = { + let data = &instruction_data[9..]; + let prev_len = account.data_len(); + account.realloc(prev_len + data.len(), false)?; + account.data.borrow_mut()[prev_len..].copy_from_slice(data); + unsafe { + // write a sentinel value just outside the account data to + // check that when CPI zeroes the realloc region it doesn't + // zero too much + *account + .data + .borrow_mut() + .as_mut_ptr() + .add(prev_len + data.len()) = SENTINEL; + }; + account.data.borrow().to_vec() + }; + + let mut expected = account.data.borrow().to_vec(); + let new_len = usize::from_le_bytes(instruction_data[1..9].try_into().unwrap()); + expected.extend_from_slice(&instruction_data[9..]); + let mut instruction_data = + vec![TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS_NESTED]; + instruction_data.extend_from_slice(&new_len.to_le_bytes()); + invoke( + &create_instruction( + *invoke_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (invoke_program_id, false, false), + ], + instruction_data, + ), + accounts, + ) + .unwrap(); + + assert_eq!(*account.data.borrow(), &prev_data[..new_len]); + assert_eq!( + unsafe { + slice::from_raw_parts( + account.data.borrow().as_ptr().add(new_len), + prev_data.len() - new_len, + ) + }, + &vec![0; prev_data.len() - new_len] + ); + assert_eq!( + unsafe { *account.data.borrow().as_ptr().add(prev_data.len()) }, + SENTINEL + ); + } + TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS_NESTED => { + msg!("TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS_NESTED"); + const ARGUMENT_INDEX: usize = 0; + let account = &accounts[ARGUMENT_INDEX]; + let new_len = usize::from_le_bytes(instruction_data[1..9].try_into().unwrap()); + account.realloc(new_len, false).unwrap(); + } + TEST_CPI_INVALID_KEY_POINTER => { + msg!("TEST_CPI_INVALID_KEY_POINTER"); + const CALLEE_PROGRAM_INDEX: usize = 2; + let account = &accounts[ARGUMENT_INDEX]; + let key = *account.key; + let key = &key as *const _ as usize; + unsafe { + *mem::transmute::<_, *mut *const Pubkey>(&account.key) = key as *const Pubkey; + } + let callee_program_id = accounts[CALLEE_PROGRAM_INDEX].key; + + invoke( + &create_instruction( + *callee_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (callee_program_id, false, false), + ], + vec![], + ), + accounts, + ) + .unwrap(); + } + TEST_CPI_INVALID_LAMPORTS_POINTER => { + msg!("TEST_CPI_INVALID_LAMPORTS_POINTER"); + const CALLEE_PROGRAM_INDEX: usize = 2; + let account = &accounts[ARGUMENT_INDEX]; + let mut lamports = account.lamports(); + account + .lamports + .replace(unsafe { mem::transmute(&mut lamports) }); + let callee_program_id = accounts[CALLEE_PROGRAM_INDEX].key; + + invoke( + &create_instruction( + *callee_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (callee_program_id, false, false), + ], + vec![], + ), + accounts, + ) + .unwrap(); + } + TEST_CPI_INVALID_OWNER_POINTER => { + msg!("TEST_CPI_INVALID_OWNER_POINTER"); + const CALLEE_PROGRAM_INDEX: usize = 2; + let account = &accounts[ARGUMENT_INDEX]; + let owner = account.owner as *const _ as usize + 1; + unsafe { + *mem::transmute::<_, *mut *const Pubkey>(&account.owner) = owner as *const Pubkey; + } + let callee_program_id = accounts[CALLEE_PROGRAM_INDEX].key; + + invoke( + &create_instruction( + *callee_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (callee_program_id, false, false), + ], + vec![], + ), + accounts, + ) + .unwrap(); + } + TEST_CPI_INVALID_DATA_POINTER => { + msg!("TEST_CPI_INVALID_DATA_POINTER"); + const CALLEE_PROGRAM_INDEX: usize = 2; + let account = &accounts[ARGUMENT_INDEX]; + let data = unsafe { + slice::from_raw_parts_mut(account.data.borrow_mut()[1..].as_mut_ptr(), 0) + }; + account.data.replace(data); + let callee_program_id = accounts[CALLEE_PROGRAM_INDEX].key; + + invoke( + &create_instruction( + *callee_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (callee_program_id, false, false), + ], + vec![], + ), + accounts, + ) + .unwrap(); + } + TEST_CPI_CHANGE_ACCOUNT_DATA_MEMORY_ALLOCATION => { + msg!("TEST_CPI_CHANGE_ACCOUNT_DATA_MEMORY_ALLOCATION"); + const CALLEE_PROGRAM_INDEX: usize = 2; + let account = &accounts[ARGUMENT_INDEX]; + let callee_program_id = accounts[CALLEE_PROGRAM_INDEX].key; + let original_data_len = account.data_len(); + + // Initial data is all [0xFF; 20] + assert_eq!(&*account.data.borrow(), &[0xFF; 20]); + + // Realloc to [0xFE; 10] + invoke( + &create_instruction( + *callee_program_id, + &[ + (account.key, true, false), + (callee_program_id, false, false), + ], + vec![0xFE; 10], + ), + accounts, + ) + .unwrap(); + + // Check that [10..20] is zeroed + let new_len = account.data_len(); + assert_eq!(&*account.data.borrow(), &[0xFE; 10]); + assert_eq!( + unsafe { + slice::from_raw_parts( + account.data.borrow().as_ptr().add(new_len), + original_data_len - new_len, + ) + }, + &vec![0; original_data_len - new_len] + ); + + // Realloc to [0xFD; 5] + invoke( + &create_instruction( + *callee_program_id, + &[ + (accounts[ARGUMENT_INDEX].key, true, false), + (callee_program_id, false, false), + ], + vec![0xFD; 5], + ), + accounts, + ) + .unwrap(); + + // Check that [5..20] is zeroed + let new_len = account.data_len(); + assert_eq!(&*account.data.borrow(), &[0xFD; 5]); + assert_eq!( + unsafe { + slice::from_raw_parts( + account.data.borrow().as_ptr().add(new_len), + original_data_len - new_len, + ) + }, + &vec![0; original_data_len - new_len] + ); + } + TEST_WRITE_ACCOUNT => { + msg!("TEST_WRITE_ACCOUNT"); + let target_account_index = instruction_data[1] as usize; + let target_account = &accounts[target_account_index]; + let byte_index = usize::from_le_bytes(instruction_data[2..10].try_into().unwrap()); + target_account.data.borrow_mut()[byte_index] = instruction_data[10]; + } + _ => panic!("unexpected program data"), } Ok(()) } + +#[repr(C)] +struct RcBox { + strong: usize, + weak: usize, + value: T, +} diff --git a/programs/sbf/rust/invoked/src/instructions.rs b/programs/sbf/rust/invoked/src/instructions.rs index 9f98da7d92..46a4f1d9f2 100644 --- a/programs/sbf/rust/invoked/src/instructions.rs +++ b/programs/sbf/rust/invoked/src/instructions.rs @@ -19,6 +19,7 @@ pub const VERIFY_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: u8 = 10; pub const WRITE_ACCOUNT: u8 = 11; pub const CREATE_AND_INIT: u8 = 12; pub const SET_RETURN_DATA: u8 = 13; +pub const ASSIGN_ACCOUNT_TO_CALLER: u8 = 14; pub fn create_instruction( program_id: Pubkey, diff --git a/programs/sbf/rust/invoked/src/processor.rs b/programs/sbf/rust/invoked/src/processor.rs index 73bf25cac7..52d02dc99a 100644 --- a/programs/sbf/rust/invoked/src/processor.rs +++ b/programs/sbf/rust/invoked/src/processor.rs @@ -297,6 +297,14 @@ fn process_instruction( set_return_data(b"Set by invoked"); } + ASSIGN_ACCOUNT_TO_CALLER => { + msg!("Assigning account to caller"); + const ARGUMENT_INDEX: usize = 0; + const CALLER_PROGRAM_ID: usize = 2; + let account = &accounts[ARGUMENT_INDEX]; + let caller_program_id = accounts[CALLER_PROGRAM_ID].key; + account.assign(caller_program_id); + } _ => panic!(), } diff --git a/programs/sbf/rust/realloc/src/instructions.rs b/programs/sbf/rust/realloc/src/instructions.rs index 831a69029e..e15ba5d48c 100644 --- a/programs/sbf/rust/realloc/src/instructions.rs +++ b/programs/sbf/rust/realloc/src/instructions.rs @@ -16,6 +16,7 @@ pub const CHECK: u8 = 8; pub const ZERO_INIT: u8 = 9; pub const REALLOC_EXTEND_AND_UNDO: u8 = 10; pub const EXTEND_AND_WRITE_U64: u8 = 11; +pub const REALLOC_EXTEND_FROM_SLICE: u8 = 12; pub fn realloc(program_id: &Pubkey, address: &Pubkey, size: usize, bump: &mut u8) -> Instruction { let mut instruction_data = vec![REALLOC, *bump]; diff --git a/programs/sbf/rust/realloc/src/processor.rs b/programs/sbf/rust/realloc/src/processor.rs index 782eb1cd50..172ed7498f 100644 --- a/programs/sbf/rust/realloc/src/processor.rs +++ b/programs/sbf/rust/realloc/src/processor.rs @@ -1,6 +1,7 @@ //! Example Rust-based SBF realloc test program #![cfg(feature = "program")] +#![allow(clippy::integer_arithmetic)] extern crate solana_program; use { @@ -184,6 +185,13 @@ fn process_instruction( } } } + REALLOC_EXTEND_FROM_SLICE => { + msg!("realloc extend from slice"); + let data = &instruction_data[1..]; + let prev_len = account.data_len(); + account.realloc(prev_len.saturating_add(data.len()), false)?; + account.data.borrow_mut()[prev_len..].copy_from_slice(data); + } _ => panic!(), } diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 6acb18e479..b690ea2ffe 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -347,9 +347,9 @@ fn test_program_sbf_sanity() { let instruction = Instruction::new_with_bytes(program_id, &[1], account_metas); let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); if program.1 { - assert!(result.is_ok()); + assert!(result.is_ok(), "{result:?}"); } else { - assert!(result.is_err()); + assert!(result.is_err(), "{result:?}"); } } } @@ -389,7 +389,7 @@ fn test_program_sbf_loader_deprecated() { .advance_slot(1, &Pubkey::default()) .expect("Failed to advance the slot"); let account_metas = vec![AccountMeta::new(mint_keypair.pubkey(), true)]; - let instruction = Instruction::new_with_bytes(program_id, &[1], account_metas); + let instruction = Instruction::new_with_bytes(program_id, &[255], account_metas); let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); assert!(result.is_ok()); } @@ -437,7 +437,7 @@ fn test_sol_alloc_free_no_longer_deployable() { Message::new( &[Instruction::new_with_bytes( program_address, - &[1], + &[255], vec![AccountMeta::new(mint_keypair.pubkey(), true)], )], Some(&mint_keypair.pubkey()), @@ -3939,5 +3939,640 @@ fn test_program_sbf_inner_instruction_alignment_checks() { instruction.data[0] += 1; let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction.clone()); - assert!(result.is_ok()); + assert!(result.is_ok(), "{result:?}"); +} + +#[test] +#[cfg(feature = "sbf_rust")] +fn test_cpi_account_ownership_writability() { + solana_logger::setup(); + + for direct_mapping in [false, true] { + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(100_123_456_789); + let mut bank = Bank::new_for_tests(&genesis_config); + let mut feature_set = FeatureSet::all_enabled(); + if !direct_mapping { + feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id()); + } + bank.feature_set = Arc::new(feature_set); + let bank = Arc::new(bank); + let mut bank_client = BankClient::new_shared(bank); + + let invoke_program_id = load_program( + &bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_sbf_rust_invoke", + ); + + let invoked_program_id = load_program( + &bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_sbf_rust_invoked", + ); + + let (bank, realloc_program_id) = load_program_and_advance_slot( + &mut bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_sbf_rust_realloc", + ); + + let account_keypair = Keypair::new(); + + let mint_pubkey = mint_keypair.pubkey(); + let account_metas = vec![ + AccountMeta::new(mint_pubkey, true), + AccountMeta::new(account_keypair.pubkey(), false), + AccountMeta::new_readonly(invoked_program_id, false), + AccountMeta::new_readonly(invoke_program_id, false), + AccountMeta::new_readonly(realloc_program_id, false), + ]; + + for (account_size, byte_index) in [ + (0, 0), // first realloc byte + (0, MAX_PERMITTED_DATA_INCREASE as u8), // last realloc byte + (2, 0), // first data byte + (2, 1), // last data byte + (2, 3), // first realloc byte + (2, 2 + MAX_PERMITTED_DATA_INCREASE as u8), // last realloc byte + ] { + for instruction_id in [ + TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLEE, + TEST_FORBID_WRITE_AFTER_OWNERSHIP_CHANGE_IN_CALLER, + ] { + bank.register_recent_blockhash(&Hash::new_unique()); + let account = AccountSharedData::new(42, account_size, &invoke_program_id); + bank.store_account(&account_keypair.pubkey(), &account); + + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &[instruction_id, byte_index, 42, 42], + account_metas.clone(), + ); + + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + + if (byte_index as usize) < account_size || direct_mapping { + assert_eq!( + result.unwrap_err().unwrap(), + TransactionError::InstructionError( + 0, + InstructionError::ExternalAccountDataModified + ) + ); + } else { + // without direct mapping, changes to the realloc padding + // outside the account length are ignored + assert!(result.is_ok(), "{result:?}"); + } + } + } + // Test that the CPI code that updates `ref_to_len_in_vm` fails if we + // make it write to an invalid location. This is the first variant which + // correctly triggers ExternalAccountDataModified when direct mapping is + // disabled. When direct mapping is enabled this tests fails early + // because we move the account data pointer. + // TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE is able to make more + // progress when direct mapping is on. + let account = AccountSharedData::new(42, 0, &invoke_program_id); + bank.store_account(&account_keypair.pubkey(), &account); + let instruction_data = vec![ + TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE_MOVING_DATA_POINTER, + 42, + 42, + 42, + ]; + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &instruction_data, + account_metas.clone(), + ); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + assert_eq!( + result.unwrap_err().unwrap(), + if direct_mapping { + // We move the data pointer, direct mapping doesn't allow it + // anymore so it errors out earlier. See + // test_cpi_invalid_account_info_pointers. + TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete) + } else { + // We managed to make CPI write into the account data, but the + // usual checks still apply and we get an error. + TransactionError::InstructionError(0, InstructionError::ExternalAccountDataModified) + } + ); + + // We're going to try and make CPI write ref_to_len_in_vm into a 2nd + // account, so we add an extra one here. + let account2_keypair = Keypair::new(); + let mut account_metas = account_metas.clone(); + account_metas.push(AccountMeta::new(account2_keypair.pubkey(), false)); + + for target_account in [1, account_metas.len() as u8 - 1] { + // Similar to the test above where we try to make CPI write into account + // data. This variant is for when direct mapping is enabled. + let account = AccountSharedData::new(42, 0, &invoke_program_id); + bank.store_account(&account_keypair.pubkey(), &account); + let account = AccountSharedData::new(42, 0, &invoke_program_id); + bank.store_account(&account2_keypair.pubkey(), &account); + let instruction_data = vec![ + TEST_FORBID_LEN_UPDATE_AFTER_OWNERSHIP_CHANGE, + target_account, + 42, + 42, + ]; + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &instruction_data, + account_metas.clone(), + ); + let message = Message::new(&[instruction], Some(&mint_pubkey)); + let tx = Transaction::new(&[&mint_keypair], message.clone(), bank.last_blockhash()); + let (result, _, logs) = process_transaction_and_record_inner(&bank, tx); + if direct_mapping { + assert_eq!( + result.unwrap_err(), + TransactionError::InstructionError( + 0, + InstructionError::ProgramFailedToComplete + ) + ); + // We haven't moved the data pointer, but ref_to_len_vm _is_ in + // the account data vm range and that's not allowed either. + assert!( + logs.iter().any(|log| log.contains("Invalid pointer")), + "{logs:?}" + ); + } else { + // we expect this to succeed as after updating `ref_to_len_in_vm`, + // CPI will sync the actual account data between the callee and the + // caller, _always_ writing over the location pointed by + // `ref_to_len_in_vm`. To verify this, we check that the account + // data is in fact all zeroes like it is in the callee. + result.unwrap(); + let account = bank.get_account(&account_keypair.pubkey()).unwrap(); + assert_eq!(account.data(), vec![0; 40]); + } + } + } +} + +#[test] +#[cfg(feature = "sbf_rust")] +fn test_cpi_account_data_updates() { + solana_logger::setup(); + + for direct_mapping in [false, true] { + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(100_123_456_789); + let mut bank = Bank::new_for_tests(&genesis_config); + let mut feature_set = FeatureSet::all_enabled(); + if !direct_mapping { + feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id()); + } + bank.feature_set = Arc::new(feature_set); + let bank = Arc::new(bank); + let mut bank_client = BankClient::new_shared(bank); + + let invoke_program_id = load_program( + &bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_sbf_rust_invoke", + ); + + let (bank, realloc_program_id) = load_program_and_advance_slot( + &mut bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_sbf_rust_realloc", + ); + + let account_keypair = Keypair::new(); + let mint_pubkey = mint_keypair.pubkey(); + let account_metas = vec![ + AccountMeta::new(mint_pubkey, true), + AccountMeta::new(account_keypair.pubkey(), false), + AccountMeta::new_readonly(realloc_program_id, false), + AccountMeta::new_readonly(invoke_program_id, false), + ]; + + // This tests the case where a caller extends an account beyond the original + // data length. The callee should see the extended data (asserted in the + // callee program, not here). + let mut account = AccountSharedData::new(42, 0, &invoke_program_id); + account.set_data(b"foo".to_vec()); + bank.store_account(&account_keypair.pubkey(), &account); + let mut instruction_data = vec![TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS]; + instruction_data.extend_from_slice(b"bar"); + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &instruction_data, + account_metas.clone(), + ); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + assert!(result.is_ok(), "{result:?}"); + let account = bank.get_account(&account_keypair.pubkey()).unwrap(); + // "bar" here was copied from the realloc region + assert_eq!(account.data(), b"foobar"); + + // This tests the case where a callee extends an account beyond the original + // data length. The caller should see the extended data where the realloc + // region contains the new data. In this test the callee owns the account, + // the caller can't write but the CPI glue still updates correctly. + let mut account = AccountSharedData::new(42, 0, &realloc_program_id); + account.set_data(b"foo".to_vec()); + bank.store_account(&account_keypair.pubkey(), &account); + let mut instruction_data = vec![TEST_CPI_ACCOUNT_UPDATE_CALLEE_GROWS]; + instruction_data.extend_from_slice(b"bar"); + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &instruction_data, + account_metas.clone(), + ); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + result.unwrap(); + let account = bank.get_account(&account_keypair.pubkey()).unwrap(); + // "bar" here was copied from the realloc region + assert_eq!(account.data(), b"foobar"); + + // This tests the case where a callee shrinks an account, the caller data + // slice must be truncated accordingly and post_len..original_data_len must + // be zeroed (zeroing is checked in the invoked program not here). Same as + // above, the callee owns the account but the changes are still reflected in + // the caller even if things are readonly from the caller's POV. + let mut account = AccountSharedData::new(42, 0, &realloc_program_id); + account.set_data(b"foobar".to_vec()); + bank.store_account(&account_keypair.pubkey(), &account); + let mut instruction_data = + vec![TEST_CPI_ACCOUNT_UPDATE_CALLEE_SHRINKS_SMALLER_THAN_ORIGINAL_LEN]; + instruction_data.extend_from_slice(4usize.to_le_bytes().as_ref()); + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &instruction_data, + account_metas.clone(), + ); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + assert!(result.is_ok(), "{result:?}"); + let account = bank.get_account(&account_keypair.pubkey()).unwrap(); + assert_eq!(account.data(), b"foob"); + + // This tests the case where the program extends an account, then calls + // itself and in the inner call it shrinks the account to a size that is + // still larger than the original size. The account data must be set to the + // correct value in the caller frame, and the realloc region must be zeroed + // (again tested in the invoked program). + let mut account = AccountSharedData::new(42, 0, &invoke_program_id); + account.set_data(b"foo".to_vec()); + bank.store_account(&account_keypair.pubkey(), &account); + let mut instruction_data = vec![TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS]; + // realloc to "foobazbad" then shrink to "foobazb" + instruction_data.extend_from_slice(7usize.to_le_bytes().as_ref()); + instruction_data.extend_from_slice(b"bazbad"); + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &instruction_data, + account_metas.clone(), + ); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + assert!(result.is_ok(), "{result:?}"); + let account = bank.get_account(&account_keypair.pubkey()).unwrap(); + assert_eq!(account.data(), b"foobazb"); + + // Similar to the test above, but this time the nested invocation shrinks to + // _below_ the original data length. Both the spare capacity in the account + // data _end_ the realloc region must be zeroed. + let mut account = AccountSharedData::new(42, 0, &invoke_program_id); + account.set_data(b"foo".to_vec()); + bank.store_account(&account_keypair.pubkey(), &account); + let mut instruction_data = vec![TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS]; + // realloc to "foobazbad" then shrink to "f" + instruction_data.extend_from_slice(1usize.to_le_bytes().as_ref()); + instruction_data.extend_from_slice(b"bazbad"); + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &instruction_data, + account_metas.clone(), + ); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + assert!(result.is_ok(), "{result:?}"); + let account = bank.get_account(&account_keypair.pubkey()).unwrap(); + assert_eq!(account.data(), b"f"); + } +} + +#[test] +#[cfg(feature = "sbf_rust")] +fn test_cpi_deprecated_loader_realloc() { + solana_logger::setup(); + + for direct_mapping in [false, true] { + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(100_123_456_789); + let mut bank = Bank::new_for_tests(&genesis_config); + let mut feature_set = FeatureSet::all_enabled(); + if !direct_mapping { + feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id()); + } + bank.feature_set = Arc::new(feature_set); + let bank = Arc::new(bank); + + let deprecated_program_id = create_program( + &bank, + &bpf_loader_deprecated::id(), + "solana_sbf_rust_deprecated_loader", + ); + + let mut bank_client = BankClient::new_shared(bank); + + let (bank, invoke_program_id) = load_program_and_advance_slot( + &mut bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_sbf_rust_invoke", + ); + + let account_keypair = Keypair::new(); + + let mint_pubkey = mint_keypair.pubkey(); + let account_metas = vec![ + AccountMeta::new(mint_pubkey, true), + AccountMeta::new(account_keypair.pubkey(), false), + AccountMeta::new_readonly(deprecated_program_id, false), + AccountMeta::new_readonly(deprecated_program_id, false), + AccountMeta::new_readonly(invoke_program_id, false), + ]; + + // If a bpf_loader_deprecated program extends an account, the callee + // accidentally sees the extended data when direct mapping is off, but + // direct mapping fixes the issue + let mut account = AccountSharedData::new(42, 0, &deprecated_program_id); + account.set_data(b"foo".to_vec()); + bank.store_account(&account_keypair.pubkey(), &account); + let mut instruction_data = vec![TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS]; + instruction_data.extend_from_slice(b"bar"); + let instruction = Instruction::new_with_bytes( + deprecated_program_id, + &instruction_data, + account_metas.clone(), + ); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + // when direct mapping is off, the realloc will accidentally clobber + // whatever comes after the data slice (owner, executable, rent epoch + // etc). When direct mapping is on, you get an InvalidRealloc error. + if direct_mapping { + assert_eq!( + result.unwrap_err().unwrap(), + TransactionError::InstructionError(0, InstructionError::InvalidRealloc) + ); + } else { + assert_eq!( + result.unwrap_err().unwrap(), + TransactionError::InstructionError(0, InstructionError::ModifiedProgramId) + ); + } + + // check that if a bpf_loader_deprecated program extends an account, the + // extended data is ignored + let mut account = AccountSharedData::new(42, 0, &deprecated_program_id); + account.set_data(b"foo".to_vec()); + bank.store_account(&account_keypair.pubkey(), &account); + let mut instruction_data = vec![TEST_CPI_ACCOUNT_UPDATE_CALLEE_GROWS]; + instruction_data.extend_from_slice(b"bar"); + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &instruction_data, + account_metas.clone(), + ); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + assert!(result.is_ok(), "{result:?}"); + let account = bank.get_account(&account_keypair.pubkey()).unwrap(); + assert_eq!(account.data(), b"foo"); + + // check that if a bpf_loader_deprecated program truncates an account, + // the caller doesn't see the truncation + let mut account = AccountSharedData::new(42, 0, &deprecated_program_id); + account.set_data(b"foobar".to_vec()); + bank.store_account(&account_keypair.pubkey(), &account); + let mut instruction_data = + vec![TEST_CPI_ACCOUNT_UPDATE_CALLEE_SHRINKS_SMALLER_THAN_ORIGINAL_LEN]; + instruction_data.extend_from_slice(4usize.to_le_bytes().as_ref()); + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &instruction_data, + account_metas.clone(), + ); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + assert!(result.is_ok(), "{result:?}"); + let account = bank.get_account(&account_keypair.pubkey()).unwrap(); + assert_eq!(account.data(), b"foobar"); + } +} + +#[test] +#[cfg(feature = "sbf_rust")] +fn test_cpi_change_account_data_memory_allocation() { + use solana_program_runtime::{declare_process_instruction, loaded_programs::LoadedProgram}; + + solana_logger::setup(); + + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(100_123_456_789); + let mut bank = Bank::new_for_tests(&genesis_config); + let feature_set = FeatureSet::all_enabled(); + bank.feature_set = Arc::new(feature_set); + + declare_process_instruction!(process_instruction, 42, |invoke_context| { + let transaction_context = &invoke_context.transaction_context; + let instruction_context = transaction_context.get_current_instruction_context()?; + let instruction_data = instruction_context.get_instruction_data(); + + let index_in_transaction = + instruction_context.get_index_of_instruction_account_in_transaction(0)?; + + let mut account = transaction_context + .accounts() + .get(index_in_transaction) + .unwrap() + .borrow_mut(); + + // Test changing the account data both in place and by changing the + // underlying vector. CPI will have to detect the vector change and + // update the corresponding memory region. In both cases CPI will have + // to zero the spare bytes correctly. + if instruction_data[0] == 0xFE { + account.set_data(instruction_data.to_vec()); + } else { + account.set_data_from_slice(instruction_data); + } + + Ok(()) + }); + + let builtin_program_id = Pubkey::new_unique(); + bank.add_builtin( + builtin_program_id, + "test_cpi_change_account_data_memory_allocation_builtin".to_string(), + LoadedProgram::new_builtin(0, 42, process_instruction), + ); + + let bank = Arc::new(bank); + let mut bank_client = BankClient::new_shared(bank); + + let (bank, invoke_program_id) = load_program_and_advance_slot( + &mut bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_sbf_rust_invoke", + ); + + let account_keypair = Keypair::new(); + let mint_pubkey = mint_keypair.pubkey(); + let account_metas = vec![ + AccountMeta::new(mint_pubkey, true), + AccountMeta::new(account_keypair.pubkey(), false), + AccountMeta::new_readonly(builtin_program_id, false), + AccountMeta::new_readonly(invoke_program_id, false), + ]; + + let mut account = AccountSharedData::new(42, 20, &builtin_program_id); + account.set_data(vec![0xFF; 20]); + bank.store_account(&account_keypair.pubkey(), &account); + let mut instruction_data = vec![TEST_CPI_CHANGE_ACCOUNT_DATA_MEMORY_ALLOCATION]; + instruction_data.extend_from_slice(builtin_program_id.as_ref()); + let instruction = + Instruction::new_with_bytes(invoke_program_id, &instruction_data, account_metas.clone()); + + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + assert!(result.is_ok(), "{result:?}"); +} + +#[test] +#[cfg(feature = "sbf_rust")] +fn test_cpi_invalid_account_info_pointers() { + solana_logger::setup(); + + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(100_123_456_789); + let mut bank = Bank::new_for_tests(&genesis_config); + let feature_set = FeatureSet::all_enabled(); + bank.feature_set = Arc::new(feature_set); + let bank = Arc::new(bank); + let mut bank_client = BankClient::new_shared(bank); + + let c_invoke_program_id = + load_program(&bank_client, &bpf_loader::id(), &mint_keypair, "invoke"); + + let (bank, invoke_program_id) = load_program_and_advance_slot( + &mut bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_sbf_rust_invoke", + ); + + let account_keypair = Keypair::new(); + let mint_pubkey = mint_keypair.pubkey(); + let account_metas = vec![ + AccountMeta::new(mint_pubkey, true), + AccountMeta::new(account_keypair.pubkey(), false), + AccountMeta::new_readonly(invoke_program_id, false), + AccountMeta::new_readonly(c_invoke_program_id, false), + ]; + + for invoke_program_id in [invoke_program_id, c_invoke_program_id] { + for ix in [ + TEST_CPI_INVALID_KEY_POINTER, + TEST_CPI_INVALID_LAMPORTS_POINTER, + TEST_CPI_INVALID_OWNER_POINTER, + TEST_CPI_INVALID_DATA_POINTER, + ] { + let account = AccountSharedData::new(42, 5, &invoke_program_id); + bank.store_account(&account_keypair.pubkey(), &account); + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &[ix, 42, 42, 42], + account_metas.clone(), + ); + + let message = Message::new(&[instruction], Some(&mint_pubkey)); + let tx = Transaction::new(&[&mint_keypair], message.clone(), bank.last_blockhash()); + let (result, _, logs) = process_transaction_and_record_inner(&bank, tx); + assert!(result.is_err(), "{result:?}"); + assert!( + logs.iter().any(|log| log.contains("Invalid pointer")), + "{logs:?}" + ); + } + } +} + +#[test] +#[cfg(feature = "sbf_rust")] +fn test_deny_executable_write() { + solana_logger::setup(); + + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(100_123_456_789); + + for direct_mapping in [false, true] { + let mut bank = Bank::new_for_tests(&genesis_config); + let feature_set = Arc::make_mut(&mut bank.feature_set); + // by default test banks have all features enabled, so we only need to + // disable when needed + if !direct_mapping { + feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id()); + } + let bank = Arc::new(bank); + let mut bank_client = BankClient::new_shared(bank); + + let (_bank, invoke_program_id) = load_program_and_advance_slot( + &mut bank_client, + &bpf_loader::id(), + &mint_keypair, + "solana_sbf_rust_invoke", + ); + + let account_keypair = Keypair::new(); + let mint_pubkey = mint_keypair.pubkey(); + let account_metas = vec![ + AccountMeta::new(mint_pubkey, true), + AccountMeta::new(account_keypair.pubkey(), false), + AccountMeta::new_readonly(invoke_program_id, false), + ]; + + let mut instruction_data = vec![TEST_WRITE_ACCOUNT, 2]; + instruction_data.extend_from_slice(4usize.to_le_bytes().as_ref()); + instruction_data.push(42); + let instruction = Instruction::new_with_bytes( + invoke_program_id, + &instruction_data, + account_metas.clone(), + ); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + assert_eq!( + result.unwrap_err().unwrap(), + TransactionError::InstructionError(0, InstructionError::ExecutableDataModified) + ); + } }