Restrict transaction fee payers to system accounts (#4198)

automerge

runtime/src/accounts.rs

@@ -15,6 +15,7 @@ use solana_sdk::hash::{Hash, Hasher};
 use solana_sdk::native_loader;
 use solana_sdk::pubkey::Pubkey;
 use solana_sdk::signature::{Keypair, KeypairUtil};
+use solana_sdk::system_program;
 use solana_sdk::transaction::Result;
 use solana_sdk::transaction::{Transaction, TransactionError};
 use std::borrow::Borrow;
@@ -191,6 +192,9 @@ impl Accounts {
             if called_accounts.is_empty() || called_accounts[0].lamports == 0 {
                 error_counters.account_not_found += 1;
                 Err(TransactionError::AccountNotFound)
+            } else if called_accounts[0].owner != system_program::id() {
+                error_counters.invalid_account_for_fee += 1;
+                Err(TransactionError::InvalidAccountForFee)
             } else if called_accounts[0].lamports < fee {
                 error_counters.insufficient_funds += 1;
                 Err(TransactionError::InsufficientFundsForFee)
@@ -733,6 +737,36 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_load_accounts_invalid_account_for_fee() {
+        let mut accounts: Vec<(Pubkey, Account)> = Vec::new();
+        let mut error_counters = ErrorCounters::default();
+
+        let keypair = Keypair::new();
+        let key0 = keypair.pubkey();
+
+        let account = Account::new(1, 1, &Pubkey::new_rand()); // <-- owner is not the system program
+        accounts.push((key0, account));
+
+        let instructions = vec![CompiledInstruction::new(1, &(), vec![0])];
+        let tx = Transaction::new_with_compiled_instructions(
+            &[&keypair],
+            &[],
+            Hash::default(),
+            vec![native_loader::id()],
+            instructions,
+        );
+
+        let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters);
+
+        assert_eq!(error_counters.invalid_account_for_fee, 1);
+        assert_eq!(loaded_accounts.len(), 1);
+        assert_eq!(
+            loaded_accounts[0],
+            Err(TransactionError::InvalidAccountForFee)
+        );
+    }
+
     #[test]
     fn test_load_accounts_no_loaders() {
         let mut accounts: Vec<(Pubkey, Account)> = Vec::new();

runtime/src/accounts_db.rs

@@ -42,6 +42,7 @@ pub struct ErrorCounters {
     pub blockhash_not_found: usize,
     pub blockhash_too_old: usize,
     pub reserve_blockhash: usize,
+    pub invalid_account_for_fee: usize,
     pub insufficient_funds: usize,
     pub invalid_account_index: usize,
     pub duplicate_signature: usize,

runtime/src/bank.rs

@@ -602,6 +602,14 @@ impl Bank {
                 1000
             );
         }
+        if 0 != error_counters.invalid_account_for_fee {
+            inc_new_counter_error!(
+                "bank-process_transactions-error-invalid_account_for_fee",
+                error_counters.invalid_account_for_fee,
+                0,
+                1000
+            );
+        }
         if 0 != error_counters.insufficient_funds {
             inc_new_counter_error!(
                 "bank-process_transactions-error-insufficient_funds",

sdk/src/transaction.rs

@@ -28,6 +28,9 @@ pub enum TransactionError {
     /// The from `Pubkey` does not have sufficient balance to pay the fee to schedule the transaction
     InsufficientFundsForFee,
 
+    /// This account may not be used to pay transaction fees
+    InvalidAccountForFee,
+
     /// The bank has seen `Signature` before. This can occur under normal operation
     /// when a UDP packet is duplicated, as a user error from a client not updating
     /// its `recent_blockhash`, or as a double-spend attack.