Skip to content

Commit

Permalink
Ensure that account info address is not in an account
Browse files Browse the repository at this point in the history
  • Loading branch information
seanyoung committed Oct 3, 2024
1 parent 43cc2dd commit e6a5e5e
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 6 deletions.
21 changes: 15 additions & 6 deletions programs/bpf_loader/src/syscalls/cpi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,12 +321,6 @@ impl<'a, 'b> CallerAccount<'a, 'b> {
.saturating_sub(account_info as *const _ as *const u64 as u64);

let ref_to_len_in_vm = if direct_mapping {
// In the same vein as the other check_account_info_pointer() checks, we don't lock this
// pointer to a specific address but we don't want it to be inside accounts, or callees
// might be able to write to the pointed memory.
if data_len_vm_addr >= ebpf::MM_INPUT_START {
return Err(SyscallError::InvalidPointer.into());
}
VmValue::VmAddress {
vm_addr: data_len_vm_addr,
memory_mapping,
Expand Down Expand Up @@ -804,6 +798,21 @@ fn translate_account_infos<'a, T, F>(
where
F: Fn(&T) -> u64,
{
let direct_mapping = invoke_context
.get_feature_set()
.is_active(&feature_set::bpf_account_data_direct_mapping::id());

// In the same vein as the other check_account_info_pointer() checks, we don't lock
// this pointer to a specific address but we don't want it to be inside accounts, or
// callees might be able to write to the pointed memory.
if direct_mapping
&& account_infos_addr
.saturating_add(account_infos_len.saturating_mul(std::mem::size_of::<T>() as u64))
>= ebpf::MM_INPUT_START
{
return Err(SyscallError::InvalidPointer.into());
}

let account_infos = translate_slice::<T>(
memory_mapping,
account_infos_addr,
Expand Down
38 changes: 38 additions & 0 deletions programs/sbf/rust/invoke/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,44 @@ fn process_instruction<'a>(
)
.unwrap();
}
TEST_ACCOUNT_INFO_IN_ACCOUNT => {
msg!("TEST_ACCOUNT_INFO_IN_ACCOUNT");

let account_offset = usize::from_le_bytes(instruction_data[1..9].try_into().unwrap());

let mut instruction_data = vec![TEST_WRITE_ACCOUNT, 1];
instruction_data.extend_from_slice(&1u64.to_le_bytes());
instruction_data.push(1);

let data = accounts[ARGUMENT_INDEX].data.borrow().as_ptr();
let len = accounts.len();

let account_info_in_account: &mut [AccountInfo] = unsafe {
std::slice::from_raw_parts_mut(data.add(account_offset) as *mut AccountInfo, len)
};

unsafe {
std::ptr::copy_nonoverlapping(
accounts.as_ptr(),
account_info_in_account.as_mut_ptr(),
len,
);
}

invoke(
&create_instruction(
*program_id,
&[
(program_id, false, false),
(accounts[1].key, true, false),
(accounts[0].key, false, false),
],
instruction_data.to_vec(),
),
account_info_in_account,
)
.unwrap();
}
_ => panic!("unexpected program data"),
}

Expand Down
1 change: 1 addition & 0 deletions programs/sbf/rust/invoke_dep/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub const TEST_CPI_CHANGE_ACCOUNT_DATA_MEMORY_ALLOCATION: u8 = 39;
pub const TEST_WRITE_ACCOUNT: u8 = 40;
pub const TEST_CALLEE_ACCOUNT_UPDATES: u8 = 41;
pub const TEST_STACK_HEAP_ZEROED: u8 = 42;
pub const TEST_ACCOUNT_INFO_IN_ACCOUNT: u8 = 43;

pub const MINT_INDEX: usize = 0;
pub const ARGUMENT_INDEX: usize = 1;
Expand Down
60 changes: 60 additions & 0 deletions programs/sbf/tests/programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4929,6 +4929,66 @@ fn test_update_callee_account() {
}
}

#[test]
fn test_account_info_in_account() {
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, bank_forks) = bank.wrap_with_bank_forks_for_tests();
let mut bank_client = BankClient::new_shared(bank.clone());
let authority_keypair = Keypair::new();

let (bank, invoke_program_id) = load_upgradeable_program_and_advance_slot(
&mut bank_client,
bank_forks.as_ref(),
&mint_keypair,
&authority_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_ACCOUNT_INFO_IN_ACCOUNT];
instruction_data.extend_from_slice(32usize.to_le_bytes().as_ref());

let instruction =
Instruction::new_with_bytes(invoke_program_id, &instruction_data, account_metas);

let account = AccountSharedData::new(42, 10240, &invoke_program_id);

bank.store_account(&account_keypair.pubkey(), &account);

let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
if direct_mapping {
assert!(result.is_err());
} else {
assert!(result.is_ok());
}
}
}

#[test]
fn test_clone_account_data() {
// Test cloning account data works as expect with
Expand Down

0 comments on commit e6a5e5e

Please sign in to comment.