Skip to content

Commit

Permalink
v2.1: Ensure that Rc in AccountInfo is not stored in an account (back…
Browse files Browse the repository at this point in the history
…port of #3471) (#3723)

Ensure that Rc in AccountInfo is not stored in an account (#3471)

* Ensure that Rc in AccountInfo is not stored in an account

(cherry picked from commit 030a558)

Co-authored-by: Sean Young <[email protected]>
  • Loading branch information
mergify[bot] and seanyoung authored Dec 4, 2024
1 parent 086cc09 commit 32b445a
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 0 deletions.
8 changes: 8 additions & 0 deletions programs/bpf_loader/src/syscalls/cpi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ impl<'a, 'b> CallerAccount<'a, 'b> {
invoke_context.get_check_aligned(),
)?;
if direct_mapping {
if account_info.lamports.as_ptr() as u64 >= ebpf::MM_INPUT_START {
return Err(SyscallError::InvalidPointer.into());
}

check_account_info_pointer(
invoke_context,
*ptr,
Expand All @@ -154,6 +158,10 @@ impl<'a, 'b> CallerAccount<'a, 'b> {
)?;

let (serialized_data, vm_data_addr, ref_to_len_in_vm) = {
if direct_mapping && account_info.data.as_ptr() as u64 >= ebpf::MM_INPUT_START {
return Err(SyscallError::InvalidPointer.into());
}

// Double translate data out of RefCell
let data = *translate_type::<&[u8]>(
memory_mapping,
Expand Down
72 changes: 72 additions & 0 deletions programs/sbf/rust/invoke/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1522,6 +1522,78 @@ fn process_instruction<'a>(
)
.unwrap();
}
TEST_ACCOUNT_INFO_LAMPORTS_RC => {
msg!("TEST_ACCOUNT_INFO_LAMPORTS_RC_IN_ACCOUNT");

let mut account0 = accounts[0].clone();
let account1 = accounts[1].clone();
let account2 = accounts[2].clone();

account0.lamports = unsafe {
let dst = account1.data.borrow_mut().as_mut_ptr();
// 32 = size_of::<RcBox>()
std::ptr::copy(
std::mem::transmute::<Rc<RefCell<&mut u64>>, *const u8>(account0.lamports),
dst,
32,
);
std::mem::transmute::<*mut u8, Rc<RefCell<&mut u64>>>(dst)
};

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

invoke(
&create_instruction(
*program_id,
&[
(program_id, false, false),
(accounts[1].key, true, false),
(accounts[0].key, false, false),
],
instruction_data.to_vec(),
),
&[account0, account1, account2],
)
.unwrap();
}
TEST_ACCOUNT_INFO_DATA_RC => {
msg!("TEST_ACCOUNT_INFO_DATA_RC_IN_ACCOUNT");

let mut account0 = accounts[0].clone();
let account1 = accounts[1].clone();
let account2 = accounts[2].clone();

account0.data = unsafe {
let dst = account1.data.borrow_mut().as_mut_ptr();
// 32 = size_of::<RcBox>()
std::ptr::copy(
std::mem::transmute::<Rc<RefCell<&mut [u8]>>, *const u8>(account0.data),
dst,
32,
);
std::mem::transmute::<*mut u8, Rc<RefCell<&mut [u8]>>>(dst)
};

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

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

Expand Down
2 changes: 2 additions & 0 deletions programs/sbf/rust/invoke_dep/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ 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 TEST_ACCOUNT_INFO_LAMPORTS_RC: u8 = 44;
pub const TEST_ACCOUNT_INFO_DATA_RC: u8 = 45;

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

#[test]
fn test_account_info_rc_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 instruction_data = vec![TEST_ACCOUNT_INFO_LAMPORTS_RC, 0, 0, 0];

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

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

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

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!(
logs.last().unwrap().ends_with(" failed: Invalid pointer"),
"{logs:?}"
);
assert!(result.is_err());
} else {
assert!(result.is_ok(), "{logs:?}");
}

let instruction_data = vec![TEST_ACCOUNT_INFO_DATA_RC, 0, 0, 0];

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 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!(
logs.last().unwrap().ends_with(" failed: Invalid pointer"),
"{logs:?}"
);
assert!(result.is_err());
} else {
assert!(result.is_ok(), "{logs:?}");
}
}
}

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

0 comments on commit 32b445a

Please sign in to comment.