diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index cdf97990e3..aa40ccbfdf 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -1666,3 +1666,45 @@ fn test_program_bpf_invoke_upgradeable_via_cpi() { TransactionError::InstructionError(0, InstructionError::Custom(42)) ); } + +#[test] +#[cfg(any(feature = "bpf_c", feature = "bpf_rust"))] +fn test_program_bpf_disguised_as_bpf_loader() { + solana_logger::setup(); + + let mut programs = Vec::new(); + #[cfg(feature = "bpf_c")] + { + programs.extend_from_slice(&[("noop")]); + } + #[cfg(feature = "bpf_rust")] + { + programs.extend_from_slice(&[("noop")]); + } + + for program in programs.iter() { + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(50); + let mut bank = Bank::new(&genesis_config); + let (name, id, entrypoint) = solana_bpf_loader_deprecated_program!(); + bank.add_builtin(&name, id, entrypoint); + let bank_client = BankClient::new(bank); + + let program_id = load_bpf_program( + &bank_client, + &bpf_loader_deprecated::id(), + &mint_keypair, + program, + ); + let account_metas = vec![AccountMeta::new_readonly(program_id, false)]; + let instruction = Instruction::new(bpf_loader_deprecated::id(), &1u8, account_metas); + let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); + assert_eq!( + result.unwrap_err().unwrap(), + TransactionError::InstructionError(0, InstructionError::IncorrectProgramId) + ); + } +} diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 6f37cf96c3..ea4b940e61 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -153,6 +153,12 @@ fn write_program_data( Ok(()) } +fn check_loader_id(id: &Pubkey) -> bool { + bpf_loader::check_id(id) + || bpf_loader_deprecated::check_id(id) + || bpf_loader_upgradeable::check_id(id) +} + /// Default program heap size, allocators /// are expected to enforce this const DEFAULT_HEAP_SIZE: usize = 32 * 1024; @@ -217,41 +223,39 @@ fn process_instruction_common( ) -> Result<(), InstructionError> { let logger = invoke_context.get_logger(); - if !(bpf_loader::check_id(program_id) - || bpf_loader_deprecated::check_id(program_id) - || bpf_loader_upgradeable::check_id(program_id)) - { - log!(logger, "Invalid BPF loader id"); - return Err(InstructionError::IncorrectProgramId); - } - let account_iter = &mut keyed_accounts.iter(); let first_account = next_keyed_account(account_iter)?; if first_account.executable()? { - let (program, keyed_accounts, offset) = if bpf_loader_upgradeable::check_id(program_id) { - if let UpgradeableLoaderState::Program { - programdata_address, - } = first_account.state()? - { - let programdata = next_keyed_account(account_iter)?; - if programdata_address != *programdata.unsigned_key() { - log!(logger, "Wrong ProgramData account for this Program account"); - return Err(InstructionError::InvalidArgument); - } - ( - programdata, - &keyed_accounts[1..], - UpgradeableLoaderState::programdata_data_offset()?, - ) - } else { - log!(logger, "Invalid Program account"); - return Err(InstructionError::InvalidAccountData); - } - } else { - (first_account, keyed_accounts, 0) - }; + if first_account.unsigned_key() != program_id { + log!(logger, "Program id mismatch"); + return Err(InstructionError::IncorrectProgramId); + } - if program.owner()? != *program_id { + let (program, keyed_accounts, offset) = + if bpf_loader_upgradeable::check_id(&first_account.owner()?) { + if let UpgradeableLoaderState::Program { + programdata_address, + } = first_account.state()? + { + let programdata = next_keyed_account(account_iter)?; + if programdata_address != *programdata.unsigned_key() { + log!(logger, "Wrong ProgramData account for this Program account"); + return Err(InstructionError::InvalidArgument); + } + ( + programdata, + &keyed_accounts[1..], + UpgradeableLoaderState::programdata_data_offset()?, + ) + } else { + log!(logger, "Invalid Program account"); + return Err(InstructionError::InvalidAccountData); + } + } else { + (first_account, keyed_accounts, 0) + }; + + if !check_loader_id(&program.owner()?) { log!(logger, "Executable account not owned by the BPF loader"); return Err(InstructionError::IncorrectProgramId); } @@ -266,28 +270,35 @@ fn process_instruction_common( )?, }; executor.execute( - program_id, + &program.owner()?, keyed_accounts, instruction_data, invoke_context, use_jit, )? - } else if bpf_loader_upgradeable::check_id(program_id) { - process_loader_upgradeable_instruction( - program_id, - keyed_accounts, - instruction_data, - invoke_context, - use_jit, - )?; } else { - process_loader_instruction( - program_id, - keyed_accounts, - instruction_data, - invoke_context, - use_jit, - )?; + if !check_loader_id(program_id) { + log!(logger, "Invalid BPF loader id"); + return Err(InstructionError::IncorrectProgramId); + } + + if bpf_loader_upgradeable::check_id(program_id) { + process_loader_upgradeable_instruction( + program_id, + keyed_accounts, + instruction_data, + invoke_context, + use_jit, + )?; + } else { + process_loader_instruction( + program_id, + keyed_accounts, + instruction_data, + invoke_context, + use_jit, + )?; + } } Ok(()) } @@ -961,20 +972,20 @@ mod tests { // Case: Empty keyed accounts assert_eq!( Err(InstructionError::NotEnoughAccountKeys), - process_instruction(&bpf_loader::id(), &[], &[], &mut invoke_context) + process_instruction(&program_id, &[], &[], &mut invoke_context) ); // Case: Only a program account assert_eq!( Ok(()), - process_instruction(&bpf_loader::id(), &keyed_accounts, &[], &mut invoke_context) + process_instruction(&program_key, &keyed_accounts, &[], &mut invoke_context) ); - // Case: Account not program + // Case: Account not a program keyed_accounts[0].account.borrow_mut().executable = false; assert_eq!( Err(InstructionError::InvalidInstructionData), - process_instruction(&bpf_loader::id(), &keyed_accounts, &[], &mut invoke_context) + process_instruction(&program_id, &keyed_accounts, &[], &mut invoke_context) ); keyed_accounts[0].account.borrow_mut().executable = true; @@ -983,7 +994,7 @@ mod tests { keyed_accounts.push(KeyedAccount::new(&program_key, false, ¶meter_account)); assert_eq!( Ok(()), - process_instruction(&bpf_loader::id(), &keyed_accounts, &[], &mut invoke_context) + process_instruction(&program_key, &keyed_accounts, &[], &mut invoke_context) ); // Case: limited budget @@ -1014,7 +1025,7 @@ mod tests { ); assert_eq!( Err(InstructionError::ProgramFailedToComplete), - process_instruction(&bpf_loader::id(), &keyed_accounts, &[], &mut invoke_context) + process_instruction(&program_key, &keyed_accounts, &[], &mut invoke_context) ); // Case: With duplicate accounts @@ -1026,7 +1037,7 @@ mod tests { assert_eq!( Ok(()), process_instruction( - &bpf_loader::id(), + &program_key, &keyed_accounts, &[], &mut MockInvokeContext::default() @@ -1054,7 +1065,7 @@ mod tests { assert_eq!( Ok(()), process_instruction( - &bpf_loader_deprecated::id(), + &program_key, &keyed_accounts, &[], &mut MockInvokeContext::default() @@ -1070,7 +1081,7 @@ mod tests { assert_eq!( Ok(()), process_instruction( - &bpf_loader_deprecated::id(), + &program_key, &keyed_accounts, &[], &mut MockInvokeContext::default() @@ -1098,7 +1109,7 @@ mod tests { assert_eq!( Ok(()), process_instruction( - &bpf_loader::id(), + &program_key, &keyed_accounts, &[], &mut MockInvokeContext::default() @@ -1114,7 +1125,7 @@ mod tests { assert_eq!( Ok(()), process_instruction( - &bpf_loader::id(), + &program_key, &keyed_accounts, &[], &mut MockInvokeContext::default() diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 9fce165e20..497eaa9d7f 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -466,18 +466,19 @@ impl MessageProcessor { /// This method calls the instruction's program entrypoint method fn process_instruction( &self, + program_id: &Pubkey, keyed_accounts: &[KeyedAccount], instruction_data: &[u8], invoke_context: &mut dyn InvokeContext, ) -> Result<(), InstructionError> { if let Some(root_account) = keyed_accounts.iter().next() { + let root_id = root_account.unsigned_key(); if native_loader::check_id(&root_account.owner()?) { - let root_id = root_account.unsigned_key(); for (id, process_instruction) in &self.programs { if id == root_id { // Call the builtin program return process_instruction( - &root_id, + &program_id, &keyed_accounts[1..], instruction_data, invoke_context, @@ -486,7 +487,7 @@ impl MessageProcessor { } // Call the program via the native loader return self.native_loader.process_instruction( - &native_loader::id(), + program_id, keyed_accounts, instruction_data, invoke_context, @@ -497,7 +498,7 @@ impl MessageProcessor { if id == owner_id { // Call the program via a builtin loader return process_instruction( - &owner_id, + &program_id, keyed_accounts, instruction_data, invoke_context, @@ -656,6 +657,8 @@ impl MessageProcessor { 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(message, instruction, accounts)?; @@ -664,7 +667,7 @@ impl MessageProcessor { Self::create_keyed_accounts(message, instruction, executable_accounts, accounts); // Invoke callee - invoke_context.push(instruction.program_id(&message.account_keys))?; + invoke_context.push(program_id)?; let mut message_processor = MessageProcessor::default(); for (program_id, process_instruction) in invoke_context.get_programs().iter() { @@ -672,6 +675,7 @@ impl MessageProcessor { } let mut result = message_processor.process_instruction( + program_id, &keyed_accounts, &instruction.data, invoke_context, @@ -840,8 +844,9 @@ impl MessageProcessor { } let pre_accounts = Self::create_pre_accounts(message, instruction, accounts); + let program_id = instruction.program_id(&message.account_keys); let mut invoke_context = ThisInvokeContext::new( - instruction.program_id(&message.account_keys), + program_id, rent_collector.rent, pre_accounts, account_deps, @@ -854,7 +859,12 @@ impl MessageProcessor { ); let keyed_accounts = Self::create_keyed_accounts(message, instruction, executable_accounts, accounts); - self.process_instruction(&keyed_accounts, &instruction.data, &mut invoke_context)?; + self.process_instruction( + program_id, + &keyed_accounts, + &instruction.data, + &mut invoke_context, + )?; Self::verify( message, instruction,