fix: only allow select instructions in privileged mode

src/access_permissions.rs

@@ -5,24 +5,28 @@ use solana_transaction_error::TransactionError;
 
 use crate::account_loader::LoadedTransaction;
 
+const MAGIC_PROGRAM_ID: Pubkey =
+    Pubkey::from_str_const("Magic11111111111111111111111111111111111111");
+const PRIVILEGED_MAGIC_DISCRIMINANTS: [u32; 9] = [0, 8, 9, 16, 17, 18, 19, 20, 22];
+
 // NOTE:
-// this impl is kept separately to simplify synchoronization with upstream
+// this impl is kept separately to simplify synchronization with upstream
 impl LoadedTransaction {
     /// Validates that a transaction does not attempt to write to non-delegated accounts.
     ///
     /// This is a critical security check to prevent privilege escalation by ensuring
     /// account modifications are restricted to accounts explicitly delegated to the
     /// validator node.
     ///
-    /// ## Logic
-    /// This function enforces a security rule with a key exception: **if the fee payer
-    /// has privileged access, this check is bypassed entirely.**
+    /// Privileged fee payers may bypass this check only for the Magic program's
+    /// allowlisted control instructions.
     ///
     /// For standard, non-privileged transactions, it enforces that any account
     /// marked as writable (excluding the fee payer) must be either:
     /// 1. delegated
     /// 2. undelegating
     /// 3. ephemeral
+    /// 4. confined
     ///
     /// Read-only accounts are ignored. The fee payer's writability is handled in
     /// separate validation logic.
@@ -31,13 +35,36 @@ impl LoadedTransaction {
         message: &impl SVMMessage,
     ) -> Result<(), (TransactionError, Pubkey)> {
         let payer = self.accounts.first().map(|(_, acc)| acc);
-        if payer.map(|p| p.privileged()).unwrap_or_default() {
-            // Payer has privileged access, so we can skip the validation.
+        let mut privileged = payer.is_some_and(AccountSharedData::privileged);
+        if privileged {
+            for i in message.instructions_iter() {
+                let Some(program) = message.account_keys().get(i.program_id_index as usize) else {
+                    privileged = false;
+                    break;
+                };
+                if *program != MAGIC_PROGRAM_ID {
+                    privileged = false;
+                    break;
+                }
+                let discriminant = i
+                    .data
+                    .get(0..4)
+                    .and_then(|b| <[u8; 4]>::try_from(b).ok())
+                    .map(u32::from_le_bytes)
+                    .unwrap_or(u32::MAX);
+                if !PRIVILEGED_MAGIC_DISCRIMINANTS.contains(&discriminant) {
+                    privileged = false;
+                    break;
+                }
+            }
+        }
+        if privileged {
             return Ok(());
         }
 
-        let mutation_allowed =
-            |acc: &AccountSharedData| acc.delegated() || acc.undelegating() || acc.ephemeral();
+        let mutation_allowed = |acc: &AccountSharedData| {
+            acc.delegated() || acc.undelegating() || acc.ephemeral() || acc.confined()
+        };
 
         // For non-privileged payers, validate the rest of the accounts.
         // Skip the fee payer (index 0), as its writability is validated elsewhere.
@@ -49,3 +76,111 @@ impl LoadedTransaction {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use {
+        super::*,
+        crate::{account_loader::LoadedTransaction, rollback_accounts::RollbackAccounts},
+        solana_account::{
+            test_utils::{create_borrowed_account_shared_data, BorrowedAccountBufferArea},
+            AccountSharedData,
+        },
+        solana_compute_budget::compute_budget_limits::ComputeBudgetLimits,
+        solana_fee_structure::FeeDetails,
+        solana_hash::Hash,
+        solana_message::{
+            compiled_instruction::CompiledInstruction, LegacyMessage, Message, MessageHeader,
+            SanitizedMessage,
+        },
+        solana_rent_debits::RentDebits,
+        solana_reserved_account_keys::ReservedAccountKeys,
+    };
+
+    fn privileged_account() -> (BorrowedAccountBufferArea, AccountSharedData) {
+        let account = AccountSharedData::default();
+        let (buffer, mut account) = create_borrowed_account_shared_data(&account, 0);
+        account.as_borrowed_mut().unwrap().set_privileged(true);
+        (buffer, account)
+    }
+
+    fn loaded_transaction(
+        payer: Pubkey,
+        payer_account: AccountSharedData,
+        writable: Pubkey,
+    ) -> LoadedTransaction {
+        LoadedTransaction {
+            accounts: vec![
+                (payer, payer_account),
+                (writable, AccountSharedData::default()),
+                (MAGIC_PROGRAM_ID, AccountSharedData::default()),
+            ],
+            program_indices: vec![],
+            fee_details: FeeDetails::default(),
+            rollback_accounts: RollbackAccounts::default(),
+            compute_budget_limits: ComputeBudgetLimits::default(),
+            rent: 0,
+            rent_debits: RentDebits::default(),
+            loaded_accounts_data_size: 0,
+        }
+    }
+
+    fn message(program: Pubkey, data: Vec<u8>) -> SanitizedMessage {
+        SanitizedMessage::Legacy(LegacyMessage::new(
+            Message {
+                account_keys: vec![Pubkey::new_unique(), Pubkey::new_unique(), program],
+                header: MessageHeader {
+                    num_required_signatures: 1,
+                    num_readonly_signed_accounts: 0,
+                    num_readonly_unsigned_accounts: 1,
+                },
+                instructions: vec![CompiledInstruction {
+                    program_id_index: 2,
+                    accounts: vec![1],
+                    data,
+                }],
+                recent_blockhash: Hash::default(),
+            },
+            &ReservedAccountKeys::empty_key_set(),
+        ))
+    }
+
+    #[test]
+    fn privileged_payer_allows_magic_control_instruction() {
+        let payer = Pubkey::new_unique();
+        let writable = Pubkey::new_unique();
+        let (_buffer, payer_account) = privileged_account();
+        let tx = loaded_transaction(payer, payer_account, writable);
+        let message = message(MAGIC_PROGRAM_ID, 8u32.to_le_bytes().to_vec());
+
+        assert_eq!(tx.validate_accounts_access(&message), Ok(()));
+    }
+
+    #[test]
+    fn privileged_payer_rejects_non_magic_write() {
+        let payer = Pubkey::new_unique();
+        let writable = Pubkey::new_unique();
+        let (_buffer, payer_account) = privileged_account();
+        let tx = loaded_transaction(payer, payer_account, writable);
+        let message = message(Pubkey::new_unique(), 8u32.to_le_bytes().to_vec());
+
+        assert_eq!(
+            tx.validate_accounts_access(&message),
+            Err((TransactionError::InvalidWritableAccount, writable))
+        );
+    }
+
+    #[test]
+    fn privileged_payer_rejects_unlisted_magic_instruction() {
+        let payer = Pubkey::new_unique();
+        let writable = Pubkey::new_unique();
+        let (_buffer, payer_account) = privileged_account();
+        let tx = loaded_transaction(payer, payer_account, writable);
+        let message = message(MAGIC_PROGRAM_ID, 1u32.to_le_bytes().to_vec());
+
+        assert_eq!(
+            tx.validate_accounts_access(&message),
+            Err((TransactionError::InvalidWritableAccount, writable))
+        );
+    }
+}

tests/integration_test.rs

@@ -310,11 +310,7 @@ impl SvmTestEnvironment<'_> {
         // check that all the account states we care about are present and correct
         for (pubkey, expected_account_data) in self.test_entry.final_accounts.iter() {
             let actual_account_data = final_accounts_actual.get(pubkey);
-            assert!(
-                actual_account_data.is_some(),
-                "missing account {}",
-                pubkey
-            );
+            assert!(actual_account_data.is_some(), "missing account {}", pubkey);
             assert!(
                 solana_account::accounts_equal(expected_account_data, actual_account_data.unwrap()),
                 "mismatch on account {}\n  expected: {:?}\n  actual: {:?}",
@@ -2422,7 +2418,7 @@ fn svm_inspect_account() {
         Hash::default(),
     );
 
-    initial_test_entry.push_transaction(transaction);
+    initial_test_entry.push_transaction_with_status(transaction, ExecutionStatus::ExecutedFailed);
 
     let mut recipient_account = AccountSharedData::default();
     recipient_account.set_lamports(transfer_amount);
@@ -2493,7 +2489,7 @@ fn svm_inspect_account() {
         Hash::default(),
     );
 
-    final_test_entry.push_transaction(transaction);
+    final_test_entry.push_transaction_with_status(transaction, ExecutionStatus::ExecutedFailed);
 
     final_test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 2);
     final_test_entry.decrease_expected_lamports(&sender, transfer_amount);
@@ -3110,11 +3106,11 @@ fn enforce_access_permissions_false_with_multiple_non_delegated_accounts() {
 }
 
 // -----------------
-// Privileged Accounts Override Access Checks
+// Privileged Accounts
 // -----------------
 #[test]
-fn test_privileged_fee_payer_with_non_delegated_writable_accounts() {
-    // Test that privileged fee payer can write to non-delegated writable accounts
+fn privileged_fee_payer_with_non_magic_program_cannot_write_non_delegated_accounts() {
+    // Privileged fee payers only bypass access checks for allowlisted Magic instructions.
     let fee_payer_keypair = Keypair::new();
     let fee_payer = fee_payer_keypair.pubkey();
     let (_guard, mut fee_payer_account) = create_privileged_account(LAMPORTS_PER_SOL);
@@ -3143,9 +3139,9 @@ fn test_privileged_fee_payer_with_non_delegated_writable_accounts() {
         LAST_BLOCKHASH,
     );
 
-    test_entry.push_transaction(transaction);
+    test_entry.push_transaction_with_status(transaction, ExecutionStatus::ExecutedFailed);
     test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE);
 
     let env = SvmTestEnvironment::create(test_entry);
     env.execute();
-}
+}