From 597733d2a8508ec474694af1a792ff51c250e234 Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Tue, 5 Sep 2023 09:04:47 -0400 Subject: [PATCH] Revert "CPI: improve test coverage (#31986)" This reverts commit 6679153ca17f8e96f7e3b57bc4c8e565774f421f. --- programs/sbf/Cargo.lock | 1 - programs/sbf/c/src/invoke/invoke.c | 73 -- .../sbf/rust/deprecated_loader/src/lib.rs | 152 +---- 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, 35 insertions(+), 1487 deletions(-) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 2ab0a3e3d82dae..bce80df4c551ff 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5755,7 +5755,6 @@ 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 1ff4e6b69a096c..4cb038cc7f6950 100644 --- a/programs/sbf/c/src/invoke/invoke.c +++ b/programs/sbf/c/src/invoke/invoke.c @@ -31,12 +31,6 @@ 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; @@ -669,73 +663,6 @@ 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 a9b801b5e43eea..772e0c0f594cd2 100644 --- a/programs/sbf/rust/deprecated_loader/src/lib.rs +++ b/programs/sbf/rust/deprecated_loader/src/lib.rs @@ -5,21 +5,10 @@ extern crate solana_program; use solana_program::{ - account_info::AccountInfo, - bpf_loader, - entrypoint_deprecated::ProgramResult, - instruction::{AccountMeta, Instruction}, - log::*, - msg, - program::invoke, + account_info::AccountInfo, bpf_loader, entrypoint_deprecated::ProgramResult, log::*, msg, 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, @@ -50,122 +39,37 @@ fn process_instruction( assert!(!bpf_loader::check_id(program_id)); - // 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; - - 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() - }; - - 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 - use std methods, unwrap - - // 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 - struct return - - let s = return_sstruct(); - assert_eq!(s.x + s.y + s.z, 6); - } - - { - // Test - arch config - #[cfg(not(target_os = "solana"))] - panic!(); - } - } + // 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 - use std methods, unwrap + + // 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); } - Ok(()) -} + { + // Test - struct return -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, + let s = return_sstruct(); + assert_eq!(s.x + s.y + s.z, 6); } + + { + // Test - arch config + #[cfg(not(target_os = "solana"))] + panic!(); + } + + Ok(()) } #[cfg(test)] diff --git a/programs/sbf/rust/invoke/Cargo.toml b/programs/sbf/rust/invoke/Cargo.toml index 66b07e500d897f..616beab7a49b30 100644 --- a/programs/sbf/rust/invoke/Cargo.toml +++ b/programs/sbf/rust/invoke/Cargo.toml @@ -16,7 +16,6 @@ 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 b335fb52f5b6b1..db8be12dea5619 100644 --- a/programs/sbf/rust/invoke/src/instructions.rs +++ b/programs/sbf/rust/invoke/src/instructions.rs @@ -21,24 +21,6 @@ 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 2e1ee8cac9cc42..dce6492ecefff0 100644 --- a/programs/sbf/rust/invoke/src/processor.rs +++ b/programs/sbf/rust/invoke/src/processor.rs @@ -2,13 +2,11 @@ #![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, @@ -18,11 +16,9 @@ use { syscalls::{ MAX_CPI_ACCOUNT_INFOS, MAX_CPI_INSTRUCTION_ACCOUNTS, MAX_CPI_INSTRUCTION_DATA_LEN, }, - system_instruction, system_program, + system_instruction, }, 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 { @@ -692,614 +688,8 @@ fn process_instruction( invoked_instruction.accounts[1].is_writable = true; invoke(&invoked_instruction, accounts)?; } - 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"), + _ => panic!(), } 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 46a4f1d9f2f45c..9f98da7d92aa52 100644 --- a/programs/sbf/rust/invoked/src/instructions.rs +++ b/programs/sbf/rust/invoked/src/instructions.rs @@ -19,7 +19,6 @@ 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 52d02dc99a6c21..73bf25cac79bfc 100644 --- a/programs/sbf/rust/invoked/src/processor.rs +++ b/programs/sbf/rust/invoked/src/processor.rs @@ -297,14 +297,6 @@ 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 e15ba5d48c5e21..831a69029ed166 100644 --- a/programs/sbf/rust/realloc/src/instructions.rs +++ b/programs/sbf/rust/realloc/src/instructions.rs @@ -16,7 +16,6 @@ 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 172ed7498f88c8..782eb1cd505519 100644 --- a/programs/sbf/rust/realloc/src/processor.rs +++ b/programs/sbf/rust/realloc/src/processor.rs @@ -1,7 +1,6 @@ //! Example Rust-based SBF realloc test program #![cfg(feature = "program")] -#![allow(clippy::integer_arithmetic)] extern crate solana_program; use { @@ -185,13 +184,6 @@ 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 b690ea2ffef434..6acb18e4793c6e 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(), "{result:?}"); + assert!(result.is_ok()); } else { - assert!(result.is_err(), "{result:?}"); + assert!(result.is_err()); } } } @@ -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, &[255], account_metas); + let instruction = Instruction::new_with_bytes(program_id, &[1], 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, - &[255], + &[1], vec![AccountMeta::new(mint_keypair.pubkey(), true)], )], Some(&mint_keypair.pubkey()), @@ -3939,640 +3939,5 @@ 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(), "{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) - ); - } + assert!(result.is_ok()); }