From 7ce327b9e9685a4fb6e6020ba05836cc591160b6 Mon Sep 17 00:00:00 2001 From: Babur Makhmudov Date: Tue, 19 May 2026 16:57:49 +0400 Subject: [PATCH 1/4] fix: tighten privileged access permissions --- svm/src/access_permissions.rs | 154 ++++++++++++++++++++++++++++++++-- 1 file changed, 148 insertions(+), 6 deletions(-) diff --git a/svm/src/access_permissions.rs b/svm/src/access_permissions.rs index f9aef19..628ce60 100644 --- a/svm/src/access_permissions.rs +++ b/svm/src/access_permissions.rs @@ -1,11 +1,16 @@ use solana_account::AccountSharedData; +use solana_pubkey::Pubkey; 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; 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 ExecutedTransaction { /// Validates that a transaction does not attempt to write to non-delegated accounts. /// @@ -13,9 +18,8 @@ impl ExecutedTransaction { /// 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 +32,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() { @@ -43,7 +47,7 @@ impl ExecutedTransaction { let mut offender = None; let is_mutable = - |acc: &AccountSharedData| acc.delegated() || acc.undelegating() || acc.ephemeral(); + |acc: &AccountSharedData| acc.delegated() || 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. for (i, (pk, acc)) in accounts.iter().enumerate().skip(1) { @@ -62,3 +66,141 @@ 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 != 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) -> 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) + ); + } +} From 1e6e2012f44941368e84e99009448207c47e58f3 Mon Sep 17 00:00:00 2001 From: Babur Makhmudov Date: Tue, 19 May 2026 18:05:32 +0400 Subject: [PATCH 2/4] fix: allow for program deploy --- svm/src/access_permissions.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/svm/src/access_permissions.rs b/svm/src/access_permissions.rs index 628ce60..3c56e05 100644 --- a/svm/src/access_permissions.rs +++ b/svm/src/access_permissions.rs @@ -1,5 +1,6 @@ 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; @@ -75,6 +76,9 @@ fn has_privileged_access(message: &impl SVMMessage) -> bool { else { return false; }; + if *program == loader_v4::ID { + continue; + } if *program != MAGIC_PROGRAM_ID { return false; } From 709615beba17ab79b06fa190810289842019c3b4 Mon Sep 17 00:00:00 2001 From: Babur Makhmudov Date: Tue, 19 May 2026 21:26:14 +0400 Subject: [PATCH 3/4] fix: allow undelegating mutations --- svm/src/access_permissions.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/svm/src/access_permissions.rs b/svm/src/access_permissions.rs index 3c56e05..21b1cca 100644 --- a/svm/src/access_permissions.rs +++ b/svm/src/access_permissions.rs @@ -47,8 +47,9 @@ impl ExecutedTransaction { } let mut offender = None; - let is_mutable = - |acc: &AccountSharedData| acc.delegated() || acc.ephemeral() || acc.confined(); + 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) { From b1861f82ccdaa29eaf20e6a96de2c1bccdcb5475 Mon Sep 17 00:00:00 2001 From: Babur Makhmudov Date: Wed, 20 May 2026 11:11:49 +0400 Subject: [PATCH 4/4] fix: add missing instructions --- svm/src/access_permissions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/svm/src/access_permissions.rs b/svm/src/access_permissions.rs index 21b1cca..11085d1 100644 --- a/svm/src/access_permissions.rs +++ b/svm/src/access_permissions.rs @@ -8,7 +8,7 @@ use crate::transaction_execution_result::ExecutedTransaction; 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]; +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 synchronization with upstream