fix native_loader behavior for invalid accounts (#12814)

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)
+            ))
+        );
+    }
 }

runtime/src/message_processor.rs

@@ -454,8 +454,9 @@ impl MessageProcessor {
         instruction_data: &[u8],
         invoke_context: &mut dyn InvokeContext,
     ) -> Result<(), InstructionError> {
-        if native_loader::check_id(&keyed_accounts[0].owner()?) {
+        if let Some(root_account) = keyed_accounts.iter().next() {
-            let root_id = keyed_accounts[0].unsigned_key();
+            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
@@ -470,7 +471,11 @@ impl MessageProcessor {
                 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);
+                        return process_instruction(
+                            &root_id,
+                            &keyed_accounts[1..],
+                            instruction_data,
+                        );
                     }
                 }
                 // Call the program via the native loader
@@ -481,7 +486,7 @@ impl MessageProcessor {
                     invoke_context,
                 );
             } else {
-            let owner_id = &keyed_accounts[0].owner()?;
+                let owner_id = &root_account.owner()?;
                 for (id, process_instruction) in &self.loaders {
                     if id == owner_id {
                         // Call the program via a builtin loader
@@ -494,6 +499,7 @@ impl MessageProcessor {
                     }
                 }
             }
+        }
         Err(InstructionError::UnsupportedProgramId)
     }
 
@@ -506,8 +512,7 @@ impl MessageProcessor {
         accounts: &[Rc<RefCell<Account>>],
         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)?;
 
@@ -526,6 +531,10 @@ impl MessageProcessor {
             invoke_context.pop();
 
             result
+        } else {
+            // This function is always called with a valid instruction, if that changes return an error
+            Err(InstructionError::GenericError)
+        }
     }
 
     /// Record the initial state of the accounts so that they can be compared

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 {
+    fn create_path(name: &str) -> Result<PathBuf, InstructionError> {
-        let current_exe = env::current_exe().unwrap_or_else(|e| {
+        let current_exe = env::current_exe().map_err(|e| {
-            panic!("create_path(\"{}\"): current exe not found: {:?}", name, e)
+            error!("create_path(\"{}\"): current exe not found: {:?}", name, e);
-        });
+            InstructionError::from(NativeLoaderError::EntrypointNotFound)
-        let current_exe_directory = PathBuf::from(current_exe.parent().unwrap_or_else(|| {
+        })?;
-            panic!(
+        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::<T>(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 =