diff --git a/programs/bpf/c/src/invoke/invoke.c b/programs/bpf/c/src/invoke/invoke.c index c36dac2a17..62d8a0d5a6 100644 --- a/programs/bpf/c/src/invoke/invoke.c +++ b/programs/bpf/c/src/invoke/invoke.c @@ -291,7 +291,8 @@ extern uint64_t entrypoint(const uint8_t *input) { case TEST_EMPTY_ACCOUNTS_SLICE: { sol_log("Empty accounts slice"); - SolAccountMeta arguments[] = {}; + SolAccountMeta arguments[] = { + {accounts[INVOKED_ARGUMENT_INDEX].key, false, false}}; uint8_t data[] = {}; const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key, arguments, SOL_ARRAY_SIZE(arguments), diff --git a/programs/bpf/rust/invoke/src/lib.rs b/programs/bpf/rust/invoke/src/lib.rs index 9c33c5ef09..44c5c9b8fc 100644 --- a/programs/bpf/rust/invoke/src/lib.rs +++ b/programs/bpf/rust/invoke/src/lib.rs @@ -385,7 +385,11 @@ fn process_instruction( } TEST_EMPTY_ACCOUNTS_SLICE => { msg!("Empty accounts slice"); - let instruction = create_instruction(*accounts[INVOKED_PROGRAM_INDEX].key, &[], vec![]); + let instruction = create_instruction( + *accounts[INVOKED_PROGRAM_INDEX].key, + &[(accounts[INVOKED_ARGUMENT_INDEX].key, false, false)], + vec![], + ); invoke(&instruction, &[])?; } TEST_CAP_SEEDS => { diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index edc507f6bb..cdf97990e3 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -989,7 +989,7 @@ fn test_program_bpf_invoke() { let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); assert_eq!( result.unwrap_err().unwrap(), - TransactionError::InstructionError(0, InstructionError::ModifiedProgramId) + TransactionError::InstructionError(0, InstructionError::MissingRequiredSignature) ); assert_eq!(10, bank.get_balance(&from_pubkey)); assert_eq!(0, bank.get_balance(&to_pubkey)); diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 2e520f502f..d1e75b9256 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -23,7 +23,6 @@ use solana_sdk::{ hash::{Hasher, HASH_BYTES}, instruction::{AccountMeta, Instruction, InstructionError}, keyed_account::KeyedAccount, - message::Message, native_loader, process_instruction::{stable_log, ComputeMeter, InvokeContext, Logger}, program_error::ProgramError, @@ -834,7 +833,10 @@ struct AccountReferences<'a> { ref_to_len_in_vm: &'a mut u64, serialized_len_ptr: &'a mut u64, } -type TranslatedAccounts<'a> = (Vec>>, Vec>); +type TranslatedAccounts<'a> = ( + Vec>>, + Vec>>, +); /// Implemented by language specific data structure translators trait SyscallInvokeSigned<'a> { @@ -847,7 +849,8 @@ trait SyscallInvokeSigned<'a> { ) -> Result>; fn translate_accounts( &self, - message: &Message, + account_keys: &[Pubkey], + program_account_index: usize, account_infos_addr: u64, account_infos_len: u64, memory_mapping: &MemoryMapping, @@ -905,7 +908,8 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> { fn translate_accounts( &self, - message: &Message, + account_keys: &[Pubkey], + program_account_index: usize, account_infos_addr: u64, account_infos_len: u64, memory_mapping: &MemoryMapping, @@ -921,9 +925,13 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> { &[] }; - let mut accounts = Vec::with_capacity(message.account_keys.len()); - let mut refs = Vec::with_capacity(message.account_keys.len()); - 'root: for account_key in message.account_keys.iter() { + let mut accounts = Vec::with_capacity(account_keys.len()); + let mut refs = Vec::with_capacity(account_keys.len()); + 'root: for (i, account_key) in account_keys.iter().enumerate() { + if i == program_account_index { + // Don't look for caller passed executable, runtime already has it + continue 'root; + } for account_info in account_infos.iter() { let key = translate_type::( memory_mapping, @@ -986,14 +994,14 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedRust<'a> { owner: *owner, rent_epoch: account_info.rent_epoch, }))); - refs.push(AccountReferences { + refs.push(Some(AccountReferences { lamports, owner, data, vm_data_addr, ref_to_len_in_vm, serialized_len_ptr, - }); + })); continue 'root; } } @@ -1182,7 +1190,8 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> { fn translate_accounts( &self, - message: &Message, + account_keys: &[Pubkey], + program_account_index: usize, account_infos_addr: u64, account_infos_len: u64, memory_mapping: &MemoryMapping, @@ -1193,9 +1202,13 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> { account_infos_len, self.loader_id, )?; - let mut accounts = Vec::with_capacity(message.account_keys.len()); - let mut refs = Vec::with_capacity(message.account_keys.len()); - 'root: for account_key in message.account_keys.iter() { + let mut accounts = Vec::with_capacity(account_keys.len()); + let mut refs = Vec::with_capacity(account_keys.len()); + 'root: for (i, account_key) in account_keys.iter().enumerate() { + if i == program_account_index { + // Don't look for caller passed executable, runtime already has it + continue 'root; + } for account_info in account_infos.iter() { let key = translate_type::( memory_mapping, @@ -1247,14 +1260,14 @@ impl<'a> SyscallInvokeSigned<'a> for SyscallInvokeSignedC<'a> { owner: *owner, rent_epoch: account_info.rent_epoch, }))); - refs.push(AccountReferences { + refs.push(Some(AccountReferences { lamports, owner, data, vm_data_addr, ref_to_len_in_vm, serialized_len_ptr, - }); + })); continue 'root; } } @@ -1381,12 +1394,13 @@ fn call<'a>( .get_callers_keyed_accounts() .iter() .collect::>(); - let (message, callee_program_id) = + let (message, callee_program_id, program_id_index) = MessageProcessor::create_message(&instruction, &keyed_account_refs, &signers) .map_err(SyscallError::InstructionError)?; is_authorized_program(&callee_program_id)?; - let (accounts, account_refs) = syscall.translate_accounts( - &message, + let (mut accounts, mut account_refs) = syscall.translate_accounts( + &message.account_keys, + program_id_index, account_infos_addr, account_infos_len, memory_mapping, @@ -1426,6 +1440,8 @@ fn call<'a>( } else { None }; + accounts.insert(program_id_index, Rc::new(program_account.clone())); + account_refs.insert(program_id_index, None); let mut executable_accounts = vec![(callee_program_id, program_account)]; if let Some(programdata) = programdata_executable { executable_accounts.push(programdata); @@ -1446,37 +1462,40 @@ fn call<'a>( } // Copy results back to caller - for (i, (account, account_ref)) in accounts.iter().zip(account_refs).enumerate() { let account = account.borrow(); - if message.is_writable(i) && !account.executable { - *account_ref.lamports = account.lamports; - *account_ref.owner = account.owner; - if account_ref.data.len() != account.data.len() { - if !account_ref.data.is_empty() { - // Only support for `CreateAccount` at this time. - // Need a way to limit total realloc size across multiple CPI calls - return Err( - SyscallError::InstructionError(InstructionError::InvalidRealloc).into(), - ); + if let Some(account_ref) = account_ref { + if message.is_writable(i) && !account.executable { + *account_ref.lamports = account.lamports; + *account_ref.owner = account.owner; + if account_ref.data.len() != account.data.len() { + if !account_ref.data.is_empty() { + // Only support for `CreateAccount` at this time. + // Need a way to limit total realloc size across multiple CPI calls + return Err(SyscallError::InstructionError( + InstructionError::InvalidRealloc, + ) + .into()); + } + if account.data.len() > account_ref.data.len() + MAX_PERMITTED_DATA_INCREASE { + return Err(SyscallError::InstructionError( + InstructionError::InvalidRealloc, + ) + .into()); + } + let _ = translate( + memory_mapping, + AccessType::Store, + account_ref.vm_data_addr, + account.data.len() as u64, + )?; + *account_ref.ref_to_len_in_vm = account.data.len() as u64; + *account_ref.serialized_len_ptr = account.data.len() as u64; } - if account.data.len() > account_ref.data.len() + MAX_PERMITTED_DATA_INCREASE { - return Err( - SyscallError::InstructionError(InstructionError::InvalidRealloc).into(), - ); - } - let _ = translate( - memory_mapping, - AccessType::Store, - account_ref.vm_data_addr, - account.data.len() as u64, - )?; - *account_ref.ref_to_len_in_vm = account.data.len() as u64; - *account_ref.serialized_len_ptr = account.data.len() as u64; + account_ref + .data + .clone_from_slice(&account.data[0..account_ref.data.len()]); } - account_ref - .data - .clone_from_slice(&account.data[0..account_ref.data.len()]); } } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index bcc99ba2e1..9fce165e20 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -509,58 +509,11 @@ impl MessageProcessor { Err(InstructionError::UnsupportedProgramId) } - /// Verify instruction against the caller - pub fn verify_instruction( - keyed_accounts: &[&KeyedAccount], - instruction: &Instruction, - signers: &[Pubkey], - ) -> Result<(), 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(InstructionError::MissingAccount)?; - // Readonly account cannot become writable - if account.is_writable && !keyed_account.is_writable() { - 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 - ) { - return Err(InstructionError::PrivilegeEscalation); - } - } - - // validate the caller has access to the program account - let _ = keyed_accounts - .iter() - .find_map(|keyed_account| { - if &instruction.program_id == keyed_account.unsigned_key() { - Some(keyed_account) - } else { - None - } - }) - .ok_or(InstructionError::MissingAccount)?; - - Ok(()) - } - pub fn create_message( instruction: &Instruction, keyed_accounts: &[&KeyedAccount], signers: &[Pubkey], - ) -> Result<(Message, Pubkey), InstructionError> { + ) -> Result<(Message, Pubkey, usize), InstructionError> { // Check for privilege escalation for account in instruction.accounts.iter() { let keyed_account = keyed_accounts @@ -588,6 +541,7 @@ impl MessageProcessor { } // validate the caller has access to the program account + let program_id = instruction.program_id; let _ = keyed_accounts .iter() .find_map(|keyed_account| { @@ -600,10 +554,9 @@ impl MessageProcessor { .ok_or(InstructionError::MissingAccount)?; let message = Message::new(&[instruction.clone()], None); - let id = *message - .program_id(0) - .ok_or(InstructionError::MissingAccount)?; - Ok((message, id)) + 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 @@ -621,7 +574,7 @@ impl MessageProcessor { .iter() .map(|seeds| Pubkey::create_program_address(&seeds, caller_program_id)) .collect::, solana_sdk::pubkey::PubkeyError>>()?; - let (message, callee_program_id) = + let (message, callee_program_id, _) = Self::create_message(&instruction, &keyed_accounts, &signers)?; let mut accounts = vec![]; let mut account_refs = vec![];