diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 31f9b517e1..5f0ad18007 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4073,6 +4073,7 @@ mod tests { create_genesis_config_with_leader, create_genesis_config_with_vote_accounts, GenesisConfigInfo, ValidatorVoteKeypairs, BOOTSTRAP_VALIDATOR_LAMPORTS, }, + native_loader::NativeLoaderError, process_instruction::InvokeContext, status_cache::MAX_CACHE_ENTRIES, }; @@ -9888,4 +9889,85 @@ mod tests { "<< Transaction log truncated >>\n" ); } + + #[test] + fn test_program_is_native_loader() { + let (genesis_config, mint_keypair) = create_genesis_config(50000); + let bank = Bank::new(&genesis_config); + + let tx = Transaction::new_signed_with_payer( + &[Instruction::new(native_loader::id(), &(), vec![])], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + bank.last_blockhash(), + ); + assert_eq!( + bank.process_transaction(&tx), + Err(TransactionError::InstructionError( + 0, + InstructionError::UnsupportedProgramId + )) + ); + } + + #[test] + fn test_bad_native_loader() { + let (genesis_config, mint_keypair) = create_genesis_config(50000); + let bank = Bank::new(&genesis_config); + let to_keypair = Keypair::new(); + + let tx = Transaction::new_signed_with_payer( + &[ + system_instruction::create_account( + &mint_keypair.pubkey(), + &to_keypair.pubkey(), + 10000, + 0, + &native_loader::id(), + ), + Instruction::new( + native_loader::id(), + &(), + vec![AccountMeta::new(to_keypair.pubkey(), false)], + ), + ], + Some(&mint_keypair.pubkey()), + &[&mint_keypair, &to_keypair], + bank.last_blockhash(), + ); + assert_eq!( + bank.process_transaction(&tx), + Err(TransactionError::InstructionError( + 1, + InstructionError::Custom(NativeLoaderError::InvalidAccountData as u32) + )) + ); + + let tx = Transaction::new_signed_with_payer( + &[ + system_instruction::create_account( + &mint_keypair.pubkey(), + &to_keypair.pubkey(), + 10000, + 100, + &native_loader::id(), + ), + Instruction::new( + native_loader::id(), + &(), + vec![AccountMeta::new(to_keypair.pubkey(), false)], + ), + ], + Some(&mint_keypair.pubkey()), + &[&mint_keypair, &to_keypair], + bank.last_blockhash(), + ); + assert_eq!( + bank.process_transaction(&tx), + Err(TransactionError::InstructionError( + 1, + InstructionError::Custom(NativeLoaderError::InvalidAccountData as u32) + )) + ); + } } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 05730239a0..e0d0e723b9 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -454,43 +454,49 @@ impl MessageProcessor { instruction_data: &[u8], invoke_context: &mut dyn InvokeContext, ) -> Result<(), InstructionError> { - if native_loader::check_id(&keyed_accounts[0].owner()?) { - let root_id = keyed_accounts[0].unsigned_key(); - for (id, process_instruction) in &self.loaders { - if id == root_id { - // Call the program via a builtin loader - return process_instruction( - &root_id, - &keyed_accounts[1..], - instruction_data, - invoke_context, - ); + if let Some(root_account) = keyed_accounts.iter().next() { + if native_loader::check_id(&root_account.owner()?) { + let root_id = root_account.unsigned_key(); + for (id, process_instruction) in &self.loaders { + if id == root_id { + // Call the program via a builtin loader + return process_instruction( + &root_id, + &keyed_accounts[1..], + instruction_data, + invoke_context, + ); + } } - } - for (id, process_instruction) in &self.programs { - if id == root_id { - // Call the builtin program - return process_instruction(&root_id, &keyed_accounts[1..], instruction_data); + for (id, process_instruction) in &self.programs { + if id == root_id { + // Call the builtin program + return process_instruction( + &root_id, + &keyed_accounts[1..], + instruction_data, + ); + } } - } - // Call the program via the native loader - return self.native_loader.process_instruction( - &native_loader::id(), - keyed_accounts, - instruction_data, - invoke_context, - ); - } else { - let owner_id = &keyed_accounts[0].owner()?; - for (id, process_instruction) in &self.loaders { - if id == owner_id { - // Call the program via a builtin loader - return process_instruction( - &owner_id, - keyed_accounts, - instruction_data, - invoke_context, - ); + // Call the program via the native loader + return self.native_loader.process_instruction( + &native_loader::id(), + keyed_accounts, + instruction_data, + invoke_context, + ); + } else { + let owner_id = &root_account.owner()?; + for (id, process_instruction) in &self.loaders { + if id == owner_id { + // Call the program via a builtin loader + return process_instruction( + &owner_id, + keyed_accounts, + instruction_data, + invoke_context, + ); + } } } } @@ -506,26 +512,29 @@ impl MessageProcessor { accounts: &[Rc>], invoke_context: &mut dyn InvokeContext, ) -> Result<(), InstructionError> { - let instruction = &message.instructions[0]; + if let Some(instruction) = message.instructions.get(0) { + // Verify the calling program hasn't misbehaved + invoke_context.verify_and_update(message, instruction, accounts)?; - // Verify the calling program hasn't misbehaved - invoke_context.verify_and_update(message, instruction, accounts)?; + // Construct keyed accounts + let keyed_accounts = + Self::create_keyed_accounts(message, instruction, executable_accounts, accounts)?; - // Construct keyed accounts - let keyed_accounts = - Self::create_keyed_accounts(message, instruction, executable_accounts, accounts)?; + // Invoke callee + invoke_context.push(instruction.program_id(&message.account_keys))?; + let mut result = + self.process_instruction(&keyed_accounts, &instruction.data, invoke_context); + if result.is_ok() { + // Verify the called program has not misbehaved + result = invoke_context.verify_and_update(message, instruction, accounts); + } + invoke_context.pop(); - // Invoke callee - invoke_context.push(instruction.program_id(&message.account_keys))?; - let mut result = - self.process_instruction(&keyed_accounts, &instruction.data, invoke_context); - if result.is_ok() { - // Verify the called program has not misbehaved - result = invoke_context.verify_and_update(message, instruction, accounts); + result + } else { + // This function is always called with a valid instruction, if that changes return an error + Err(InstructionError::GenericError) } - invoke_context.pop(); - - result } /// Record the initial state of the accounts so that they can be compared diff --git a/runtime/src/native_loader.rs b/runtime/src/native_loader.rs index 8d4a817c12..8f8188ea3d 100644 --- a/runtime/src/native_loader.rs +++ b/runtime/src/native_loader.rs @@ -19,7 +19,7 @@ use thiserror::Error; #[derive(Error, Debug, Serialize, Clone, PartialEq, FromPrimitive, ToPrimitive)] pub enum NativeLoaderError { #[error("Entrypoint name in the account data is not a valid UTF-8 string")] - InvalidEntrypointName = 0x0aaa_0001, + InvalidAccountData = 0x0aaa_0001, #[error("Entrypoint was not found in the module")] EntrypointNotFound = 0x0aaa_0002, #[error("Failed to load the module")] @@ -56,16 +56,18 @@ pub struct NativeLoader { loader_symbol_cache: LoaderSymbolCache, } impl NativeLoader { - fn create_path(name: &str) -> PathBuf { - let current_exe = env::current_exe().unwrap_or_else(|e| { - panic!("create_path(\"{}\"): current exe not found: {:?}", name, e) - }); - let current_exe_directory = PathBuf::from(current_exe.parent().unwrap_or_else(|| { - panic!( + fn create_path(name: &str) -> Result { + let current_exe = env::current_exe().map_err(|e| { + error!("create_path(\"{}\"): current exe not found: {:?}", name, e); + InstructionError::from(NativeLoaderError::EntrypointNotFound) + })?; + let current_exe_directory = PathBuf::from(current_exe.parent().ok_or_else(|| { + error!( "create_path(\"{}\"): no parent directory of {:?}", - name, current_exe, - ) - })); + name, current_exe + ); + InstructionError::from(NativeLoaderError::FailedToLoad) + })?); let library_file_name = PathBuf::from(PLATFORM_FILE_PREFIX.to_string() + name) .with_extension(PLATFORM_FILE_EXTENSION); @@ -74,10 +76,10 @@ impl NativeLoader { // from the deps/ subdirectory let file_path = current_exe_directory.join(&library_file_name); if file_path.exists() { - file_path + Ok(file_path) } else { // `cargo build` places dependent libraries in the deps/ subdirectory - current_exe_directory.join("deps").join(library_file_name) + Ok(current_exe_directory.join("deps").join(library_file_name)) } } @@ -100,7 +102,7 @@ impl NativeLoader { if let Some(entrypoint) = cache.get(name) { Ok(entrypoint.clone()) } else { - match Self::library_open(&Self::create_path(&name)) { + match Self::library_open(&Self::create_path(&name)?) { Ok(library) => { let result = unsafe { library.get::(name.as_bytes()) }; match result { @@ -109,12 +111,14 @@ impl NativeLoader { Ok(entrypoint) } Err(e) => { - panic!("Unable to find program entrypoint in {:?}: {:?})", name, e); + error!("Unable to find program entrypoint in {:?}: {:?})", name, e); + Err(NativeLoaderError::EntrypointNotFound.into()) } } } Err(e) => { - panic!("Failed to load: {:?}", e); + error!("Failed to load: {:?}", e); + Err(NativeLoaderError::FailedToLoad.into()) } } } @@ -134,9 +138,14 @@ impl NativeLoader { let name = match str::from_utf8(name_vec) { Ok(v) => v, Err(e) => { - panic!("Invalid UTF-8 sequence: {}", e); + error!("Invalid UTF-8 sequence: {}", e); + return Err(NativeLoaderError::InvalidAccountData.into()); } }; + if name.is_empty() || name.starts_with('\0') { + error!("Empty name string"); + return Err(NativeLoaderError::InvalidAccountData.into()); + } trace!("Call native {:?}", name); if name.ends_with("loader_program") { let entrypoint =