gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Always bail if program modifies a ro account (#17569)

Browse Source
This commit is contained in:
Jack May
2021-05-28 09:50:25 -07:00
committed by GitHub
parent 8924fbf6a0
commit a3240aebde
13 changed files with 248 additions and 151 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
programs/bpf_loader/src/lib.rs
View File

@ -32,7 +32,7 @@ use solana_sdk::{
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
clock::Clock,
entrypoint::SUCCESS,
feature_set::{skip_ro_deserialization, upgradeable_close_instruction},
feature_set::upgradeable_close_instruction,
ic_logger_msg, ic_msg,
instruction::InstructionError,
keyed_account::{from_keyed_account, keyed_account_at_index},
@ -827,12 +827,7 @@ impl Executor for BpfExecutor {
}
let mut deserialize_time = Measure::start("deserialize");
let keyed_accounts = invoke_context.get_keyed_accounts()?;
deserialize_parameters(
loader_id,
keyed_accounts,
parameter_bytes.as_slice(),
invoke_context.is_feature_active(&skip_ro_deserialization::id()),
)?;
deserialize_parameters(loader_id, keyed_accounts, parameter_bytes.as_slice())?;
deserialize_time.stop();
invoke_context.update_timing(
serialize_time.as_us(),

55
programs/bpf_loader/src/serialization.rs
View File

@ -40,12 +40,11 @@ pub fn deserialize_parameters(
loader_id: &Pubkey,
keyed_accounts: &[KeyedAccount],
buffer: &[u8],
skip_ro_deserialization: bool,
) -> Result<(), InstructionError> {
if *loader_id == bpf_loader_deprecated::id() {
deserialize_parameters_unaligned(keyed_accounts, buffer, skip_ro_deserialization)
deserialize_parameters_unaligned(keyed_accounts, buffer)
} else {
deserialize_parameters_aligned(keyed_accounts, buffer, skip_ro_deserialization)
deserialize_parameters_aligned(keyed_accounts, buffer)
}
}
@ -127,33 +126,28 @@ pub fn serialize_parameters_unaligned(
pub fn deserialize_parameters_unaligned(
keyed_accounts: &[KeyedAccount],
buffer: &[u8],
skip_ro_deserialization: bool,
) -> Result<(), InstructionError> {
let mut start = size_of::<u64>(); // number of accounts
for (i, keyed_account) in keyed_accounts.iter().enumerate() {
let (is_dup, _) = is_dup(&keyed_accounts[..i], keyed_account);
start += 1; // is_dup
if !is_dup {
if keyed_account.is_writable() || !skip_ro_deserialization {
start += size_of::<u8>(); // is_signer
start += size_of::<u8>(); // is_writable
start += size_of::<Pubkey>(); // key
keyed_account
.try_account_ref_mut()?
.set_lamports(LittleEndian::read_u64(&buffer[start..]));
start += size_of::<u64>() // lamports
start += size_of::<u8>(); // is_signer
start += size_of::<u8>(); // is_writable
start += size_of::<Pubkey>(); // key
keyed_account
.try_account_ref_mut()?
.set_lamports(LittleEndian::read_u64(&buffer[start..]));
start += size_of::<u64>() // lamports
+ size_of::<u64>(); // data length
let end = start + keyed_account.data_len()?;
keyed_account
.try_account_ref_mut()?
.set_data_from_slice(&buffer[start..end]);
start += keyed_account.data_len()? // data
let end = start + keyed_account.data_len()?;
keyed_account
.try_account_ref_mut()?
.set_data_from_slice(&buffer[start..end]);
start += keyed_account.data_len()? // data
+ size_of::<Pubkey>() // owner
+ size_of::<u8>() // executable
+ size_of::<u64>(); // rent_epoch
} else {
start += get_serialized_account_size_unaligned(keyed_account)?;
}
}
}
Ok(())
@ -253,7 +247,6 @@ pub fn serialize_parameters_aligned(
pub fn deserialize_parameters_aligned(
keyed_accounts: &[KeyedAccount],
buffer: &[u8],
skip_ro_deserialization: bool,
) -> Result<(), InstructionError> {
let mut start = size_of::<u64>(); // number of accounts
for (i, keyed_account) in keyed_accounts.iter().enumerate() {
@ -261,7 +254,7 @@ pub fn deserialize_parameters_aligned(
start += size_of::<u8>(); // position
if is_dup {
start += 7; // padding to 64-bit aligned
} else if keyed_account.is_writable() || !skip_ro_deserialization {
} else {
let mut account = keyed_account.try_account_ref_mut()?;
start += size_of::<u8>() // is_signer
+ size_of::<u8>() // is_writable
@ -286,8 +279,6 @@ pub fn deserialize_parameters_aligned(
start += pre_len + MAX_PERMITTED_DATA_INCREASE; // data
start += (start as *const u8).align_offset(align_of::<u128>());
start += size_of::<u64>(); // rent_epoch
} else {
start += get_serialized_account_size_aligned(keyed_account)?;
}
}
Ok(())
@ -454,24 +445,13 @@ mod tests {
}
})
.collect();
deserialize_parameters(
&bpf_loader::id(),
&de_keyed_accounts,
serialized.as_slice(),
true,
)
.unwrap();
deserialize_parameters(&bpf_loader::id(), &de_keyed_accounts, serialized.as_slice())
.unwrap();
for ((account, de_keyed_account), key) in
accounts.iter().zip(de_keyed_accounts).zip(keys.clone())
{
assert_eq!(key, *de_keyed_account.unsigned_key());
let account = account.borrow();
assert_eq!(account.lamports(), de_keyed_account.lamports().unwrap());
assert_eq!(
account.data(),
de_keyed_account.try_account_ref().unwrap().data()
);
assert_eq!(*account.owner(), de_keyed_account.owner().unwrap());
assert_eq!(account.executable(), de_keyed_account.executable().unwrap());
assert_eq!(account.rent_epoch(), de_keyed_account.rent_epoch().unwrap());
}
@ -517,7 +497,6 @@ mod tests {
&bpf_loader_deprecated::id(),
&de_keyed_accounts,
serialized.as_slice(),
true,
)
.unwrap();
for ((account, de_keyed_account), key) in

19
programs/bpf_loader/src/syscalls.rs
View File

@ -19,9 +19,9 @@ use solana_sdk::{
entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
epoch_schedule::EpochSchedule,
feature_set::{
cpi_data_cost, cpi_share_ro_and_exec_accounts, demote_sysvar_write_locks,
enforce_aligned_host_addrs, keccak256_syscall_enabled,
set_upgrade_authority_via_cpi_enabled, sysvar_via_syscall, update_data_on_realloc,
cpi_data_cost, demote_sysvar_write_locks, enforce_aligned_host_addrs,
keccak256_syscall_enabled, set_upgrade_authority_via_cpi_enabled, sysvar_via_syscall,
update_data_on_realloc,
},
hash::{Hasher, HASH_BYTES},
ic_msg,
@ -1169,7 +1169,6 @@ trait SyscallInvokeSigned<'a> {
fn translate_accounts(
&self,
account_keys: &[Pubkey],
caller_write_privileges: &[bool],
program_account_index: usize,
account_infos_addr: u64,
account_infos_len: u64,
@ -1246,7 +1245,6 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> {
fn translate_accounts(
&self,
account_keys: &[Pubkey],
caller_write_privileges: &[bool],
program_account_index: usize,
account_infos_addr: u64,
account_infos_len: u64,
@ -1368,7 +1366,6 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> {
get_translated_accounts(
account_keys,
caller_write_privileges,
program_account_index,
&account_info_keys,
account_infos,
@ -1585,7 +1582,6 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> {
fn translate_accounts(
&self,
account_keys: &[Pubkey],
caller_write_privileges: &[bool],
program_account_index: usize,
account_infos_addr: u64,
account_infos_len: u64,
@ -1689,7 +1685,6 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> {
get_translated_accounts(
account_keys,
caller_write_privileges,
program_account_index,
&account_info_keys,
account_infos,
@ -1779,7 +1774,6 @@ impl<'a> SyscallObject<BpfError> for SyscallInvokeSignedC<'a> {
fn get_translated_accounts<'a, T, F>(
account_keys: &[Pubkey],
caller_write_privileges: &[bool],
program_account_index: usize,
account_info_keys: &[&Pubkey],
account_infos: &[T],
@ -1801,11 +1795,7 @@ where
SyscallError::InstructionError(InstructionError::MissingAccount)
})?;
if i == program_account_index
|| account.borrow().executable()
|| (invoke_context.is_feature_active(&cpi_share_ro_and_exec_accounts::id())
&& !caller_write_privileges[i])
{
if i == program_account_index || account.borrow().executable() {
// Use the known account
accounts.push(account);
refs.push(None);
@ -2000,7 +1990,6 @@ fn call<'a>(
check_authorized_program(&callee_program_id, &instruction.data, &invoke_context)?;
let (accounts, account_refs) = syscall.translate_accounts(
&message.account_keys,
&caller_write_privileges,
callee_program_id_index,
account_infos_addr,
account_infos_len,