From 2a48864a673e2dd2efcd048e2e9aec0777dde111 Mon Sep 17 00:00:00 2001 From: Jack May Date: Mon, 13 Sep 2021 22:57:37 -0700 Subject: [PATCH 1/2] Fix native invoke writable privileges (#19750) * Fix native invoke writable privileges * build downstream spl bpf programs for tests (cherry picked from commit 00d7981f644dc5f2a102d73716551b804ca1c57f) # Conflicts: # program-runtime/src/instruction_processor.rs # runtime/src/message_processor.rs # sdk/src/feature_set.rs --- program-runtime/src/instruction_processor.rs | 1214 ++++++++++++++++++ runtime/src/message_processor.rs | 250 +++- sdk/src/feature_set.rs | 16 + 3 files changed, 1475 insertions(+), 5 deletions(-) create mode 100644 program-runtime/src/instruction_processor.rs diff --git a/program-runtime/src/instruction_processor.rs b/program-runtime/src/instruction_processor.rs new file mode 100644 index 00000000000..26f62f93a8a --- /dev/null +++ b/program-runtime/src/instruction_processor.rs @@ -0,0 +1,1214 @@ +use crate::native_loader::NativeLoader; +use serde::{Deserialize, Serialize}; +use solana_sdk::{ + account::{AccountSharedData, ReadableAccount, WritableAccount}, + account_utils::StateMut, + bpf_loader_upgradeable::{self, UpgradeableLoaderState}, + feature_set::{demote_program_write_locks, fix_write_privs}, + ic_logger_msg, ic_msg, + instruction::{CompiledInstruction, Instruction, InstructionError}, + keyed_account::{keyed_account_at_index, KeyedAccount}, + message::Message, + process_instruction::{Executor, InvokeContext, Logger, ProcessInstructionWithContext}, + pubkey::Pubkey, + rent::Rent, + system_program, +}; +use std::{ + cell::{Ref, RefCell}, + collections::HashMap, + rc::Rc, + sync::Arc, +}; + +pub struct Executors { + pub executors: HashMap>, + pub is_dirty: bool, +} +impl Default for Executors { + fn default() -> Self { + Self { + executors: HashMap::default(), + is_dirty: false, + } + } +} +impl Executors { + pub fn insert(&mut self, key: Pubkey, executor: Arc) { + let _ = self.executors.insert(key, executor); + self.is_dirty = true; + } + pub fn get(&self, key: &Pubkey) -> Option> { + self.executors.get(key).cloned() + } +} + +#[derive(Default, Debug)] +pub struct ProgramTiming { + pub accumulated_us: u64, + pub accumulated_units: u64, + pub count: u32, +} + +#[derive(Default, Debug)] +pub struct ExecuteDetailsTimings { + pub serialize_us: u64, + pub create_vm_us: u64, + pub execute_us: u64, + pub deserialize_us: u64, + pub changed_account_count: u64, + pub total_account_count: u64, + pub total_data_size: usize, + pub data_size_changed: usize, + pub per_program_timings: HashMap, +} +impl ExecuteDetailsTimings { + pub fn accumulate(&mut self, other: &ExecuteDetailsTimings) { + self.serialize_us += other.serialize_us; + self.create_vm_us += other.create_vm_us; + self.execute_us += other.execute_us; + self.deserialize_us += other.deserialize_us; + self.changed_account_count += other.changed_account_count; + self.total_account_count += other.total_account_count; + self.total_data_size += other.total_data_size; + self.data_size_changed += other.data_size_changed; + for (id, other) in &other.per_program_timings { + let program_timing = self.per_program_timings.entry(*id).or_default(); + program_timing.accumulated_us = program_timing + .accumulated_us + .saturating_add(other.accumulated_us); + program_timing.accumulated_units = program_timing + .accumulated_units + .saturating_add(other.accumulated_units); + program_timing.count = program_timing.count.saturating_add(other.count); + } + } + pub fn accumulate_program(&mut self, program_id: &Pubkey, us: u64, units: u64) { + let program_timing = self.per_program_timings.entry(*program_id).or_default(); + program_timing.accumulated_us = program_timing.accumulated_us.saturating_add(us); + program_timing.accumulated_units = program_timing.accumulated_units.saturating_add(units); + program_timing.count = program_timing.count.saturating_add(1); + } +} + +// The relevant state of an account before an Instruction executes, used +// to verify account integrity after the Instruction completes +#[derive(Clone, Debug, Default)] +pub struct PreAccount { + key: Pubkey, + account: Rc>, + changed: bool, +} +impl PreAccount { + pub fn new(key: &Pubkey, account: &AccountSharedData) -> Self { + Self { + key: *key, + account: Rc::new(RefCell::new(account.clone())), + changed: false, + } + } + + pub fn verify( + &self, + program_id: &Pubkey, + is_writable: bool, + rent: &Rent, + post: &AccountSharedData, + timings: &mut ExecuteDetailsTimings, + outermost_call: bool, + updated_verify_policy: bool, + ) -> Result<(), InstructionError> { + let pre = self.account.borrow(); + + // Only the owner of the account may change owner and + // only if the account is writable and + // only if the account is not executable and + // only if the data is zero-initialized or empty + let owner_changed = pre.owner() != post.owner(); + if owner_changed + && (!is_writable // line coverage used to get branch coverage + || pre.executable() + || program_id != pre.owner() + || !Self::is_zeroed(post.data())) + { + return Err(InstructionError::ModifiedProgramId); + } + + // An account not assigned to the program cannot have its balance decrease. + if program_id != pre.owner() // line coverage used to get branch coverage + && pre.lamports() > post.lamports() + { + return Err(InstructionError::ExternalAccountLamportSpend); + } + + // The balance of read-only and executable accounts may not change + let lamports_changed = pre.lamports() != post.lamports(); + if lamports_changed { + if !is_writable { + return Err(InstructionError::ReadonlyLamportChange); + } + if pre.executable() { + return Err(InstructionError::ExecutableLamportChange); + } + } + + // Only the system program can change the size of the data + // and only if the system program owns the account + let data_len_changed = pre.data().len() != post.data().len(); + if data_len_changed + && (!system_program::check_id(program_id) // line coverage used to get branch coverage + || !system_program::check_id(pre.owner())) + { + return Err(InstructionError::AccountDataSizeChanged); + } + + // Only the owner may change account data + // and if the account is writable + // and if the account is not executable + if !(program_id == pre.owner() + && is_writable // line coverage used to get branch coverage + && !pre.executable()) + && pre.data() != post.data() + { + if pre.executable() { + return Err(InstructionError::ExecutableDataModified); + } else if is_writable { + return Err(InstructionError::ExternalAccountDataModified); + } else { + return Err(InstructionError::ReadonlyDataModified); + } + } + + // executable is one-way (false->true) and only the account owner may set it. + let executable_changed = pre.executable() != post.executable(); + if executable_changed { + if !rent.is_exempt(post.lamports(), post.data().len()) { + return Err(InstructionError::ExecutableAccountNotRentExempt); + } + let owner = if updated_verify_policy { + post.owner() + } else { + pre.owner() + }; + if !is_writable // line coverage used to get branch coverage + || pre.executable() + || program_id != owner + { + return Err(InstructionError::ExecutableModified); + } + } + + // No one modifies rent_epoch (yet). + let rent_epoch_changed = pre.rent_epoch() != post.rent_epoch(); + if rent_epoch_changed { + return Err(InstructionError::RentEpochModified); + } + + if outermost_call { + timings.total_account_count += 1; + timings.total_data_size += post.data().len(); + if owner_changed + || lamports_changed + || data_len_changed + || executable_changed + || rent_epoch_changed + || self.changed + { + timings.changed_account_count += 1; + timings.data_size_changed += post.data().len(); + } + } + + Ok(()) + } + + pub fn update(&mut self, account: &AccountSharedData) { + let mut pre = self.account.borrow_mut(); + let rent_epoch = pre.rent_epoch(); + *pre = account.clone(); + pre.set_rent_epoch(rent_epoch); + + self.changed = true; + } + + pub fn key(&self) -> &Pubkey { + &self.key + } + + pub fn data(&self) -> Ref<[u8]> { + Ref::map(self.account.borrow(), |account| account.data()) + } + + pub fn lamports(&self) -> u64 { + self.account.borrow().lamports() + } + + pub fn executable(&self) -> bool { + self.account.borrow().executable() + } + + pub fn is_zeroed(buf: &[u8]) -> bool { + const ZEROS_LEN: usize = 1024; + static ZEROS: [u8; ZEROS_LEN] = [0; ZEROS_LEN]; + let mut chunks = buf.chunks_exact(ZEROS_LEN); + + chunks.all(|chunk| chunk == &ZEROS[..]) + && chunks.remainder() == &ZEROS[..chunks.remainder().len()] + } +} + +#[derive(Deserialize, Serialize)] +pub struct InstructionProcessor { + #[serde(skip)] + programs: Vec<(Pubkey, ProcessInstructionWithContext)>, + #[serde(skip)] + native_loader: NativeLoader, +} + +impl std::fmt::Debug for InstructionProcessor { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + #[derive(Debug)] + struct MessageProcessor<'a> { + programs: Vec, + native_loader: &'a NativeLoader, + } + + // These are just type aliases for work around of Debug-ing above pointers + type ErasedProcessInstructionWithContext = fn( + &'static Pubkey, + &'static [u8], + &'static mut dyn InvokeContext, + ) -> Result<(), InstructionError>; + + // rustc doesn't compile due to bug without this work around + // https://github.com/rust-lang/rust/issues/50280 + // https://users.rust-lang.org/t/display-function-pointer/17073/2 + let processor = MessageProcessor { + programs: self + .programs + .iter() + .map(|(pubkey, instruction)| { + let erased_instruction: ErasedProcessInstructionWithContext = *instruction; + format!("{}: {:p}", pubkey, erased_instruction) + }) + .collect::>(), + native_loader: &self.native_loader, + }; + + write!(f, "{:?}", processor) + } +} + +impl Default for InstructionProcessor { + fn default() -> Self { + Self { + programs: vec![], + native_loader: NativeLoader::default(), + } + } +} +impl Clone for InstructionProcessor { + fn clone(&self) -> Self { + InstructionProcessor { + programs: self.programs.clone(), + native_loader: NativeLoader::default(), + } + } +} + +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl ::solana_frozen_abi::abi_example::AbiExample for InstructionProcessor { + fn example() -> Self { + // MessageProcessor's fields are #[serde(skip)]-ed and not Serialize + // so, just rely on Default anyway. + InstructionProcessor::default() + } +} + +impl InstructionProcessor { + pub fn programs(&self) -> &[(Pubkey, ProcessInstructionWithContext)] { + &self.programs + } + + /// Add a static entrypoint to intercept instructions before the dynamic loader. + pub fn add_program( + &mut self, + program_id: Pubkey, + process_instruction: ProcessInstructionWithContext, + ) { + match self.programs.iter_mut().find(|(key, _)| program_id == *key) { + Some((_, processor)) => *processor = process_instruction, + None => self.programs.push((program_id, process_instruction)), + } + } + + /// Process an instruction + /// This method calls the instruction's program entrypoint method + pub fn process_instruction( + &self, + program_id: &Pubkey, + instruction_data: &[u8], + invoke_context: &mut dyn InvokeContext, + ) -> Result<(), InstructionError> { + if let Some(root_account) = invoke_context.get_keyed_accounts()?.iter().next() { + let root_id = root_account.unsigned_key(); + if solana_sdk::native_loader::check_id(&root_account.owner()?) { + for (id, process_instruction) in &self.programs { + if id == root_id { + invoke_context.remove_first_keyed_account()?; + // Call the builtin program + return process_instruction(program_id, instruction_data, invoke_context); + } + } + // Call the program via the native loader + return self.native_loader.process_instruction( + &solana_sdk::native_loader::id(), + instruction_data, + invoke_context, + ); + } else { + let owner_id = &root_account.owner()?; + for (id, process_instruction) in &self.programs { + if id == owner_id { + // Call the program via a builtin loader + return process_instruction(program_id, instruction_data, invoke_context); + } + } + } + } + Err(InstructionError::UnsupportedProgramId) + } + + pub fn create_message( + instruction: &Instruction, + keyed_accounts: &[&KeyedAccount], + signers: &[Pubkey], + invoke_context: &Ref<&mut dyn InvokeContext>, + ) -> Result<(Message, Pubkey, usize), InstructionError> { + // Check for privilege escalation + for account in instruction.accounts.iter() { + let keyed_account = keyed_accounts + .iter() + .find_map(|keyed_account| { + if &account.pubkey == keyed_account.unsigned_key() { + Some(keyed_account) + } else { + None + } + }) + .ok_or_else(|| { + ic_msg!( + invoke_context, + "Instruction references an unknown account {}", + account.pubkey + ); + InstructionError::MissingAccount + })?; + // Readonly account cannot become writable + if account.is_writable && !keyed_account.is_writable() { + ic_msg!( + invoke_context, + "{}'s writable privilege escalated", + account.pubkey + ); + return Err(InstructionError::PrivilegeEscalation); + } + + if account.is_signer && // If message indicates account is signed + !( // one of the following needs to be true: + keyed_account.signer_key().is_some() // Signed in the parent instruction + || signers.contains(&account.pubkey) // Signed by the program + ) { + ic_msg!( + invoke_context, + "{}'s signer privilege escalated", + account.pubkey + ); + return Err(InstructionError::PrivilegeEscalation); + } + } + + // validate the caller has access to the program account and that it is executable + let program_id = instruction.program_id; + match keyed_accounts + .iter() + .find(|keyed_account| &program_id == keyed_account.unsigned_key()) + { + Some(keyed_account) => { + if !keyed_account.executable()? { + ic_msg!( + invoke_context, + "Account {} is not executable", + keyed_account.unsigned_key() + ); + return Err(InstructionError::AccountNotExecutable); + } + } + None => { + ic_msg!(invoke_context, "Unknown program {}", program_id); + return Err(InstructionError::MissingAccount); + } + } + + let message = Message::new(&[instruction.clone()], None); + let program_id_index = message.instructions[0].program_id_index as usize; + + Ok((message, program_id, program_id_index)) + } + + /// Entrypoint for a cross-program invocation from a native program + pub fn native_invoke( + invoke_context: &mut dyn InvokeContext, + instruction: Instruction, + keyed_account_indices: &[usize], + signers: &[Pubkey], + ) -> Result<(), InstructionError> { + let invoke_context = RefCell::new(invoke_context); + + let ( + message, + program_indices, + accounts, + keyed_account_indices_reordered, + caller_write_privileges, + ) = { + let invoke_context = invoke_context.borrow(); + + let caller_keyed_accounts = invoke_context.get_keyed_accounts()?; + let callee_keyed_accounts = keyed_account_indices + .iter() + .map(|index| keyed_account_at_index(caller_keyed_accounts, *index)) + .collect::, InstructionError>>()?; + let (message, callee_program_id, _) = Self::create_message( + &instruction, + &callee_keyed_accounts, + signers, + &invoke_context, + )?; + let mut keyed_account_indices_reordered = + Vec::with_capacity(message.account_keys.len()); + let mut accounts = Vec::with_capacity(message.account_keys.len()); + let mut caller_write_privileges = Vec::with_capacity(message.account_keys.len()); + + // Translate and verify caller's data + if invoke_context.is_feature_active(&fix_write_privs::id()) { + 'root: for account_key in message.account_keys.iter() { + for keyed_account_index in keyed_account_indices { + let keyed_account = &caller_keyed_accounts[*keyed_account_index]; + if account_key == keyed_account.unsigned_key() { + accounts.push((*account_key, Rc::new(keyed_account.account.clone()))); + caller_write_privileges.push(keyed_account.is_writable()); + keyed_account_indices_reordered.push(*keyed_account_index); + continue 'root; + } + } + ic_msg!( + invoke_context, + "Instruction references an unknown account {}", + account_key + ); + return Err(InstructionError::MissingAccount); + } + } else { + let keyed_accounts = invoke_context.get_keyed_accounts()?; + for index in keyed_account_indices.iter() { + caller_write_privileges.push(keyed_accounts[*index].is_writable()); + } + caller_write_privileges.insert(0, false); + let keyed_accounts = invoke_context.get_keyed_accounts()?; + 'root2: for account_key in message.account_keys.iter() { + for keyed_account_index in keyed_account_indices { + let keyed_account = &keyed_accounts[*keyed_account_index]; + if account_key == keyed_account.unsigned_key() { + accounts.push((*account_key, Rc::new(keyed_account.account.clone()))); + keyed_account_indices_reordered.push(*keyed_account_index); + continue 'root2; + } + } + ic_msg!( + invoke_context, + "Instruction references an unknown account {}", + account_key + ); + return Err(InstructionError::MissingAccount); + } + } + + // Process instruction + + invoke_context.record_instruction(&instruction); + + let (program_account_index, program_account) = invoke_context + .get_account(&callee_program_id) + .ok_or_else(|| { + ic_msg!(invoke_context, "Unknown program {}", callee_program_id); + InstructionError::MissingAccount + })?; + if !program_account.borrow().executable() { + ic_msg!( + invoke_context, + "Account {} is not executable", + callee_program_id + ); + return Err(InstructionError::AccountNotExecutable); + } + let mut program_indices = vec![]; + if program_account.borrow().owner() == &bpf_loader_upgradeable::id() { + if let UpgradeableLoaderState::Program { + programdata_address, + } = program_account.borrow().state()? + { + if let Some((programdata_account_index, _programdata_account)) = + invoke_context.get_account(&programdata_address) + { + program_indices.push(programdata_account_index); + } else { + ic_msg!( + invoke_context, + "Unknown upgradeable programdata account {}", + programdata_address, + ); + return Err(InstructionError::MissingAccount); + } + } else { + ic_msg!( + invoke_context, + "Upgradeable program account state not valid {}", + callee_program_id, + ); + return Err(InstructionError::MissingAccount); + } + } + program_indices.insert(0, program_account_index); + ( + message, + program_indices, + accounts, + keyed_account_indices_reordered, + caller_write_privileges, + ) + }; + + #[allow(clippy::deref_addrof)] + InstructionProcessor::process_cross_program_instruction( + &message, + &program_indices, + &accounts, + &caller_write_privileges, + *(&mut *(invoke_context.borrow_mut())), + )?; + + // Copy results back to caller + + { + let invoke_context = invoke_context.borrow(); + let demote_program_write_locks = + invoke_context.is_feature_active(&demote_program_write_locks::id()); + let keyed_accounts = invoke_context.get_keyed_accounts()?; + for (src_keyed_account_index, ((_key, account), dst_keyed_account_index)) in accounts + .iter() + .zip(keyed_account_indices_reordered) + .enumerate() + { + let dst_keyed_account = &keyed_accounts[dst_keyed_account_index]; + let src_keyed_account = account.borrow(); + if message.is_writable(src_keyed_account_index, demote_program_write_locks) + && !src_keyed_account.executable() + { + if dst_keyed_account.data_len()? != src_keyed_account.data().len() + && dst_keyed_account.data_len()? != 0 + { + // Only support for `CreateAccount` at this time. + // Need a way to limit total realloc size across multiple CPI calls + ic_msg!( + invoke_context, + "Inner instructions do not support realloc, only SystemProgram::CreateAccount", + ); + return Err(InstructionError::InvalidRealloc); + } + dst_keyed_account + .try_account_ref_mut()? + .set_lamports(src_keyed_account.lamports()); + dst_keyed_account + .try_account_ref_mut()? + .set_owner(*src_keyed_account.owner()); + dst_keyed_account + .try_account_ref_mut()? + .set_data(src_keyed_account.data().to_vec()); + } + } + } + + Ok(()) + } + + /// Process a cross-program instruction + /// This method calls the instruction's program entrypoint function + pub fn process_cross_program_instruction( + message: &Message, + program_indices: &[usize], + accounts: &[(Pubkey, Rc>)], + caller_write_privileges: &[bool], + invoke_context: &mut dyn InvokeContext, + ) -> Result<(), InstructionError> { + if let Some(instruction) = message.instructions.get(0) { + let program_id = instruction.program_id(&message.account_keys); + + // Verify the calling program hasn't misbehaved + invoke_context.verify_and_update(instruction, accounts, caller_write_privileges)?; + + // clear the return data + invoke_context.set_return_data(None); + + // Invoke callee + invoke_context.push(program_id, message, instruction, program_indices, accounts)?; + + let mut instruction_processor = InstructionProcessor::default(); + for (program_id, process_instruction) in invoke_context.get_programs().iter() { + instruction_processor.add_program(*program_id, *process_instruction); + } + + let mut result = instruction_processor.process_instruction( + program_id, + &instruction.data, + invoke_context, + ); + if result.is_ok() { + // Verify the called program has not misbehaved + let demote_program_write_locks = + invoke_context.is_feature_active(&demote_program_write_locks::id()); + let write_privileges: Vec = (0..message.account_keys.len()) + .map(|i| message.is_writable(i, demote_program_write_locks)) + .collect(); + result = invoke_context.verify_and_update(instruction, accounts, &write_privileges); + } + + // Restore previous state + invoke_context.pop(); + result + } else { + // This function is always called with a valid instruction, if that changes return an error + Err(InstructionError::GenericError) + } + } + + /// Verify the results of a cross-program instruction + #[allow(clippy::too_many_arguments)] + pub fn verify_and_update( + instruction: &CompiledInstruction, + pre_accounts: &mut [PreAccount], + accounts: &[(Pubkey, Rc>)], + program_id: &Pubkey, + rent: &Rent, + write_privileges: &[bool], + timings: &mut ExecuteDetailsTimings, + logger: Rc>, + updated_verify_policy: bool, + ) -> Result<(), InstructionError> { + // Verify the per-account instruction results + let (mut pre_sum, mut post_sum) = (0_u128, 0_u128); + let mut work = |_unique_index: usize, account_index: usize| { + if account_index < write_privileges.len() && account_index < accounts.len() { + let (key, account) = &accounts[account_index]; + let is_writable = write_privileges[account_index]; + // Find the matching PreAccount + for pre_account in pre_accounts.iter_mut() { + if key == pre_account.key() { + { + // Verify account has no outstanding references + let _ = account + .try_borrow_mut() + .map_err(|_| InstructionError::AccountBorrowOutstanding)?; + } + let account = account.borrow(); + pre_account + .verify( + program_id, + is_writable, + rent, + &account, + timings, + false, + updated_verify_policy, + ) + .map_err(|err| { + ic_logger_msg!(logger, "failed to verify account {}: {}", key, err); + err + })?; + pre_sum += u128::from(pre_account.lamports()); + post_sum += u128::from(account.lamports()); + if is_writable && !pre_account.executable() { + pre_account.update(&account); + } + return Ok(()); + } + } + } + Err(InstructionError::MissingAccount) + }; + instruction.visit_each_account(&mut work)?; + + // Verify that the total sum of all the lamports did not change + if pre_sum != post_sum { + return Err(InstructionError::UnbalancedInstruction); + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use solana_sdk::{account::Account, instruction::InstructionError}; + + #[test] + fn test_is_zeroed() { + const ZEROS_LEN: usize = 1024; + let mut buf = [0; ZEROS_LEN]; + assert!(PreAccount::is_zeroed(&buf)); + buf[0] = 1; + assert!(!PreAccount::is_zeroed(&buf)); + + let mut buf = [0; ZEROS_LEN - 1]; + assert!(PreAccount::is_zeroed(&buf)); + buf[0] = 1; + assert!(!PreAccount::is_zeroed(&buf)); + + let mut buf = [0; ZEROS_LEN + 1]; + assert!(PreAccount::is_zeroed(&buf)); + buf[0] = 1; + assert!(!PreAccount::is_zeroed(&buf)); + + let buf = vec![]; + assert!(PreAccount::is_zeroed(&buf)); + } + + struct Change { + program_id: Pubkey, + is_writable: bool, + rent: Rent, + pre: PreAccount, + post: AccountSharedData, + } + impl Change { + pub fn new(owner: &Pubkey, program_id: &Pubkey) -> Self { + Self { + program_id: *program_id, + rent: Rent::default(), + is_writable: true, + pre: PreAccount::new( + &solana_sdk::pubkey::new_rand(), + &AccountSharedData::from(Account { + owner: *owner, + lamports: std::u64::MAX, + data: vec![], + ..Account::default() + }), + ), + post: AccountSharedData::from(Account { + owner: *owner, + lamports: std::u64::MAX, + ..Account::default() + }), + } + } + pub fn read_only(mut self) -> Self { + self.is_writable = false; + self + } + pub fn executable(mut self, pre: bool, post: bool) -> Self { + self.pre.account.borrow_mut().set_executable(pre); + self.post.set_executable(post); + self + } + pub fn lamports(mut self, pre: u64, post: u64) -> Self { + self.pre.account.borrow_mut().set_lamports(pre); + self.post.set_lamports(post); + self + } + pub fn owner(mut self, post: &Pubkey) -> Self { + self.post.set_owner(*post); + self + } + pub fn data(mut self, pre: Vec, post: Vec) -> Self { + self.pre.account.borrow_mut().set_data(pre); + self.post.set_data(post); + self + } + pub fn rent_epoch(mut self, pre: u64, post: u64) -> Self { + self.pre.account.borrow_mut().set_rent_epoch(pre); + self.post.set_rent_epoch(post); + self + } + pub fn verify(&self) -> Result<(), InstructionError> { + self.pre.verify( + &self.program_id, + self.is_writable, + &self.rent, + &self.post, + &mut ExecuteDetailsTimings::default(), + false, + true, + ) + } + } + + #[test] + fn test_verify_account_changes_owner() { + let system_program_id = system_program::id(); + let alice_program_id = solana_sdk::pubkey::new_rand(); + let mallory_program_id = solana_sdk::pubkey::new_rand(); + + assert_eq!( + Change::new(&system_program_id, &system_program_id) + .owner(&alice_program_id) + .verify(), + Ok(()), + "system program should be able to change the account owner" + ); + assert_eq!( + Change::new(&system_program_id, &system_program_id) + .owner(&alice_program_id) + .read_only() + .verify(), + Err(InstructionError::ModifiedProgramId), + "system program should not be able to change the account owner of a read-only account" + ); + assert_eq!( + Change::new(&mallory_program_id, &system_program_id) + .owner(&alice_program_id) + .verify(), + Err(InstructionError::ModifiedProgramId), + "system program should not be able to change the account owner of a non-system account" + ); + assert_eq!( + Change::new(&mallory_program_id, &mallory_program_id) + .owner(&alice_program_id) + .verify(), + Ok(()), + "mallory should be able to change the account owner, if she leaves clear data" + ); + assert_eq!( + Change::new(&mallory_program_id, &mallory_program_id) + .owner(&alice_program_id) + .data(vec![42], vec![0]) + .verify(), + Ok(()), + "mallory should be able to change the account owner, if she leaves clear data" + ); + assert_eq!( + Change::new(&mallory_program_id, &mallory_program_id) + .owner(&alice_program_id) + .executable(true, true) + .data(vec![42], vec![0]) + .verify(), + Err(InstructionError::ModifiedProgramId), + "mallory should not be able to change the account owner, if the account executable" + ); + assert_eq!( + Change::new(&mallory_program_id, &mallory_program_id) + .owner(&alice_program_id) + .data(vec![42], vec![42]) + .verify(), + Err(InstructionError::ModifiedProgramId), + "mallory should not be able to inject data into the alice program" + ); + } + + #[test] + fn test_verify_account_changes_executable() { + let owner = solana_sdk::pubkey::new_rand(); + let mallory_program_id = solana_sdk::pubkey::new_rand(); + let system_program_id = system_program::id(); + + assert_eq!( + Change::new(&owner, &system_program_id) + .executable(false, true) + .verify(), + Err(InstructionError::ExecutableModified), + "system program can't change executable if system doesn't own the account" + ); + assert_eq!( + Change::new(&owner, &system_program_id) + .executable(true, true) + .data(vec![1], vec![2]) + .verify(), + Err(InstructionError::ExecutableDataModified), + "system program can't change executable data if system doesn't own the account" + ); + assert_eq!( + Change::new(&owner, &owner).executable(false, true).verify(), + Ok(()), + "owner should be able to change executable" + ); + assert_eq!( + Change::new(&owner, &owner) + .executable(false, true) + .read_only() + .verify(), + Err(InstructionError::ExecutableModified), + "owner can't modify executable of read-only accounts" + ); + assert_eq!( + Change::new(&owner, &owner).executable(true, false).verify(), + Err(InstructionError::ExecutableModified), + "owner program can't reverse executable" + ); + assert_eq!( + Change::new(&owner, &mallory_program_id) + .executable(false, true) + .verify(), + Err(InstructionError::ExecutableModified), + "malicious Mallory should not be able to change the account executable" + ); + assert_eq!( + Change::new(&owner, &owner) + .executable(false, true) + .data(vec![1], vec![2]) + .verify(), + Ok(()), + "account data can change in the same instruction that sets the bit" + ); + assert_eq!( + Change::new(&owner, &owner) + .executable(true, true) + .data(vec![1], vec![2]) + .verify(), + Err(InstructionError::ExecutableDataModified), + "owner should not be able to change an account's data once its marked executable" + ); + assert_eq!( + Change::new(&owner, &owner) + .executable(true, true) + .lamports(1, 2) + .verify(), + Err(InstructionError::ExecutableLamportChange), + "owner should not be able to add lamports once marked executable" + ); + assert_eq!( + Change::new(&owner, &owner) + .executable(true, true) + .lamports(1, 2) + .verify(), + Err(InstructionError::ExecutableLamportChange), + "owner should not be able to add lamports once marked executable" + ); + assert_eq!( + Change::new(&owner, &owner) + .executable(true, true) + .lamports(2, 1) + .verify(), + Err(InstructionError::ExecutableLamportChange), + "owner should not be able to subtract lamports once marked executable" + ); + let data = vec![1; 100]; + let min_lamports = Rent::default().minimum_balance(data.len()); + assert_eq!( + Change::new(&owner, &owner) + .executable(false, true) + .lamports(0, min_lamports) + .data(data.clone(), data.clone()) + .verify(), + Ok(()), + ); + assert_eq!( + Change::new(&owner, &owner) + .executable(false, true) + .lamports(0, min_lamports - 1) + .data(data.clone(), data) + .verify(), + Err(InstructionError::ExecutableAccountNotRentExempt), + "owner should not be able to change an account's data once its marked executable" + ); + } + + #[test] + fn test_verify_account_changes_data_len() { + let alice_program_id = solana_sdk::pubkey::new_rand(); + + assert_eq!( + Change::new(&system_program::id(), &system_program::id()) + .data(vec![0], vec![0, 0]) + .verify(), + Ok(()), + "system program should be able to change the data len" + ); + assert_eq!( + Change::new(&alice_program_id, &system_program::id()) + .data(vec![0], vec![0,0]) + .verify(), + Err(InstructionError::AccountDataSizeChanged), + "system program should not be able to change the data length of accounts it does not own" + ); + } + + #[test] + fn test_verify_account_changes_data() { + let alice_program_id = solana_sdk::pubkey::new_rand(); + let mallory_program_id = solana_sdk::pubkey::new_rand(); + + assert_eq!( + Change::new(&alice_program_id, &alice_program_id) + .data(vec![0], vec![42]) + .verify(), + Ok(()), + "alice program should be able to change the data" + ); + assert_eq!( + Change::new(&mallory_program_id, &alice_program_id) + .data(vec![0], vec![42]) + .verify(), + Err(InstructionError::ExternalAccountDataModified), + "non-owner mallory should not be able to change the account data" + ); + assert_eq!( + Change::new(&alice_program_id, &alice_program_id) + .data(vec![0], vec![42]) + .read_only() + .verify(), + Err(InstructionError::ReadonlyDataModified), + "alice isn't allowed to touch a CO account" + ); + } + + #[test] + fn test_verify_account_changes_rent_epoch() { + let alice_program_id = solana_sdk::pubkey::new_rand(); + + assert_eq!( + Change::new(&alice_program_id, &system_program::id()).verify(), + Ok(()), + "nothing changed!" + ); + assert_eq!( + Change::new(&alice_program_id, &system_program::id()) + .rent_epoch(0, 1) + .verify(), + Err(InstructionError::RentEpochModified), + "no one touches rent_epoch" + ); + } + + #[test] + fn test_verify_account_changes_deduct_lamports_and_reassign_account() { + let alice_program_id = solana_sdk::pubkey::new_rand(); + let bob_program_id = solana_sdk::pubkey::new_rand(); + + // positive test of this capability + assert_eq!( + Change::new(&alice_program_id, &alice_program_id) + .owner(&bob_program_id) + .lamports(42, 1) + .data(vec![42], vec![0]) + .verify(), + Ok(()), + "alice should be able to deduct lamports and give the account to bob if the data is zeroed", + ); + } + + #[test] + fn test_verify_account_changes_lamports() { + let alice_program_id = solana_sdk::pubkey::new_rand(); + + assert_eq!( + Change::new(&alice_program_id, &system_program::id()) + .lamports(42, 0) + .read_only() + .verify(), + Err(InstructionError::ExternalAccountLamportSpend), + "debit should fail, even if system program" + ); + assert_eq!( + Change::new(&alice_program_id, &alice_program_id) + .lamports(42, 0) + .read_only() + .verify(), + Err(InstructionError::ReadonlyLamportChange), + "debit should fail, even if owning program" + ); + assert_eq!( + Change::new(&alice_program_id, &system_program::id()) + .lamports(42, 0) + .owner(&system_program::id()) + .verify(), + Err(InstructionError::ModifiedProgramId), + "system program can't debit the account unless it was the pre.owner" + ); + assert_eq!( + Change::new(&system_program::id(), &system_program::id()) + .lamports(42, 0) + .owner(&alice_program_id) + .verify(), + Ok(()), + "system can spend (and change owner)" + ); + } + + #[test] + fn test_verify_account_changes_data_size_changed() { + let alice_program_id = solana_sdk::pubkey::new_rand(); + + assert_eq!( + Change::new(&alice_program_id, &system_program::id()) + .data(vec![0], vec![0, 0]) + .verify(), + Err(InstructionError::AccountDataSizeChanged), + "system program should not be able to change another program's account data size" + ); + assert_eq!( + Change::new(&alice_program_id, &alice_program_id) + .data(vec![0], vec![0, 0]) + .verify(), + Err(InstructionError::AccountDataSizeChanged), + "non-system programs cannot change their data size" + ); + assert_eq!( + Change::new(&system_program::id(), &system_program::id()) + .data(vec![0], vec![0, 0]) + .verify(), + Ok(()), + "system program should be able to change account data size" + ); + } + + #[test] + fn test_verify_account_changes_owner_executable() { + let alice_program_id = solana_sdk::pubkey::new_rand(); + let bob_program_id = solana_sdk::pubkey::new_rand(); + + assert_eq!( + Change::new(&alice_program_id, &alice_program_id) + .owner(&bob_program_id) + .executable(false, true) + .verify(), + Err(InstructionError::ExecutableModified), + "Program should not be able to change owner and executable at the same time" + ); + } + + #[test] + fn test_debug() { + let mut instruction_processor = InstructionProcessor::default(); + #[allow(clippy::unnecessary_wraps)] + fn mock_process_instruction( + _program_id: &Pubkey, + _data: &[u8], + _invoke_context: &mut dyn InvokeContext, + ) -> Result<(), InstructionError> { + Ok(()) + } + #[allow(clippy::unnecessary_wraps)] + fn mock_ix_processor( + _pubkey: &Pubkey, + _data: &[u8], + _context: &mut dyn InvokeContext, + ) -> Result<(), InstructionError> { + Ok(()) + } + let program_id = solana_sdk::pubkey::new_rand(); + instruction_processor.add_program(program_id, mock_process_instruction); + instruction_processor.add_program(program_id, mock_ix_processor); + + assert!(!format!("{:?}", instruction_processor).is_empty()); + } +} diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 5a1ff522291..e7db0170c61 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -2180,6 +2180,7 @@ mod tests { NoopFail, ModifyOwned, ModifyNotOwned, + ModifyReadonly, } fn mock_process_instruction( @@ -2204,6 +2205,9 @@ mod tests { MockInstruction::ModifyNotOwned => { keyed_accounts[1].try_account_ref_mut()?.data_as_mut_slice()[0] = 1 } + MockInstruction::ModifyReadonly => { + keyed_accounts[2].try_account_ref_mut()?.data_as_mut_slice()[0] = 1 + } } } else { return Err(InstructionError::InvalidInstructionData); @@ -2214,6 +2218,7 @@ mod tests { let caller_program_id = solana_sdk::pubkey::new_rand(); let callee_program_id = solana_sdk::pubkey::new_rand(); +<<<<<<< HEAD let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); program_account.set_executable(true); let executable_accounts = vec![( @@ -2221,8 +2226,13 @@ mod tests { Rc::new(RefCell::new(program_account.clone())), )]; +======= +>>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) let owned_account = AccountSharedData::new(42, 1, &callee_program_id); let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand()); + let readonly_account = AccountSharedData::new(168, 1, &caller_program_id); + let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); + program_account.set_executable(true); #[allow(unused_mut)] let accounts = vec![ @@ -2234,23 +2244,35 @@ mod tests { solana_sdk::pubkey::new_rand(), Rc::new(RefCell::new(not_owned_account)), ), +<<<<<<< HEAD + (callee_program_id, Rc::new(RefCell::new(program_account))), + ]; +======= + ( + solana_sdk::pubkey::new_rand(), + Rc::new(RefCell::new(readonly_account)), + ), (callee_program_id, Rc::new(RefCell::new(program_account))), ]; + let program_indices = vec![3]; +>>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) - let compiled_instruction = CompiledInstruction::new(2, &(), vec![0, 1, 2]); let programs: Vec<(_, ProcessInstructionWithContext)> = vec![(callee_program_id, mock_process_instruction)]; let metas = vec![ AccountMeta::new(accounts[0].0, false), AccountMeta::new(accounts[1].0, false), + AccountMeta::new_readonly(accounts[2].0, false), ]; - let instruction = Instruction::new_with_bincode( + let caller_instruction = CompiledInstruction::new(2, &(), vec![0, 1, 2, 3]); + let callee_instruction = Instruction::new_with_bincode( callee_program_id, &MockInstruction::NoopSuccess, metas.clone(), ); - let message = Message::new(&[instruction], None); + let message = Message::new(&[callee_instruction], None); + let feature_set = FeatureSet::all_enabled(); let demote_program_write_locks = feature_set.is_active(&demote_program_write_locks::id()); @@ -2259,8 +2281,13 @@ mod tests { &caller_program_id, Rent::default(), &message, +<<<<<<< HEAD &compiled_instruction, &executable_accounts, +======= + &caller_instruction, + &program_indices, +>>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) &accounts, programs.as_slice(), None, @@ -2292,6 +2319,20 @@ mod tests { ); accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 0; + // readonly account modified by the invoker + accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 1; + assert_eq!( + InstructionProcessor::process_cross_program_instruction( + &message, + &program_indices, + &accounts, + &caller_write_privileges, + &mut invoke_context, + ), + Err(InstructionError::ReadonlyDataModified) + ); + accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 0; + let cases = vec![ (MockInstruction::NoopSuccess, Ok(())), ( @@ -2306,17 +2347,22 @@ mod tests { ]; for case in cases { - let instruction = + let callee_instruction = Instruction::new_with_bincode(callee_program_id, &case.0, metas.clone()); - let message = Message::new(&[instruction], None); + let message = Message::new(&[callee_instruction], None); let ancestors = Ancestors::default(); let mut invoke_context = ThisInvokeContext::new( &caller_program_id, Rent::default(), &message, +<<<<<<< HEAD &compiled_instruction, &executable_accounts, +======= + &caller_instruction, + &program_indices, +>>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) &accounts, programs.as_slice(), None, @@ -2348,6 +2394,7 @@ mod tests { } #[test] +<<<<<<< HEAD fn test_debug() { let mut message_processor = MessageProcessor::default(); #[allow(clippy::unnecessary_wraps)] @@ -2371,5 +2418,198 @@ mod tests { message_processor.add_loader(program_id, mock_ix_processor); assert!(!format!("{:?}", message_processor).is_empty()); +======= + fn test_native_invoke() { + #[derive(Debug, Serialize, Deserialize)] + enum MockInstruction { + NoopSuccess, + NoopFail, + ModifyOwned, + ModifyNotOwned, + ModifyReadonly, + } + + fn mock_process_instruction( + program_id: &Pubkey, + data: &[u8], + invoke_context: &mut dyn InvokeContext, + ) -> Result<(), InstructionError> { + let keyed_accounts = invoke_context.get_keyed_accounts()?; + assert_eq!(*program_id, keyed_accounts[0].owner()?); + assert_ne!( + keyed_accounts[1].owner()?, + *keyed_accounts[0].unsigned_key() + ); + + if let Ok(instruction) = bincode::deserialize(data) { + match instruction { + MockInstruction::NoopSuccess => (), + MockInstruction::NoopFail => return Err(InstructionError::GenericError), + MockInstruction::ModifyOwned => { + keyed_accounts[0].try_account_ref_mut()?.data_as_mut_slice()[0] = 1 + } + MockInstruction::ModifyNotOwned => { + keyed_accounts[1].try_account_ref_mut()?.data_as_mut_slice()[0] = 1 + } + MockInstruction::ModifyReadonly => { + keyed_accounts[2].try_account_ref_mut()?.data_as_mut_slice()[0] = 1 + } + } + } else { + return Err(InstructionError::InvalidInstructionData); + } + Ok(()) + } + + let caller_program_id = solana_sdk::pubkey::new_rand(); + let callee_program_id = solana_sdk::pubkey::new_rand(); + + let owned_account = AccountSharedData::new(42, 1, &callee_program_id); + let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand()); + let readonly_account = AccountSharedData::new(168, 1, &caller_program_id); + let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); + program_account.set_executable(true); + + #[allow(unused_mut)] + let accounts = vec![ + ( + solana_sdk::pubkey::new_rand(), + Rc::new(RefCell::new(owned_account)), + ), + ( + solana_sdk::pubkey::new_rand(), + Rc::new(RefCell::new(not_owned_account)), + ), + ( + solana_sdk::pubkey::new_rand(), + Rc::new(RefCell::new(readonly_account)), + ), + (callee_program_id, Rc::new(RefCell::new(program_account))), + ]; + let program_indices = vec![3]; + let programs: Vec<(_, ProcessInstructionWithContext)> = + vec![(callee_program_id, mock_process_instruction)]; + let metas = vec![ + AccountMeta::new(accounts[0].0, false), + AccountMeta::new(accounts[1].0, false), + AccountMeta::new_readonly(accounts[2].0, false), + ]; + + let caller_instruction = CompiledInstruction::new(2, &(), vec![0, 1, 2, 3]); + let callee_instruction = Instruction::new_with_bincode( + callee_program_id, + &MockInstruction::NoopSuccess, + metas.clone(), + ); + let message = Message::new(&[callee_instruction.clone()], None); + + let feature_set = FeatureSet::all_enabled(); + let ancestors = Ancestors::default(); + let blockhash = Hash::default(); + let fee_calculator = FeeCalculator::default(); + let mut invoke_context = ThisInvokeContext::new( + &caller_program_id, + Rent::default(), + &message, + &caller_instruction, + &program_indices, + &accounts, + programs.as_slice(), + None, + ComputeBudget::default(), + Rc::new(RefCell::new(MockComputeMeter::default())), + Rc::new(RefCell::new(Executors::default())), + None, + Arc::new(feature_set), + Arc::new(Accounts::default_for_tests()), + &ancestors, + &blockhash, + &fee_calculator, + ) + .unwrap(); + + // not owned account modified by the invoker + accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1; + assert_eq!( + InstructionProcessor::native_invoke( + &mut invoke_context, + callee_instruction.clone(), + &[0, 1, 2, 3], + &[] + ), + Err(InstructionError::ExternalAccountDataModified) + ); + accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 0; + + // readonly account modified by the invoker + accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 1; + assert_eq!( + InstructionProcessor::native_invoke( + &mut invoke_context, + callee_instruction, + &[0, 1, 2, 3], + &[] + ), + Err(InstructionError::ReadonlyDataModified) + ); + accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 0; + + // Other test cases + let cases = vec![ + (MockInstruction::NoopSuccess, Ok(())), + ( + MockInstruction::NoopFail, + Err(InstructionError::GenericError), + ), + (MockInstruction::ModifyOwned, Ok(())), + ( + MockInstruction::ModifyNotOwned, + Err(InstructionError::ExternalAccountDataModified), + ), + ( + MockInstruction::ModifyReadonly, + Err(InstructionError::ReadonlyDataModified), + ), + ]; + for case in cases { + let callee_instruction = + Instruction::new_with_bincode(callee_program_id, &case.0, metas.clone()); + let message = Message::new(&[callee_instruction.clone()], None); + + let ancestors = Ancestors::default(); + let blockhash = Hash::default(); + let fee_calculator = FeeCalculator::default(); + let mut invoke_context = ThisInvokeContext::new( + &caller_program_id, + Rent::default(), + &message, + &caller_instruction, + &program_indices, + &accounts, + programs.as_slice(), + None, + ComputeBudget::default(), + Rc::new(RefCell::new(MockComputeMeter::default())), + Rc::new(RefCell::new(Executors::default())), + None, + Arc::new(FeatureSet::all_enabled()), + Arc::new(Accounts::default_for_tests()), + &ancestors, + &blockhash, + &fee_calculator, + ) + .unwrap(); + + assert_eq!( + InstructionProcessor::native_invoke( + &mut invoke_context, + callee_instruction, + &[0, 1, 2, 3], + &[] + ), + case.1 + ); + } +>>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) } } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 430f6994262..b41b79a7f95 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -199,6 +199,17 @@ pub mod check_seed_length { solana_sdk::declare_id!("8HYXgkoKGreAMA3MfJkdjbKNVbfZRQP3jqFpa7iqN4v7"); } +<<<<<<< HEAD +======= +pub mod return_data_syscall_enabled { + solana_sdk::declare_id!("BJVXq6NdLC7jCDGjfqJv7M1XHD4Y13VrpDqRF2U7UBcC"); +} + +pub mod fix_write_privs { + solana_sdk::declare_id!("7Tr5C1tdcCeBVD8jxtHYnvjL1DGdFboYBHCJkEFdenBb"); +} + +>>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -249,6 +260,11 @@ lazy_static! { (demote_program_write_locks::id(), "demote program write locks to readonly #19593"), (allow_native_ids::id(), "allow native program ids in program derived addresses"), (check_seed_length::id(), "Check program address seed lengths"), +<<<<<<< HEAD +======= + (return_data_syscall_enabled::id(), "enable sol_{set,get}_return_data syscall"), + (fix_write_privs::id(), "fix native invoke write privileges"), +>>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) /*************** ADD NEW FEATURES HERE ***************/ ] .iter() From 6a8c61c7026a3a224430f09f887465e2d7ca0694 Mon Sep 17 00:00:00 2001 From: Jack May Date: Mon, 13 Sep 2021 23:50:04 -0700 Subject: [PATCH 2/2] resolve conflictds --- program-runtime/src/instruction_processor.rs | 1214 ------------------ runtime/src/message_processor.rs | 168 ++- sdk/src/feature_set.rs | 11 - 3 files changed, 82 insertions(+), 1311 deletions(-) delete mode 100644 program-runtime/src/instruction_processor.rs diff --git a/program-runtime/src/instruction_processor.rs b/program-runtime/src/instruction_processor.rs deleted file mode 100644 index 26f62f93a8a..00000000000 --- a/program-runtime/src/instruction_processor.rs +++ /dev/null @@ -1,1214 +0,0 @@ -use crate::native_loader::NativeLoader; -use serde::{Deserialize, Serialize}; -use solana_sdk::{ - account::{AccountSharedData, ReadableAccount, WritableAccount}, - account_utils::StateMut, - bpf_loader_upgradeable::{self, UpgradeableLoaderState}, - feature_set::{demote_program_write_locks, fix_write_privs}, - ic_logger_msg, ic_msg, - instruction::{CompiledInstruction, Instruction, InstructionError}, - keyed_account::{keyed_account_at_index, KeyedAccount}, - message::Message, - process_instruction::{Executor, InvokeContext, Logger, ProcessInstructionWithContext}, - pubkey::Pubkey, - rent::Rent, - system_program, -}; -use std::{ - cell::{Ref, RefCell}, - collections::HashMap, - rc::Rc, - sync::Arc, -}; - -pub struct Executors { - pub executors: HashMap>, - pub is_dirty: bool, -} -impl Default for Executors { - fn default() -> Self { - Self { - executors: HashMap::default(), - is_dirty: false, - } - } -} -impl Executors { - pub fn insert(&mut self, key: Pubkey, executor: Arc) { - let _ = self.executors.insert(key, executor); - self.is_dirty = true; - } - pub fn get(&self, key: &Pubkey) -> Option> { - self.executors.get(key).cloned() - } -} - -#[derive(Default, Debug)] -pub struct ProgramTiming { - pub accumulated_us: u64, - pub accumulated_units: u64, - pub count: u32, -} - -#[derive(Default, Debug)] -pub struct ExecuteDetailsTimings { - pub serialize_us: u64, - pub create_vm_us: u64, - pub execute_us: u64, - pub deserialize_us: u64, - pub changed_account_count: u64, - pub total_account_count: u64, - pub total_data_size: usize, - pub data_size_changed: usize, - pub per_program_timings: HashMap, -} -impl ExecuteDetailsTimings { - pub fn accumulate(&mut self, other: &ExecuteDetailsTimings) { - self.serialize_us += other.serialize_us; - self.create_vm_us += other.create_vm_us; - self.execute_us += other.execute_us; - self.deserialize_us += other.deserialize_us; - self.changed_account_count += other.changed_account_count; - self.total_account_count += other.total_account_count; - self.total_data_size += other.total_data_size; - self.data_size_changed += other.data_size_changed; - for (id, other) in &other.per_program_timings { - let program_timing = self.per_program_timings.entry(*id).or_default(); - program_timing.accumulated_us = program_timing - .accumulated_us - .saturating_add(other.accumulated_us); - program_timing.accumulated_units = program_timing - .accumulated_units - .saturating_add(other.accumulated_units); - program_timing.count = program_timing.count.saturating_add(other.count); - } - } - pub fn accumulate_program(&mut self, program_id: &Pubkey, us: u64, units: u64) { - let program_timing = self.per_program_timings.entry(*program_id).or_default(); - program_timing.accumulated_us = program_timing.accumulated_us.saturating_add(us); - program_timing.accumulated_units = program_timing.accumulated_units.saturating_add(units); - program_timing.count = program_timing.count.saturating_add(1); - } -} - -// The relevant state of an account before an Instruction executes, used -// to verify account integrity after the Instruction completes -#[derive(Clone, Debug, Default)] -pub struct PreAccount { - key: Pubkey, - account: Rc>, - changed: bool, -} -impl PreAccount { - pub fn new(key: &Pubkey, account: &AccountSharedData) -> Self { - Self { - key: *key, - account: Rc::new(RefCell::new(account.clone())), - changed: false, - } - } - - pub fn verify( - &self, - program_id: &Pubkey, - is_writable: bool, - rent: &Rent, - post: &AccountSharedData, - timings: &mut ExecuteDetailsTimings, - outermost_call: bool, - updated_verify_policy: bool, - ) -> Result<(), InstructionError> { - let pre = self.account.borrow(); - - // Only the owner of the account may change owner and - // only if the account is writable and - // only if the account is not executable and - // only if the data is zero-initialized or empty - let owner_changed = pre.owner() != post.owner(); - if owner_changed - && (!is_writable // line coverage used to get branch coverage - || pre.executable() - || program_id != pre.owner() - || !Self::is_zeroed(post.data())) - { - return Err(InstructionError::ModifiedProgramId); - } - - // An account not assigned to the program cannot have its balance decrease. - if program_id != pre.owner() // line coverage used to get branch coverage - && pre.lamports() > post.lamports() - { - return Err(InstructionError::ExternalAccountLamportSpend); - } - - // The balance of read-only and executable accounts may not change - let lamports_changed = pre.lamports() != post.lamports(); - if lamports_changed { - if !is_writable { - return Err(InstructionError::ReadonlyLamportChange); - } - if pre.executable() { - return Err(InstructionError::ExecutableLamportChange); - } - } - - // Only the system program can change the size of the data - // and only if the system program owns the account - let data_len_changed = pre.data().len() != post.data().len(); - if data_len_changed - && (!system_program::check_id(program_id) // line coverage used to get branch coverage - || !system_program::check_id(pre.owner())) - { - return Err(InstructionError::AccountDataSizeChanged); - } - - // Only the owner may change account data - // and if the account is writable - // and if the account is not executable - if !(program_id == pre.owner() - && is_writable // line coverage used to get branch coverage - && !pre.executable()) - && pre.data() != post.data() - { - if pre.executable() { - return Err(InstructionError::ExecutableDataModified); - } else if is_writable { - return Err(InstructionError::ExternalAccountDataModified); - } else { - return Err(InstructionError::ReadonlyDataModified); - } - } - - // executable is one-way (false->true) and only the account owner may set it. - let executable_changed = pre.executable() != post.executable(); - if executable_changed { - if !rent.is_exempt(post.lamports(), post.data().len()) { - return Err(InstructionError::ExecutableAccountNotRentExempt); - } - let owner = if updated_verify_policy { - post.owner() - } else { - pre.owner() - }; - if !is_writable // line coverage used to get branch coverage - || pre.executable() - || program_id != owner - { - return Err(InstructionError::ExecutableModified); - } - } - - // No one modifies rent_epoch (yet). - let rent_epoch_changed = pre.rent_epoch() != post.rent_epoch(); - if rent_epoch_changed { - return Err(InstructionError::RentEpochModified); - } - - if outermost_call { - timings.total_account_count += 1; - timings.total_data_size += post.data().len(); - if owner_changed - || lamports_changed - || data_len_changed - || executable_changed - || rent_epoch_changed - || self.changed - { - timings.changed_account_count += 1; - timings.data_size_changed += post.data().len(); - } - } - - Ok(()) - } - - pub fn update(&mut self, account: &AccountSharedData) { - let mut pre = self.account.borrow_mut(); - let rent_epoch = pre.rent_epoch(); - *pre = account.clone(); - pre.set_rent_epoch(rent_epoch); - - self.changed = true; - } - - pub fn key(&self) -> &Pubkey { - &self.key - } - - pub fn data(&self) -> Ref<[u8]> { - Ref::map(self.account.borrow(), |account| account.data()) - } - - pub fn lamports(&self) -> u64 { - self.account.borrow().lamports() - } - - pub fn executable(&self) -> bool { - self.account.borrow().executable() - } - - pub fn is_zeroed(buf: &[u8]) -> bool { - const ZEROS_LEN: usize = 1024; - static ZEROS: [u8; ZEROS_LEN] = [0; ZEROS_LEN]; - let mut chunks = buf.chunks_exact(ZEROS_LEN); - - chunks.all(|chunk| chunk == &ZEROS[..]) - && chunks.remainder() == &ZEROS[..chunks.remainder().len()] - } -} - -#[derive(Deserialize, Serialize)] -pub struct InstructionProcessor { - #[serde(skip)] - programs: Vec<(Pubkey, ProcessInstructionWithContext)>, - #[serde(skip)] - native_loader: NativeLoader, -} - -impl std::fmt::Debug for InstructionProcessor { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - #[derive(Debug)] - struct MessageProcessor<'a> { - programs: Vec, - native_loader: &'a NativeLoader, - } - - // These are just type aliases for work around of Debug-ing above pointers - type ErasedProcessInstructionWithContext = fn( - &'static Pubkey, - &'static [u8], - &'static mut dyn InvokeContext, - ) -> Result<(), InstructionError>; - - // rustc doesn't compile due to bug without this work around - // https://github.com/rust-lang/rust/issues/50280 - // https://users.rust-lang.org/t/display-function-pointer/17073/2 - let processor = MessageProcessor { - programs: self - .programs - .iter() - .map(|(pubkey, instruction)| { - let erased_instruction: ErasedProcessInstructionWithContext = *instruction; - format!("{}: {:p}", pubkey, erased_instruction) - }) - .collect::>(), - native_loader: &self.native_loader, - }; - - write!(f, "{:?}", processor) - } -} - -impl Default for InstructionProcessor { - fn default() -> Self { - Self { - programs: vec![], - native_loader: NativeLoader::default(), - } - } -} -impl Clone for InstructionProcessor { - fn clone(&self) -> Self { - InstructionProcessor { - programs: self.programs.clone(), - native_loader: NativeLoader::default(), - } - } -} - -#[cfg(RUSTC_WITH_SPECIALIZATION)] -impl ::solana_frozen_abi::abi_example::AbiExample for InstructionProcessor { - fn example() -> Self { - // MessageProcessor's fields are #[serde(skip)]-ed and not Serialize - // so, just rely on Default anyway. - InstructionProcessor::default() - } -} - -impl InstructionProcessor { - pub fn programs(&self) -> &[(Pubkey, ProcessInstructionWithContext)] { - &self.programs - } - - /// Add a static entrypoint to intercept instructions before the dynamic loader. - pub fn add_program( - &mut self, - program_id: Pubkey, - process_instruction: ProcessInstructionWithContext, - ) { - match self.programs.iter_mut().find(|(key, _)| program_id == *key) { - Some((_, processor)) => *processor = process_instruction, - None => self.programs.push((program_id, process_instruction)), - } - } - - /// Process an instruction - /// This method calls the instruction's program entrypoint method - pub fn process_instruction( - &self, - program_id: &Pubkey, - instruction_data: &[u8], - invoke_context: &mut dyn InvokeContext, - ) -> Result<(), InstructionError> { - if let Some(root_account) = invoke_context.get_keyed_accounts()?.iter().next() { - let root_id = root_account.unsigned_key(); - if solana_sdk::native_loader::check_id(&root_account.owner()?) { - for (id, process_instruction) in &self.programs { - if id == root_id { - invoke_context.remove_first_keyed_account()?; - // Call the builtin program - return process_instruction(program_id, instruction_data, invoke_context); - } - } - // Call the program via the native loader - return self.native_loader.process_instruction( - &solana_sdk::native_loader::id(), - instruction_data, - invoke_context, - ); - } else { - let owner_id = &root_account.owner()?; - for (id, process_instruction) in &self.programs { - if id == owner_id { - // Call the program via a builtin loader - return process_instruction(program_id, instruction_data, invoke_context); - } - } - } - } - Err(InstructionError::UnsupportedProgramId) - } - - pub fn create_message( - instruction: &Instruction, - keyed_accounts: &[&KeyedAccount], - signers: &[Pubkey], - invoke_context: &Ref<&mut dyn InvokeContext>, - ) -> Result<(Message, Pubkey, usize), InstructionError> { - // Check for privilege escalation - for account in instruction.accounts.iter() { - let keyed_account = keyed_accounts - .iter() - .find_map(|keyed_account| { - if &account.pubkey == keyed_account.unsigned_key() { - Some(keyed_account) - } else { - None - } - }) - .ok_or_else(|| { - ic_msg!( - invoke_context, - "Instruction references an unknown account {}", - account.pubkey - ); - InstructionError::MissingAccount - })?; - // Readonly account cannot become writable - if account.is_writable && !keyed_account.is_writable() { - ic_msg!( - invoke_context, - "{}'s writable privilege escalated", - account.pubkey - ); - return Err(InstructionError::PrivilegeEscalation); - } - - if account.is_signer && // If message indicates account is signed - !( // one of the following needs to be true: - keyed_account.signer_key().is_some() // Signed in the parent instruction - || signers.contains(&account.pubkey) // Signed by the program - ) { - ic_msg!( - invoke_context, - "{}'s signer privilege escalated", - account.pubkey - ); - return Err(InstructionError::PrivilegeEscalation); - } - } - - // validate the caller has access to the program account and that it is executable - let program_id = instruction.program_id; - match keyed_accounts - .iter() - .find(|keyed_account| &program_id == keyed_account.unsigned_key()) - { - Some(keyed_account) => { - if !keyed_account.executable()? { - ic_msg!( - invoke_context, - "Account {} is not executable", - keyed_account.unsigned_key() - ); - return Err(InstructionError::AccountNotExecutable); - } - } - None => { - ic_msg!(invoke_context, "Unknown program {}", program_id); - return Err(InstructionError::MissingAccount); - } - } - - let message = Message::new(&[instruction.clone()], None); - let program_id_index = message.instructions[0].program_id_index as usize; - - Ok((message, program_id, program_id_index)) - } - - /// Entrypoint for a cross-program invocation from a native program - pub fn native_invoke( - invoke_context: &mut dyn InvokeContext, - instruction: Instruction, - keyed_account_indices: &[usize], - signers: &[Pubkey], - ) -> Result<(), InstructionError> { - let invoke_context = RefCell::new(invoke_context); - - let ( - message, - program_indices, - accounts, - keyed_account_indices_reordered, - caller_write_privileges, - ) = { - let invoke_context = invoke_context.borrow(); - - let caller_keyed_accounts = invoke_context.get_keyed_accounts()?; - let callee_keyed_accounts = keyed_account_indices - .iter() - .map(|index| keyed_account_at_index(caller_keyed_accounts, *index)) - .collect::, InstructionError>>()?; - let (message, callee_program_id, _) = Self::create_message( - &instruction, - &callee_keyed_accounts, - signers, - &invoke_context, - )?; - let mut keyed_account_indices_reordered = - Vec::with_capacity(message.account_keys.len()); - let mut accounts = Vec::with_capacity(message.account_keys.len()); - let mut caller_write_privileges = Vec::with_capacity(message.account_keys.len()); - - // Translate and verify caller's data - if invoke_context.is_feature_active(&fix_write_privs::id()) { - 'root: for account_key in message.account_keys.iter() { - for keyed_account_index in keyed_account_indices { - let keyed_account = &caller_keyed_accounts[*keyed_account_index]; - if account_key == keyed_account.unsigned_key() { - accounts.push((*account_key, Rc::new(keyed_account.account.clone()))); - caller_write_privileges.push(keyed_account.is_writable()); - keyed_account_indices_reordered.push(*keyed_account_index); - continue 'root; - } - } - ic_msg!( - invoke_context, - "Instruction references an unknown account {}", - account_key - ); - return Err(InstructionError::MissingAccount); - } - } else { - let keyed_accounts = invoke_context.get_keyed_accounts()?; - for index in keyed_account_indices.iter() { - caller_write_privileges.push(keyed_accounts[*index].is_writable()); - } - caller_write_privileges.insert(0, false); - let keyed_accounts = invoke_context.get_keyed_accounts()?; - 'root2: for account_key in message.account_keys.iter() { - for keyed_account_index in keyed_account_indices { - let keyed_account = &keyed_accounts[*keyed_account_index]; - if account_key == keyed_account.unsigned_key() { - accounts.push((*account_key, Rc::new(keyed_account.account.clone()))); - keyed_account_indices_reordered.push(*keyed_account_index); - continue 'root2; - } - } - ic_msg!( - invoke_context, - "Instruction references an unknown account {}", - account_key - ); - return Err(InstructionError::MissingAccount); - } - } - - // Process instruction - - invoke_context.record_instruction(&instruction); - - let (program_account_index, program_account) = invoke_context - .get_account(&callee_program_id) - .ok_or_else(|| { - ic_msg!(invoke_context, "Unknown program {}", callee_program_id); - InstructionError::MissingAccount - })?; - if !program_account.borrow().executable() { - ic_msg!( - invoke_context, - "Account {} is not executable", - callee_program_id - ); - return Err(InstructionError::AccountNotExecutable); - } - let mut program_indices = vec![]; - if program_account.borrow().owner() == &bpf_loader_upgradeable::id() { - if let UpgradeableLoaderState::Program { - programdata_address, - } = program_account.borrow().state()? - { - if let Some((programdata_account_index, _programdata_account)) = - invoke_context.get_account(&programdata_address) - { - program_indices.push(programdata_account_index); - } else { - ic_msg!( - invoke_context, - "Unknown upgradeable programdata account {}", - programdata_address, - ); - return Err(InstructionError::MissingAccount); - } - } else { - ic_msg!( - invoke_context, - "Upgradeable program account state not valid {}", - callee_program_id, - ); - return Err(InstructionError::MissingAccount); - } - } - program_indices.insert(0, program_account_index); - ( - message, - program_indices, - accounts, - keyed_account_indices_reordered, - caller_write_privileges, - ) - }; - - #[allow(clippy::deref_addrof)] - InstructionProcessor::process_cross_program_instruction( - &message, - &program_indices, - &accounts, - &caller_write_privileges, - *(&mut *(invoke_context.borrow_mut())), - )?; - - // Copy results back to caller - - { - let invoke_context = invoke_context.borrow(); - let demote_program_write_locks = - invoke_context.is_feature_active(&demote_program_write_locks::id()); - let keyed_accounts = invoke_context.get_keyed_accounts()?; - for (src_keyed_account_index, ((_key, account), dst_keyed_account_index)) in accounts - .iter() - .zip(keyed_account_indices_reordered) - .enumerate() - { - let dst_keyed_account = &keyed_accounts[dst_keyed_account_index]; - let src_keyed_account = account.borrow(); - if message.is_writable(src_keyed_account_index, demote_program_write_locks) - && !src_keyed_account.executable() - { - if dst_keyed_account.data_len()? != src_keyed_account.data().len() - && dst_keyed_account.data_len()? != 0 - { - // Only support for `CreateAccount` at this time. - // Need a way to limit total realloc size across multiple CPI calls - ic_msg!( - invoke_context, - "Inner instructions do not support realloc, only SystemProgram::CreateAccount", - ); - return Err(InstructionError::InvalidRealloc); - } - dst_keyed_account - .try_account_ref_mut()? - .set_lamports(src_keyed_account.lamports()); - dst_keyed_account - .try_account_ref_mut()? - .set_owner(*src_keyed_account.owner()); - dst_keyed_account - .try_account_ref_mut()? - .set_data(src_keyed_account.data().to_vec()); - } - } - } - - Ok(()) - } - - /// Process a cross-program instruction - /// This method calls the instruction's program entrypoint function - pub fn process_cross_program_instruction( - message: &Message, - program_indices: &[usize], - accounts: &[(Pubkey, Rc>)], - caller_write_privileges: &[bool], - invoke_context: &mut dyn InvokeContext, - ) -> Result<(), InstructionError> { - if let Some(instruction) = message.instructions.get(0) { - let program_id = instruction.program_id(&message.account_keys); - - // Verify the calling program hasn't misbehaved - invoke_context.verify_and_update(instruction, accounts, caller_write_privileges)?; - - // clear the return data - invoke_context.set_return_data(None); - - // Invoke callee - invoke_context.push(program_id, message, instruction, program_indices, accounts)?; - - let mut instruction_processor = InstructionProcessor::default(); - for (program_id, process_instruction) in invoke_context.get_programs().iter() { - instruction_processor.add_program(*program_id, *process_instruction); - } - - let mut result = instruction_processor.process_instruction( - program_id, - &instruction.data, - invoke_context, - ); - if result.is_ok() { - // Verify the called program has not misbehaved - let demote_program_write_locks = - invoke_context.is_feature_active(&demote_program_write_locks::id()); - let write_privileges: Vec = (0..message.account_keys.len()) - .map(|i| message.is_writable(i, demote_program_write_locks)) - .collect(); - result = invoke_context.verify_and_update(instruction, accounts, &write_privileges); - } - - // Restore previous state - invoke_context.pop(); - result - } else { - // This function is always called with a valid instruction, if that changes return an error - Err(InstructionError::GenericError) - } - } - - /// Verify the results of a cross-program instruction - #[allow(clippy::too_many_arguments)] - pub fn verify_and_update( - instruction: &CompiledInstruction, - pre_accounts: &mut [PreAccount], - accounts: &[(Pubkey, Rc>)], - program_id: &Pubkey, - rent: &Rent, - write_privileges: &[bool], - timings: &mut ExecuteDetailsTimings, - logger: Rc>, - updated_verify_policy: bool, - ) -> Result<(), InstructionError> { - // Verify the per-account instruction results - let (mut pre_sum, mut post_sum) = (0_u128, 0_u128); - let mut work = |_unique_index: usize, account_index: usize| { - if account_index < write_privileges.len() && account_index < accounts.len() { - let (key, account) = &accounts[account_index]; - let is_writable = write_privileges[account_index]; - // Find the matching PreAccount - for pre_account in pre_accounts.iter_mut() { - if key == pre_account.key() { - { - // Verify account has no outstanding references - let _ = account - .try_borrow_mut() - .map_err(|_| InstructionError::AccountBorrowOutstanding)?; - } - let account = account.borrow(); - pre_account - .verify( - program_id, - is_writable, - rent, - &account, - timings, - false, - updated_verify_policy, - ) - .map_err(|err| { - ic_logger_msg!(logger, "failed to verify account {}: {}", key, err); - err - })?; - pre_sum += u128::from(pre_account.lamports()); - post_sum += u128::from(account.lamports()); - if is_writable && !pre_account.executable() { - pre_account.update(&account); - } - return Ok(()); - } - } - } - Err(InstructionError::MissingAccount) - }; - instruction.visit_each_account(&mut work)?; - - // Verify that the total sum of all the lamports did not change - if pre_sum != post_sum { - return Err(InstructionError::UnbalancedInstruction); - } - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use solana_sdk::{account::Account, instruction::InstructionError}; - - #[test] - fn test_is_zeroed() { - const ZEROS_LEN: usize = 1024; - let mut buf = [0; ZEROS_LEN]; - assert!(PreAccount::is_zeroed(&buf)); - buf[0] = 1; - assert!(!PreAccount::is_zeroed(&buf)); - - let mut buf = [0; ZEROS_LEN - 1]; - assert!(PreAccount::is_zeroed(&buf)); - buf[0] = 1; - assert!(!PreAccount::is_zeroed(&buf)); - - let mut buf = [0; ZEROS_LEN + 1]; - assert!(PreAccount::is_zeroed(&buf)); - buf[0] = 1; - assert!(!PreAccount::is_zeroed(&buf)); - - let buf = vec![]; - assert!(PreAccount::is_zeroed(&buf)); - } - - struct Change { - program_id: Pubkey, - is_writable: bool, - rent: Rent, - pre: PreAccount, - post: AccountSharedData, - } - impl Change { - pub fn new(owner: &Pubkey, program_id: &Pubkey) -> Self { - Self { - program_id: *program_id, - rent: Rent::default(), - is_writable: true, - pre: PreAccount::new( - &solana_sdk::pubkey::new_rand(), - &AccountSharedData::from(Account { - owner: *owner, - lamports: std::u64::MAX, - data: vec![], - ..Account::default() - }), - ), - post: AccountSharedData::from(Account { - owner: *owner, - lamports: std::u64::MAX, - ..Account::default() - }), - } - } - pub fn read_only(mut self) -> Self { - self.is_writable = false; - self - } - pub fn executable(mut self, pre: bool, post: bool) -> Self { - self.pre.account.borrow_mut().set_executable(pre); - self.post.set_executable(post); - self - } - pub fn lamports(mut self, pre: u64, post: u64) -> Self { - self.pre.account.borrow_mut().set_lamports(pre); - self.post.set_lamports(post); - self - } - pub fn owner(mut self, post: &Pubkey) -> Self { - self.post.set_owner(*post); - self - } - pub fn data(mut self, pre: Vec, post: Vec) -> Self { - self.pre.account.borrow_mut().set_data(pre); - self.post.set_data(post); - self - } - pub fn rent_epoch(mut self, pre: u64, post: u64) -> Self { - self.pre.account.borrow_mut().set_rent_epoch(pre); - self.post.set_rent_epoch(post); - self - } - pub fn verify(&self) -> Result<(), InstructionError> { - self.pre.verify( - &self.program_id, - self.is_writable, - &self.rent, - &self.post, - &mut ExecuteDetailsTimings::default(), - false, - true, - ) - } - } - - #[test] - fn test_verify_account_changes_owner() { - let system_program_id = system_program::id(); - let alice_program_id = solana_sdk::pubkey::new_rand(); - let mallory_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&system_program_id, &system_program_id) - .owner(&alice_program_id) - .verify(), - Ok(()), - "system program should be able to change the account owner" - ); - assert_eq!( - Change::new(&system_program_id, &system_program_id) - .owner(&alice_program_id) - .read_only() - .verify(), - Err(InstructionError::ModifiedProgramId), - "system program should not be able to change the account owner of a read-only account" - ); - assert_eq!( - Change::new(&mallory_program_id, &system_program_id) - .owner(&alice_program_id) - .verify(), - Err(InstructionError::ModifiedProgramId), - "system program should not be able to change the account owner of a non-system account" - ); - assert_eq!( - Change::new(&mallory_program_id, &mallory_program_id) - .owner(&alice_program_id) - .verify(), - Ok(()), - "mallory should be able to change the account owner, if she leaves clear data" - ); - assert_eq!( - Change::new(&mallory_program_id, &mallory_program_id) - .owner(&alice_program_id) - .data(vec![42], vec![0]) - .verify(), - Ok(()), - "mallory should be able to change the account owner, if she leaves clear data" - ); - assert_eq!( - Change::new(&mallory_program_id, &mallory_program_id) - .owner(&alice_program_id) - .executable(true, true) - .data(vec![42], vec![0]) - .verify(), - Err(InstructionError::ModifiedProgramId), - "mallory should not be able to change the account owner, if the account executable" - ); - assert_eq!( - Change::new(&mallory_program_id, &mallory_program_id) - .owner(&alice_program_id) - .data(vec![42], vec![42]) - .verify(), - Err(InstructionError::ModifiedProgramId), - "mallory should not be able to inject data into the alice program" - ); - } - - #[test] - fn test_verify_account_changes_executable() { - let owner = solana_sdk::pubkey::new_rand(); - let mallory_program_id = solana_sdk::pubkey::new_rand(); - let system_program_id = system_program::id(); - - assert_eq!( - Change::new(&owner, &system_program_id) - .executable(false, true) - .verify(), - Err(InstructionError::ExecutableModified), - "system program can't change executable if system doesn't own the account" - ); - assert_eq!( - Change::new(&owner, &system_program_id) - .executable(true, true) - .data(vec![1], vec![2]) - .verify(), - Err(InstructionError::ExecutableDataModified), - "system program can't change executable data if system doesn't own the account" - ); - assert_eq!( - Change::new(&owner, &owner).executable(false, true).verify(), - Ok(()), - "owner should be able to change executable" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(false, true) - .read_only() - .verify(), - Err(InstructionError::ExecutableModified), - "owner can't modify executable of read-only accounts" - ); - assert_eq!( - Change::new(&owner, &owner).executable(true, false).verify(), - Err(InstructionError::ExecutableModified), - "owner program can't reverse executable" - ); - assert_eq!( - Change::new(&owner, &mallory_program_id) - .executable(false, true) - .verify(), - Err(InstructionError::ExecutableModified), - "malicious Mallory should not be able to change the account executable" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(false, true) - .data(vec![1], vec![2]) - .verify(), - Ok(()), - "account data can change in the same instruction that sets the bit" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(true, true) - .data(vec![1], vec![2]) - .verify(), - Err(InstructionError::ExecutableDataModified), - "owner should not be able to change an account's data once its marked executable" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(true, true) - .lamports(1, 2) - .verify(), - Err(InstructionError::ExecutableLamportChange), - "owner should not be able to add lamports once marked executable" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(true, true) - .lamports(1, 2) - .verify(), - Err(InstructionError::ExecutableLamportChange), - "owner should not be able to add lamports once marked executable" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(true, true) - .lamports(2, 1) - .verify(), - Err(InstructionError::ExecutableLamportChange), - "owner should not be able to subtract lamports once marked executable" - ); - let data = vec![1; 100]; - let min_lamports = Rent::default().minimum_balance(data.len()); - assert_eq!( - Change::new(&owner, &owner) - .executable(false, true) - .lamports(0, min_lamports) - .data(data.clone(), data.clone()) - .verify(), - Ok(()), - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(false, true) - .lamports(0, min_lamports - 1) - .data(data.clone(), data) - .verify(), - Err(InstructionError::ExecutableAccountNotRentExempt), - "owner should not be able to change an account's data once its marked executable" - ); - } - - #[test] - fn test_verify_account_changes_data_len() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&system_program::id(), &system_program::id()) - .data(vec![0], vec![0, 0]) - .verify(), - Ok(()), - "system program should be able to change the data len" - ); - assert_eq!( - Change::new(&alice_program_id, &system_program::id()) - .data(vec![0], vec![0,0]) - .verify(), - Err(InstructionError::AccountDataSizeChanged), - "system program should not be able to change the data length of accounts it does not own" - ); - } - - #[test] - fn test_verify_account_changes_data() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - let mallory_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .data(vec![0], vec![42]) - .verify(), - Ok(()), - "alice program should be able to change the data" - ); - assert_eq!( - Change::new(&mallory_program_id, &alice_program_id) - .data(vec![0], vec![42]) - .verify(), - Err(InstructionError::ExternalAccountDataModified), - "non-owner mallory should not be able to change the account data" - ); - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .data(vec![0], vec![42]) - .read_only() - .verify(), - Err(InstructionError::ReadonlyDataModified), - "alice isn't allowed to touch a CO account" - ); - } - - #[test] - fn test_verify_account_changes_rent_epoch() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&alice_program_id, &system_program::id()).verify(), - Ok(()), - "nothing changed!" - ); - assert_eq!( - Change::new(&alice_program_id, &system_program::id()) - .rent_epoch(0, 1) - .verify(), - Err(InstructionError::RentEpochModified), - "no one touches rent_epoch" - ); - } - - #[test] - fn test_verify_account_changes_deduct_lamports_and_reassign_account() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - let bob_program_id = solana_sdk::pubkey::new_rand(); - - // positive test of this capability - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .owner(&bob_program_id) - .lamports(42, 1) - .data(vec![42], vec![0]) - .verify(), - Ok(()), - "alice should be able to deduct lamports and give the account to bob if the data is zeroed", - ); - } - - #[test] - fn test_verify_account_changes_lamports() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&alice_program_id, &system_program::id()) - .lamports(42, 0) - .read_only() - .verify(), - Err(InstructionError::ExternalAccountLamportSpend), - "debit should fail, even if system program" - ); - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .lamports(42, 0) - .read_only() - .verify(), - Err(InstructionError::ReadonlyLamportChange), - "debit should fail, even if owning program" - ); - assert_eq!( - Change::new(&alice_program_id, &system_program::id()) - .lamports(42, 0) - .owner(&system_program::id()) - .verify(), - Err(InstructionError::ModifiedProgramId), - "system program can't debit the account unless it was the pre.owner" - ); - assert_eq!( - Change::new(&system_program::id(), &system_program::id()) - .lamports(42, 0) - .owner(&alice_program_id) - .verify(), - Ok(()), - "system can spend (and change owner)" - ); - } - - #[test] - fn test_verify_account_changes_data_size_changed() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&alice_program_id, &system_program::id()) - .data(vec![0], vec![0, 0]) - .verify(), - Err(InstructionError::AccountDataSizeChanged), - "system program should not be able to change another program's account data size" - ); - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .data(vec![0], vec![0, 0]) - .verify(), - Err(InstructionError::AccountDataSizeChanged), - "non-system programs cannot change their data size" - ); - assert_eq!( - Change::new(&system_program::id(), &system_program::id()) - .data(vec![0], vec![0, 0]) - .verify(), - Ok(()), - "system program should be able to change account data size" - ); - } - - #[test] - fn test_verify_account_changes_owner_executable() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - let bob_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .owner(&bob_program_id) - .executable(false, true) - .verify(), - Err(InstructionError::ExecutableModified), - "Program should not be able to change owner and executable at the same time" - ); - } - - #[test] - fn test_debug() { - let mut instruction_processor = InstructionProcessor::default(); - #[allow(clippy::unnecessary_wraps)] - fn mock_process_instruction( - _program_id: &Pubkey, - _data: &[u8], - _invoke_context: &mut dyn InvokeContext, - ) -> Result<(), InstructionError> { - Ok(()) - } - #[allow(clippy::unnecessary_wraps)] - fn mock_ix_processor( - _pubkey: &Pubkey, - _data: &[u8], - _context: &mut dyn InvokeContext, - ) -> Result<(), InstructionError> { - Ok(()) - } - let program_id = solana_sdk::pubkey::new_rand(); - instruction_processor.add_program(program_id, mock_process_instruction); - instruction_processor.add_program(program_id, mock_ix_processor); - - assert!(!format!("{:?}", instruction_processor).is_empty()); - } -} diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index e7db0170c61..4f7a5203239 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -9,8 +9,8 @@ use solana_sdk::{ account_utils::StateMut, bpf_loader_upgradeable::{self, UpgradeableLoaderState}, feature_set::{ - demote_program_write_locks, instructions_sysvar_enabled, neon_evm_compute_budget, - updated_verify_policy, FeatureSet, + demote_program_write_locks, fix_write_privs, instructions_sysvar_enabled, + neon_evm_compute_budget, updated_verify_policy, FeatureSet, }, ic_logger_msg, ic_msg, instruction::{CompiledInstruction, Instruction, InstructionError}, @@ -762,38 +762,64 @@ impl MessageProcessor { ) = { let invoke_context = invoke_context.borrow(); - // Translate and verify caller's data - let keyed_accounts = invoke_context.get_keyed_accounts()?; - let keyed_accounts = keyed_account_indices + let caller_keyed_accounts = invoke_context.get_keyed_accounts()?; + let callee_keyed_accounts = keyed_account_indices .iter() - .map(|index| keyed_account_at_index(keyed_accounts, *index)) + .map(|index| keyed_account_at_index(caller_keyed_accounts, *index)) .collect::, InstructionError>>()?; - let (message, callee_program_id, _) = - Self::create_message(&instruction, &keyed_accounts, signers, &invoke_context)?; - let keyed_accounts = invoke_context.get_keyed_accounts()?; - let mut caller_write_privileges = keyed_account_indices - .iter() - .map(|index| keyed_accounts[*index].is_writable()) - .collect::>(); - caller_write_privileges.insert(0, false); - let mut accounts = vec![]; - let mut keyed_account_indices_reordered = vec![]; - let keyed_accounts = invoke_context.get_keyed_accounts()?; - 'root: for account_key in message.account_keys.iter() { - for keyed_account_index in keyed_account_indices { - let keyed_account = &keyed_accounts[*keyed_account_index]; - if account_key == keyed_account.unsigned_key() { - accounts.push((*account_key, Rc::new(keyed_account.account.clone()))); - keyed_account_indices_reordered.push(*keyed_account_index); - continue 'root; + let (message, callee_program_id, _) = Self::create_message( + &instruction, + &callee_keyed_accounts, + signers, + &invoke_context, + )?; + let mut keyed_account_indices_reordered = + Vec::with_capacity(message.account_keys.len()); + let mut accounts = Vec::with_capacity(message.account_keys.len()); + let mut caller_write_privileges = Vec::with_capacity(message.account_keys.len()); + + // Translate and verify caller's data + if invoke_context.is_feature_active(&fix_write_privs::id()) { + 'root: for account_key in message.account_keys.iter() { + for keyed_account_index in keyed_account_indices { + let keyed_account = &caller_keyed_accounts[*keyed_account_index]; + if account_key == keyed_account.unsigned_key() { + accounts.push((*account_key, Rc::new(keyed_account.account.clone()))); + caller_write_privileges.push(keyed_account.is_writable()); + keyed_account_indices_reordered.push(*keyed_account_index); + continue 'root; + } } + ic_msg!( + invoke_context, + "Instruction references an unknown account {}", + account_key + ); + return Err(InstructionError::MissingAccount); + } + } else { + let keyed_accounts = invoke_context.get_keyed_accounts()?; + for index in keyed_account_indices.iter() { + caller_write_privileges.push(keyed_accounts[*index].is_writable()); + } + caller_write_privileges.insert(0, false); + let keyed_accounts = invoke_context.get_keyed_accounts()?; + 'root2: for account_key in message.account_keys.iter() { + for keyed_account_index in keyed_account_indices { + let keyed_account = &keyed_accounts[*keyed_account_index]; + if account_key == keyed_account.unsigned_key() { + accounts.push((*account_key, Rc::new(keyed_account.account.clone()))); + keyed_account_indices_reordered.push(*keyed_account_index); + continue 'root2; + } + } + ic_msg!( + invoke_context, + "Instruction references an unknown account {}", + account_key + ); + return Err(InstructionError::MissingAccount); } - ic_msg!( - invoke_context, - "Instruction references an unknown account {}", - account_key - ); - return Err(InstructionError::MissingAccount); } // Process instruction @@ -2218,7 +2244,6 @@ mod tests { let caller_program_id = solana_sdk::pubkey::new_rand(); let callee_program_id = solana_sdk::pubkey::new_rand(); -<<<<<<< HEAD let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); program_account.set_executable(true); let executable_accounts = vec![( @@ -2226,13 +2251,9 @@ mod tests { Rc::new(RefCell::new(program_account.clone())), )]; -======= ->>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) let owned_account = AccountSharedData::new(42, 1, &callee_program_id); let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand()); let readonly_account = AccountSharedData::new(168, 1, &caller_program_id); - let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); - program_account.set_executable(true); #[allow(unused_mut)] let accounts = vec![ @@ -2244,18 +2265,12 @@ mod tests { solana_sdk::pubkey::new_rand(), Rc::new(RefCell::new(not_owned_account)), ), -<<<<<<< HEAD - (callee_program_id, Rc::new(RefCell::new(program_account))), - ]; -======= ( solana_sdk::pubkey::new_rand(), Rc::new(RefCell::new(readonly_account)), ), (callee_program_id, Rc::new(RefCell::new(program_account))), ]; - let program_indices = vec![3]; ->>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) let programs: Vec<(_, ProcessInstructionWithContext)> = vec![(callee_program_id, mock_process_instruction)]; @@ -2281,13 +2296,8 @@ mod tests { &caller_program_id, Rent::default(), &message, -<<<<<<< HEAD - &compiled_instruction, - &executable_accounts, -======= &caller_instruction, - &program_indices, ->>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) + &executable_accounts, &accounts, programs.as_slice(), None, @@ -2322,9 +2332,9 @@ mod tests { // readonly account modified by the invoker accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( - InstructionProcessor::process_cross_program_instruction( + MessageProcessor::process_cross_program_instruction( &message, - &program_indices, + &executable_accounts, &accounts, &caller_write_privileges, &mut invoke_context, @@ -2356,13 +2366,8 @@ mod tests { &caller_program_id, Rent::default(), &message, -<<<<<<< HEAD - &compiled_instruction, - &executable_accounts, -======= &caller_instruction, - &program_indices, ->>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) + &executable_accounts, &accounts, programs.as_slice(), None, @@ -2394,7 +2399,6 @@ mod tests { } #[test] -<<<<<<< HEAD fn test_debug() { let mut message_processor = MessageProcessor::default(); #[allow(clippy::unnecessary_wraps)] @@ -2418,7 +2422,9 @@ mod tests { message_processor.add_loader(program_id, mock_ix_processor); assert!(!format!("{:?}", message_processor).is_empty()); -======= + } + + #[test] fn test_native_invoke() { #[derive(Debug, Serialize, Deserialize)] enum MockInstruction { @@ -2464,11 +2470,16 @@ mod tests { let caller_program_id = solana_sdk::pubkey::new_rand(); let callee_program_id = solana_sdk::pubkey::new_rand(); + let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); + program_account.set_executable(true); + let executable_accounts = vec![( + callee_program_id, + Rc::new(RefCell::new(program_account.clone())), + )]; + let owned_account = AccountSharedData::new(42, 1, &callee_program_id); let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand()); let readonly_account = AccountSharedData::new(168, 1, &caller_program_id); - let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); - program_account.set_executable(true); #[allow(unused_mut)] let accounts = vec![ @@ -2486,7 +2497,6 @@ mod tests { ), (callee_program_id, Rc::new(RefCell::new(program_account))), ]; - let program_indices = vec![3]; let programs: Vec<(_, ProcessInstructionWithContext)> = vec![(callee_program_id, mock_process_instruction)]; let metas = vec![ @@ -2503,35 +2513,28 @@ mod tests { ); let message = Message::new(&[callee_instruction.clone()], None); - let feature_set = FeatureSet::all_enabled(); let ancestors = Ancestors::default(); - let blockhash = Hash::default(); - let fee_calculator = FeeCalculator::default(); let mut invoke_context = ThisInvokeContext::new( &caller_program_id, Rent::default(), &message, &caller_instruction, - &program_indices, + &executable_accounts, &accounts, programs.as_slice(), None, - ComputeBudget::default(), - Rc::new(RefCell::new(MockComputeMeter::default())), + BpfComputeBudget::default(), Rc::new(RefCell::new(Executors::default())), None, - Arc::new(feature_set), - Arc::new(Accounts::default_for_tests()), + Arc::new(FeatureSet::all_enabled()), + Arc::new(Accounts::default()), &ancestors, - &blockhash, - &fee_calculator, - ) - .unwrap(); + ); // not owned account modified by the invoker accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( - InstructionProcessor::native_invoke( + MessageProcessor::native_invoke( &mut invoke_context, callee_instruction.clone(), &[0, 1, 2, 3], @@ -2544,7 +2547,7 @@ mod tests { // readonly account modified by the invoker accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( - InstructionProcessor::native_invoke( + MessageProcessor::native_invoke( &mut invoke_context, callee_instruction, &[0, 1, 2, 3], @@ -2577,31 +2580,25 @@ mod tests { let message = Message::new(&[callee_instruction.clone()], None); let ancestors = Ancestors::default(); - let blockhash = Hash::default(); - let fee_calculator = FeeCalculator::default(); let mut invoke_context = ThisInvokeContext::new( &caller_program_id, Rent::default(), &message, &caller_instruction, - &program_indices, + &executable_accounts, &accounts, programs.as_slice(), None, - ComputeBudget::default(), - Rc::new(RefCell::new(MockComputeMeter::default())), + BpfComputeBudget::default(), Rc::new(RefCell::new(Executors::default())), None, Arc::new(FeatureSet::all_enabled()), - Arc::new(Accounts::default_for_tests()), + Arc::new(Accounts::default()), &ancestors, - &blockhash, - &fee_calculator, - ) - .unwrap(); + ); assert_eq!( - InstructionProcessor::native_invoke( + MessageProcessor::native_invoke( &mut invoke_context, callee_instruction, &[0, 1, 2, 3], @@ -2610,6 +2607,5 @@ mod tests { case.1 ); } ->>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) } } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index b41b79a7f95..3893a6f9fc5 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -199,17 +199,10 @@ pub mod check_seed_length { solana_sdk::declare_id!("8HYXgkoKGreAMA3MfJkdjbKNVbfZRQP3jqFpa7iqN4v7"); } -<<<<<<< HEAD -======= -pub mod return_data_syscall_enabled { - solana_sdk::declare_id!("BJVXq6NdLC7jCDGjfqJv7M1XHD4Y13VrpDqRF2U7UBcC"); -} - pub mod fix_write_privs { solana_sdk::declare_id!("7Tr5C1tdcCeBVD8jxtHYnvjL1DGdFboYBHCJkEFdenBb"); } ->>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -260,11 +253,7 @@ lazy_static! { (demote_program_write_locks::id(), "demote program write locks to readonly #19593"), (allow_native_ids::id(), "allow native program ids in program derived addresses"), (check_seed_length::id(), "Check program address seed lengths"), -<<<<<<< HEAD -======= - (return_data_syscall_enabled::id(), "enable sol_{set,get}_return_data syscall"), (fix_write_privs::id(), "fix native invoke write privileges"), ->>>>>>> 00d7981f6 (Fix native invoke writable privileges (#19750)) /*************** ADD NEW FEATURES HERE ***************/ ] .iter()