Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2.1: Ensure that Rc in AccountInfo is not stored in an account (backport of #3471) #3723

Merged
merged 1 commit into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading