diff --git a/program-runtime/src/instruction_processor.rs b/program-runtime/src/instruction_processor.rs index 49c6381be1e4b2..bb761ae0b47b18 100644 --- a/program-runtime/src/instruction_processor.rs +++ b/program-runtime/src/instruction_processor.rs @@ -498,27 +498,28 @@ impl InstructionProcessor { 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()?; + let (message, callee_program_id, _) = Self::create_message( + &instruction, + &callee_keyed_accounts, + signers, + &invoke_context, + )?; + + let mut accounts = Vec::with_capacity(message.account_keys.len()); + let mut caller_write_privileges = Vec::with_capacity(message.account_keys.len()); + let mut keyed_account_indices_reordered = + Vec::with_capacity(message.account_keys.len()); '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]; + 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; } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 864a196a71b67f..9a1b1dd4ab73cf 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -1127,6 +1127,7 @@ mod tests { NoopFail, ModifyOwned, ModifyNotOwned, + ModifyReadonly, } fn mock_process_instruction( @@ -1151,6 +1152,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); @@ -1170,6 +1174,7 @@ mod tests { 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); #[allow(unused_mut)] let accounts = vec![ @@ -1181,23 +1186,28 @@ mod tests { 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 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()); @@ -1208,7 +1218,7 @@ mod tests { &caller_program_id, Rent::default(), &message, - &compiled_instruction, + &caller_instruction, &executable_accounts, &accounts, programs.as_slice(), @@ -1244,6 +1254,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, + &executable_accounts, + &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(())), ( @@ -1258,9 +1282,9 @@ 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 blockhash = Hash::default(); @@ -1269,7 +1293,7 @@ mod tests { &caller_program_id, Rent::default(), &message, - &compiled_instruction, + &caller_instruction, &executable_accounts, &accounts, programs.as_slice(), @@ -1303,4 +1327,200 @@ mod tests { ); } } + + #[test] + 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 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); + + #[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 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, + &executable_accounts, + &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, + ); + + // 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, + &executable_accounts, + &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, + ); + + assert_eq!( + InstructionProcessor::native_invoke( + &mut invoke_context, + callee_instruction, + &[0, 1, 2, 3], + &[] + ), + case.1 + ); + } + } }