From f6255c2f9e11d101a24819d6050f751dc847da9d Mon Sep 17 00:00:00 2001
From: Greg Fitzgerald <greg@solana.com>
Date: Tue, 15 Oct 2019 14:35:42 -0600
Subject: [PATCH] Fix blind keyed accounts indexing in Config program (#6369)

---
 programs/config_api/src/config_processor.rs   | 34 +++++++++----------
 .../config_tests/tests/config_processor.rs    | 25 ++++++++++++++
 2 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/programs/config_api/src/config_processor.rs b/programs/config_api/src/config_processor.rs
index 553bb8c54..f016d2dd0 100644
--- a/programs/config_api/src/config_processor.rs
+++ b/programs/config_api/src/config_processor.rs
@@ -5,6 +5,7 @@ use bincode::deserialize;
 use log::*;
 use solana_sdk::account::KeyedAccount;
 use solana_sdk::instruction::InstructionError;
+use solana_sdk::instruction_processor_utils::next_keyed_account;
 use solana_sdk::pubkey::Pubkey;
 
 pub fn process_instruction(
@@ -17,10 +18,13 @@ pub fn process_instruction(
         InstructionError::InvalidInstructionData
     })?;
 
-    let current_data: ConfigKeys = deserialize(&keyed_accounts[0].account.data).map_err(|err| {
-        error!("Invalid data in account[0]: {:?} {:?}", data, err);
-        InstructionError::InvalidAccountData
-    })?;
+    let keyed_accounts_iter = &mut keyed_accounts.iter_mut();
+    let config_keyed_account = &mut next_keyed_account(keyed_accounts_iter)?;
+    let current_data: ConfigKeys =
+        deserialize(&config_keyed_account.account.data).map_err(|err| {
+            error!("Invalid data in account[0]: {:?} {:?}", data, err);
+            InstructionError::InvalidAccountData
+        })?;
     let current_signer_keys: Vec<Pubkey> = current_data
         .keys
         .iter()
@@ -31,23 +35,17 @@ pub fn process_instruction(
     if current_signer_keys.is_empty() {
         // Config account keypair must be a signer on account initilization,
         // or when no signers specified in Config data
-        if keyed_accounts[0].signer_key().is_none() {
+        if config_keyed_account.signer_key().is_none() {
             error!("account[0].signer_key().is_none()");
             return Err(InstructionError::MissingRequiredSignature);
         }
     }
 
     let mut counter = 0;
-    for (i, (signer, _)) in key_list
-        .keys
-        .iter()
-        .filter(|(_, is_signer)| *is_signer)
-        .enumerate()
-    {
+    for (signer, _) in key_list.keys.iter().filter(|(_, is_signer)| *is_signer) {
         counter += 1;
-        if signer != keyed_accounts[0].unsigned_key() {
-            let account_index = i + 1;
-            let signer_account = keyed_accounts.get(account_index);
+        if signer != config_keyed_account.unsigned_key() {
+            let signer_account = keyed_accounts_iter.next();
             if signer_account.is_none() {
                 error!("account {:?} is not in account list", signer);
                 return Err(InstructionError::MissingRequiredSignature);
@@ -60,7 +58,7 @@ pub fn process_instruction(
             if signer_key.unwrap() != signer {
                 error!(
                     "account[{:?}].signer_key() does not match Config data)",
-                    account_index
+                    counter + 1
                 );
                 return Err(InstructionError::MissingRequiredSignature);
             }
@@ -74,7 +72,7 @@ pub fn process_instruction(
                 error!("account {:?} is not in stored signer list", signer);
                 return Err(InstructionError::MissingRequiredSignature);
             }
-        } else if keyed_accounts[0].signer_key().is_none() {
+        } else if config_keyed_account.signer_key().is_none() {
             error!("account[0].signer_key().is_none()");
             return Err(InstructionError::MissingRequiredSignature);
         }
@@ -90,12 +88,12 @@ pub fn process_instruction(
         return Err(InstructionError::MissingRequiredSignature);
     }
 
-    if keyed_accounts[0].account.data.len() < data.len() {
+    if config_keyed_account.account.data.len() < data.len() {
         error!("instruction data too large");
         return Err(InstructionError::InvalidInstructionData);
     }
 
-    keyed_accounts[0].account.data[0..data.len()].copy_from_slice(&data);
+    config_keyed_account.account.data[0..data.len()].copy_from_slice(&data);
     Ok(())
 }
 
diff --git a/programs/config_tests/tests/config_processor.rs b/programs/config_tests/tests/config_processor.rs
index 0a76b1062..78fbfb02a 100644
--- a/programs/config_tests/tests/config_processor.rs
+++ b/programs/config_tests/tests/config_processor.rs
@@ -10,10 +10,12 @@ use solana_runtime::{bank::Bank, bank_client::BankClient};
 use solana_sdk::{
     client::SyncClient,
     genesis_block::create_genesis_block,
+    instruction::InstructionError,
     message::Message,
     pubkey::Pubkey,
     signature::{Keypair, KeypairUtil},
     system_instruction,
+    transaction::TransactionError,
 };
 
 #[derive(Serialize, Deserialize, Debug, PartialEq)]
@@ -364,3 +366,26 @@ fn test_config_updates_requiring_config() {
         .send_message(&[&mint_keypair, &config_keypair], message)
         .unwrap_err();
 }
+
+#[test]
+fn test_config_initialize_no_panic() {
+    let (bank, alice_keypair) = create_bank(3);
+    let bank_client = BankClient::new(bank);
+
+    let mut instructions = config_instruction::create_account::<MyConfig>(
+        &alice_keypair.pubkey(),
+        &Pubkey::new_rand(),
+        1,
+        vec![],
+    );
+    instructions[1].accounts = vec![]; // <!-- Attack! Prevent accounts from being passed into processor.
+
+    let message = Message::new(instructions);
+    assert_eq!(
+        bank_client
+            .send_message(&[&alice_keypair], message)
+            .unwrap_err()
+            .unwrap(),
+        TransactionError::InstructionError(1, InstructionError::NotEnoughAccountKeys)
+    );
+}