From 7b8e5a9f47f9b4e2ed0024a2fee360b9ae0e24cc Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Wed, 5 Aug 2020 17:03:09 +0000
Subject: [PATCH] Sanitize preflight (bp #11373) (#11376)

* Add failing test for unsane tx in RPC preflight

(cherry picked from commit e25846e1adfeb33cce79ed48ed7b274091dc26e7)

* Add From for SanitizeError > TransactionError

(cherry picked from commit 3f73affb2e79acd8bcfd62604de7a1eb0798d1fc)

* Sanitize transactions during RPC preflight test

(cherry picked from commit 29b3265dc7c9dfa246f6e8afb99f2bd7feee59bd)

* Harden RPC preflight test inputs

(cherry picked from commit 14339dec0a960e8161d1165b6a8e5cfb73e78f23)

Co-authored-by: Trent Nelson <trent@solana.com>
---
 core/src/rpc.rs         | 22 ++++++++++++++++++----
 runtime/src/accounts.rs |  3 +--
 runtime/src/bank.rs     |  7 ++++++-
 sdk/src/transaction.rs  |  6 ++++++
 4 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/core/src/rpc.rs b/core/src/rpc.rs
index b73949e85c853c..7cd3d939529367 100644
--- a/core/src/rpc.rs
+++ b/core/src/rpc.rs
@@ -3475,7 +3475,7 @@ pub mod tests {
         let block_commitment_cache = Arc::new(RwLock::new(
             BlockCommitmentCache::default_with_blockstore(blockstore.clone()),
         ));
-        let bank_forks = new_bank_forks().0;
+        let (bank_forks, mint_keypair, ..) = new_bank_forks();
         let health = RpcHealth::stub();
 
         // Freeze bank 0 to prevent a panic in `run_transaction_simulation()`
@@ -3503,8 +3503,8 @@ pub mod tests {
             )),
         );
 
-        let bad_transaction =
-            system_transaction::transfer(&Keypair::new(), &Pubkey::default(), 42, Hash::default());
+        let mut bad_transaction =
+            system_transaction::transfer(&mint_keypair, &Pubkey::new_rand(), 42, Hash::default());
 
         // sendTransaction will fail because the blockhash is invalid
         let req = format!(
@@ -3519,9 +3519,23 @@ pub mod tests {
             )
         );
 
+        // sendTransaction will fail due to insanity
+        bad_transaction.message.instructions[0].program_id_index = 255u8;
         let recent_blockhash = bank_forks.read().unwrap().root_bank().last_blockhash();
+        bad_transaction.sign(&[&mint_keypair], recent_blockhash);
+        let req = format!(
+            r#"{{"jsonrpc":"2.0","id":1,"method":"sendTransaction","params":["{}"]}}"#,
+            bs58::encode(serialize(&bad_transaction).unwrap()).into_string()
+        );
+        let res = io.handle_request_sync(&req, meta.clone());
+        assert_eq!(
+            res,
+            Some(
+                r#"{"jsonrpc":"2.0","error":{"code":-32002,"message":"Transaction simulation failed: TransactionError::SanitizeFailure"},"id":1}"#.to_string(),
+            )
+        );
         let mut bad_transaction =
-            system_transaction::transfer(&Keypair::new(), &Pubkey::default(), 42, recent_blockhash);
+            system_transaction::transfer(&mint_keypair, &Pubkey::new_rand(), 42, recent_blockhash);
 
         // sendTransaction will fail due to poor node health
         health.stub_set_health_status(Some(RpcHealthStatus::Behind));
diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs
index c4aa04047d41b4..42db83b9bc86c4 100644
--- a/runtime/src/accounts.rs
+++ b/runtime/src/accounts.rs
@@ -629,8 +629,7 @@ impl Accounts {
         use solana_sdk::sanitize::Sanitize;
         let keys: Vec<Result<_>> = OrderedIterator::new(txs, txs_iteration_order)
             .map(|tx| {
-                tx.sanitize()
-                    .map_err(|_| TransactionError::SanitizeFailure)?;
+                tx.sanitize().map_err(TransactionError::from)?;
 
                 if Self::has_duplicates(&tx.message.account_keys) {
                     return Err(TransactionError::AccountLoadedTwice);
diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs
index 9267a0c0186c1f..8ecf956db1c907 100644
--- a/runtime/src/bank.rs
+++ b/runtime/src/bank.rs
@@ -48,6 +48,7 @@ use solana_sdk::{
     native_loader, nonce,
     program_utils::limited_deserialize,
     pubkey::Pubkey,
+    sanitize::Sanitize,
     signature::{Keypair, Signature},
     slot_hashes::SlotHashes,
     slot_history::SlotHistory,
@@ -1135,7 +1136,11 @@ impl Bank {
         &'a self,
         txs: &'b [Transaction],
     ) -> TransactionBatch<'a, 'b> {
-        let mut batch = TransactionBatch::new(vec![Ok(()); txs.len()], &self, txs, None);
+        let lock_results: Vec<_> = txs
+            .iter()
+            .map(|tx| tx.sanitize().map_err(|e| e.into()))
+            .collect();
+        let mut batch = TransactionBatch::new(lock_results, &self, txs, None);
         batch.needs_unlock = false;
         batch
     }
diff --git a/sdk/src/transaction.rs b/sdk/src/transaction.rs
index 23a0f62bb5be3e..7015709b6090c0 100644
--- a/sdk/src/transaction.rs
+++ b/sdk/src/transaction.rs
@@ -81,6 +81,12 @@ impl std::fmt::Display for TransactionError {
     }
 }
 
+impl From<SanitizeError> for TransactionError {
+    fn from(_: SanitizeError) -> Self {
+        Self::SanitizeFailure
+    }
+}
+
 /// An atomic transaction
 #[derive(Debug, PartialEq, Default, Eq, Clone, Serialize, Deserialize)]
 pub struct Transaction {