Fix duplicate account translation in CPI (#23701)

* Use visit_each_account_once() in CPI syscall to prevent duplicate accounts from being translated twice.

* remove unwrap() from runtime

Co-authored-by: Jack May <jack@solana.com>

program-runtime/src/invoke_context.rs

@@ -352,7 +352,11 @@ impl<'a> InvokeContext<'a> {
                 }
                 Err(InstructionError::MissingAccount)
             };
-            visit_each_account_once(instruction_accounts, &mut work)?;
+            visit_each_account_once(
+                instruction_accounts,
+                &mut work,
+                InstructionError::NotEnoughAccountKeys,
+            )?;
         } else {
             let contains = (0..self
                 .transaction_context
@@ -517,7 +521,11 @@ impl<'a> InvokeContext<'a> {
 
             Ok(())
         };
-        visit_each_account_once(instruction_accounts, &mut work)?;
+        visit_each_account_once(
+            instruction_accounts,
+            &mut work,
+            InstructionError::NotEnoughAccountKeys,
+        )?;
 
         // Verify that the total sum of all the lamports did not change
         if pre_sum != post_sum {
@@ -615,7 +623,11 @@ impl<'a> InvokeContext<'a> {
             }
             Err(InstructionError::MissingAccount)
         };
-        visit_each_account_once(instruction_accounts, &mut work)?;
+        visit_each_account_once(
+            instruction_accounts,
+            &mut work,
+            InstructionError::NotEnoughAccountKeys,
+        )?;
 
         // Verify that the total sum of all the lamports did not change
         if pre_sum != post_sum {
@@ -1154,24 +1166,26 @@ pub fn mock_process_instruction(
 }
 
 /// Visit each unique instruction account index once
-fn visit_each_account_once(
+pub fn visit_each_account_once<E>(
     instruction_accounts: &[InstructionAccount],
-    work: &mut dyn FnMut(usize, &InstructionAccount) -> Result<(), InstructionError>,
+    work: &mut dyn FnMut(usize, &InstructionAccount) -> Result<(), E>,
-) -> Result<(), InstructionError> {
+    inner_error: E,
-    'root: for (index, instruction_account) in instruction_accounts.iter().enumerate() {
+) -> Result<(), E> {
     // Note: This is an O(n^2) algorithm,
     // but performed on a very small slice and requires no heap allocations
-        for before in instruction_accounts
+    'root: for (index, instruction_account) in instruction_accounts.iter().enumerate() {
-            .get(..index)
+        match instruction_accounts.get(..index) {
-            .ok_or(InstructionError::NotEnoughAccountKeys)?
+            Some(range) => {
-            .iter()
+                for before in range.iter() {
-        {
                     if before.index_in_transaction == instruction_account.index_in_transaction {
                         continue 'root; // skip dups
                     }
                 }
                 work(index, instruction_account)?;
             }
+            None => return Err(inner_error),
+        }
+    }
     Ok(())
 }
 
@@ -1211,7 +1225,8 @@ mod tests {
                 index_sum_b += entry.index_in_transaction;
                 Ok(())
             };
-            visit_each_account_once(accounts, &mut work).unwrap();
+            visit_each_account_once(accounts, &mut work, InstructionError::NotEnoughAccountKeys)
+                .unwrap();
 
             (unique_entries, index_sum_a, index_sum_b)
         };

programs/bpf_loader/src/syscalls.rs

@@ -4,7 +4,7 @@ use {
     alloc::Alloc,
     solana_program_runtime::{
         ic_logger_msg, ic_msg,
-        invoke_context::{ComputeMeter, InvokeContext},
+        invoke_context::{visit_each_account_once, ComputeMeter, InvokeContext},
         stable_log,
         timings::ExecuteTimings,
     },
@@ -2574,7 +2574,9 @@ where
         ))?;
     accounts.push((*program_account_index, None));
 
-    for instruction_account in instruction_accounts.iter() {
+    visit_each_account_once::<EbpfError<BpfError>>(
+        instruction_accounts,
+        &mut |_index: usize, instruction_account: &InstructionAccount| {
             let account = invoke_context
                 .transaction_context
                 .get_account_at_index(instruction_account.index_in_transaction)
@@ -2617,9 +2619,10 @@ where
                             "Internal error: index mismatch for account {}",
                             account_key
                         );
-                    return Err(
+                        return Err(SyscallError::InstructionError(
-                        SyscallError::InstructionError(InstructionError::MissingAccount).into(),
+                            InstructionError::MissingAccount,
-                    );
+                        )
+                        .into());
                     }
 
                     Some(caller_account)
@@ -2633,9 +2636,14 @@ where
                     "Instruction references an unknown account {}",
                     account_key
                 );
-            return Err(SyscallError::InstructionError(InstructionError::MissingAccount).into());
+                return Err(
-        }
+                    SyscallError::InstructionError(InstructionError::MissingAccount).into(),
+                );
             }
+            Ok(())
+        },
+        SyscallError::InstructionError(InstructionError::NotEnoughAccountKeys).into(),
+    )?;
 
     Ok(accounts)
 }