From 0dc364c17a7ff7d2bbf0a52de8e5f4c0341cf907 Mon Sep 17 00:00:00 2001
From: Michael Vines <mvines@gmail.com>
Date: Tue, 19 Mar 2019 18:03:07 -0700
Subject: [PATCH] Relocate transaction reference verification to join the other
 validity checks

---
 core/src/banking_stage.rs |  2 +-
 runtime/src/accounts.rs   |  1 +
 runtime/src/bank.rs       | 56 ++++++++++++++++++++++++++++++++++++++-
 sdk/src/transaction.rs    |  3 +++
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs
index 2b9a55df7..54b040f03 100644
--- a/core/src/banking_stage.rs
+++ b/core/src/banking_stage.rs
@@ -388,7 +388,7 @@ impl BankingStage {
                     .filter_map(|((tx, ver), index)| match tx {
                         None => None,
                         Some(tx) => {
-                            if tx.verify_refs() && ver != 0 {
+                            if ver != 0 {
                                 Some((tx, index))
                             } else {
                                 None
diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs
index 78e5f20fb..0344ce22a 100644
--- a/runtime/src/accounts.rs
+++ b/runtime/src/accounts.rs
@@ -32,6 +32,7 @@ pub struct ErrorCounters {
     pub blockhash_too_old: usize,
     pub reserve_blockhash: usize,
     pub insufficient_funds: usize,
+    pub invalid_account_index: usize,
     pub duplicate_signature: usize,
     pub call_chain_too_deep: usize,
     pub missing_signature_for_fee: usize,
diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs
index 2fba6fa3b..0f8c300eb 100644
--- a/runtime/src/bank.rs
+++ b/runtime/src/bank.rs
@@ -467,6 +467,24 @@ impl Bank {
         self.accounts
             .load_accounts(self.accounts_id, txs, results, error_counters)
     }
+    fn check_refs(
+        &self,
+        txs: &[Transaction],
+        lock_results: Vec<Result<()>>,
+        error_counters: &mut ErrorCounters,
+    ) -> Vec<Result<()>> {
+        txs.iter()
+            .zip(lock_results.into_iter())
+            .map(|(tx, lock_res)| {
+                if lock_res.is_ok() && !tx.verify_refs() {
+                    error_counters.invalid_account_index += 1;
+                    Err(TransactionError::InvalidAccountIndex)
+                } else {
+                    lock_res
+                }
+            })
+            .collect()
+    }
     fn check_age(
         &self,
         txs: &[Transaction],
@@ -524,7 +542,8 @@ impl Bank {
         debug!("processing transactions: {}", txs.len());
         let mut error_counters = ErrorCounters::default();
         let now = Instant::now();
-        let age_results = self.check_age(txs, lock_results, max_age, &mut error_counters);
+        let refs_results = self.check_refs(txs, lock_results, &mut error_counters);
+        let age_results = self.check_age(txs, refs_results, max_age, &mut error_counters);
         let sig_results = self.check_signatures(txs, age_results, &mut error_counters);
         let mut loaded_accounts = self.load_accounts(txs, sig_results, &mut error_counters);
         let tick_height = self.tick_height();
@@ -582,6 +601,12 @@ impl Bank {
                 error_counters.blockhash_not_found
             );
         }
+        if 0 != error_counters.invalid_account_index {
+            inc_new_counter_info!(
+                "bank-process_transactions-error-invalid_account_index",
+                error_counters.invalid_account_index
+            );
+        }
         if 0 != error_counters.reserve_blockhash {
             inc_new_counter_info!(
                 "bank-process_transactions-error-reserve_blockhash",
@@ -1275,6 +1300,35 @@ mod tests {
             .is_ok());
     }
 
+    #[test]
+    fn test_bank_invalid_account_index() {
+        let (genesis_block, mint_keypair) = GenesisBlock::new(1);
+        let keypair = Keypair::new();
+        let bank = Bank::new(&genesis_block);
+
+        let tx = SystemTransaction::new_move(
+            &mint_keypair,
+            &keypair.pubkey(),
+            1,
+            genesis_block.hash(),
+            0,
+        );
+
+        let mut tx_invalid_program_index = tx.clone();
+        tx_invalid_program_index.instructions[0].program_ids_index = 42;
+        assert_eq!(
+            bank.process_transaction(&tx_invalid_program_index),
+            Err(TransactionError::InvalidAccountIndex)
+        );
+
+        let mut tx_invalid_account_index = tx.clone();
+        tx_invalid_account_index.instructions[0].accounts[0] = 42;
+        assert_eq!(
+            bank.process_transaction(&tx_invalid_account_index),
+            Err(TransactionError::InvalidAccountIndex)
+        );
+    }
+
     #[test]
     fn test_bank_pay_to_self() {
         let (genesis_block, mint_keypair) = GenesisBlock::new(1);
diff --git a/sdk/src/transaction.rs b/sdk/src/transaction.rs
index 9e3c85256..a89027f8c 100644
--- a/sdk/src/transaction.rs
+++ b/sdk/src/transaction.rs
@@ -189,6 +189,9 @@ pub enum TransactionError {
 
     /// Transaction has a fee but has no signature present
     MissingSignatureForFee,
+
+    /// Transaction contains an invalid account reference
+    InvalidAccountIndex,
 }
 
 /// An atomic transaction