fix: tighten privileged access permissions

svm/src/access_permissions.rs

@@ -1,21 +1,26 @@
 use solana_account::AccountSharedData;
+use solana_pubkey::Pubkey;
+use solana_sdk_ids::loader_v4;
 use solana_svm_transaction::svm_message::SVMMessage;
 use solana_transaction_error::TransactionError;
 
 use crate::transaction_execution_result::ExecutedTransaction;
 
+const MAGIC_PROGRAM_ID: Pubkey =
+    Pubkey::from_str_const("Magic11111111111111111111111111111111111111");
+const PRIVILEGED_MAGIC_DISCRIMINANTS: [u32; 11] = [0, 8, 9, 16, 17, 18, 19, 20, 21, 22, 24];
+
 // NOTE:
-// this impl is kept separately to simplify synchoronization with upstream
+// this impl is kept separately to simplify synchronization with upstream
 impl ExecutedTransaction {
     /// 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 a delegated account.**
@@ -28,7 +33,7 @@ impl ExecutedTransaction {
         }
         let accounts = &self.loaded_transaction.accounts;
         if let Some((pk, payer)) = accounts.first() {
-            if payer.privileged() {
+            if payer.privileged() && has_privileged_access(message) {
                 return;
             }
             if !payer.delegated() && payer.lamports_changed() {
@@ -42,8 +47,9 @@ impl ExecutedTransaction {
         }
 
         let mut offender = None;
-        let is_mutable =
-            |acc: &AccountSharedData| acc.delegated() || acc.undelegating() || acc.ephemeral();
+        let is_mutable = |acc: &AccountSharedData| {
+            acc.delegated() || acc.ephemeral() || acc.confined() || acc.undelegating()
+        };
         // For non-privileged payers, validate the rest of the accounts.
         // Skip the fee payer (index 0), as its writability is validated elsewhere.
         for (i, (pk, acc)) in accounts.iter().enumerate().skip(1) {
@@ -62,3 +68,144 @@ impl ExecutedTransaction {
         }
     }
 }
+
+fn has_privileged_access(message: &impl SVMMessage) -> bool {
+    for instruction in message.instructions_iter() {
+        let Some(program) = message
+            .account_keys()
+            .get(instruction.program_id_index as usize)
+        else {
+            return false;
+        };
+        if *program == loader_v4::ID {
+            continue;
+        }
+        if *program != MAGIC_PROGRAM_ID {
+            return false;
+        }
+
+        let discriminant = instruction
+            .data
+            .get(0..4)
+            .and_then(|bytes| <[u8; 4]>::try_from(bytes).ok())
+            .map(u32::from_le_bytes)
+            .unwrap_or(u32::MAX);
+        if !PRIVILEGED_MAGIC_DISCRIMINANTS.contains(&discriminant) {
+            return false;
+        }
+    }
+    true
+}
+
+#[cfg(test)]
+mod tests {
+    use {
+        super::*,
+        crate::{
+            account_loader::LoadedTransaction, rollback_accounts::RollbackAccounts,
+            transaction_execution_result::TransactionExecutionDetails,
+        },
+        solana_fee_structure::FeeDetails,
+        solana_hash::Hash,
+        solana_message::{
+            compiled_instruction::CompiledInstruction, LegacyMessage, Message, MessageHeader,
+            SanitizedMessage,
+        },
+        solana_program_runtime::execution_budget::SVMTransactionExecutionBudget,
+        std::collections::{HashMap, HashSet},
+    };
+
+    fn privileged_account() -> AccountSharedData {
+        let mut account = AccountSharedData::default();
+        account.set_privileged(true);
+        account
+    }
+
+    fn executed_transaction(payer: Pubkey, writable: Pubkey) -> ExecutedTransaction {
+        ExecutedTransaction {
+            loaded_transaction: LoadedTransaction {
+                accounts: vec![
+                    (payer, privileged_account()),
+                    (writable, AccountSharedData::default()),
+                    (MAGIC_PROGRAM_ID, AccountSharedData::default()),
+                ],
+                program_indices: vec![],
+                fee_details: FeeDetails::default(),
+                rollback_accounts: RollbackAccounts::default(),
+                compute_budget: SVMTransactionExecutionBudget::default(),
+                loaded_accounts_data_size: 0,
+            },
+            execution_details: TransactionExecutionDetails {
+                status: Ok(()),
+                log_messages: None,
+                inner_instructions: None,
+                return_data: None,
+                executed_units: 0,
+                accounts_data_len_delta: 0,
+            },
+            programs_modified_by_tx: HashMap::new(),
+        }
+    }
+
+    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(),
+            },
+            &HashSet::new(),
+        ))
+    }
+
+    #[test]
+    fn privileged_payer_allows_magic_control_instruction() {
+        let payer = Pubkey::new_unique();
+        let writable = Pubkey::new_unique();
+        let mut tx = executed_transaction(payer, writable);
+        let message = message(MAGIC_PROGRAM_ID, 8u32.to_le_bytes().to_vec());
+
+        tx.validate_accounts_access(&message);
+
+        assert_eq!(tx.execution_details.status, Ok(()));
+    }
+
+    #[test]
+    fn privileged_payer_rejects_non_magic_write() {
+        let payer = Pubkey::new_unique();
+        let writable = Pubkey::new_unique();
+        let mut tx = executed_transaction(payer, writable);
+        let message = message(Pubkey::new_unique(), 8u32.to_le_bytes().to_vec());
+
+        tx.validate_accounts_access(&message);
+
+        assert_eq!(
+            tx.execution_details.status,
+            Err(TransactionError::InvalidWritableAccount)
+        );
+    }
+
+    #[test]
+    fn privileged_payer_rejects_unlisted_magic_instruction() {
+        let payer = Pubkey::new_unique();
+        let writable = Pubkey::new_unique();
+        let mut tx = executed_transaction(payer, writable);
+        let message = message(MAGIC_PROGRAM_ID, 1u32.to_le_bytes().to_vec());
+
+        tx.validate_accounts_access(&message);
+
+        assert_eq!(
+            tx.execution_details.status,
+            Err(TransactionError::InvalidWritableAccount)
+        );
+    }
+}