diff --git a/programs/config/src/config_processor.rs b/programs/config/src/config_processor.rs index 665b05a52..b56b89ca2 100644 --- a/programs/config/src/config_processor.rs +++ b/programs/config/src/config_processor.rs @@ -3,7 +3,7 @@ use crate::ConfigKeys; use bincode::deserialize; use solana_sdk::{ - ic_msg, + feature_set, ic_msg, instruction::InstructionError, keyed_account::{next_keyed_account, KeyedAccount}, process_instruction::InvokeContext, @@ -20,8 +20,15 @@ pub fn process_instruction( let key_list: ConfigKeys = limited_deserialize(data)?; let keyed_accounts_iter = &mut keyed_accounts.iter(); let config_keyed_account = &mut next_keyed_account(keyed_accounts_iter)?; + let current_data: ConfigKeys = { let config_account = config_keyed_account.try_account_ref_mut()?; + if invoke_context.is_feature_active(&feature_set::check_program_owner::id()) + && config_account.owner != crate::id() + { + return Err(InstructionError::InvalidAccountOwner); + } + deserialize(&config_account.data).map_err(|err| { ic_msg!( invoke_context, @@ -174,6 +181,7 @@ mod tests { }; let config_account = RefCell::new(Account { data: vec![0; space as usize], + owner: id(), ..Account::default() }); let accounts = vec![(&config_pubkey, true, &config_account)]; @@ -326,7 +334,10 @@ mod tests { let my_config = MyConfig::new(42); let instruction = config_instruction::store(&config_pubkey, false, keys, &my_config); - let signer0_account = RefCell::new(Account::default()); + let signer0_account = RefCell::new(Account { + owner: id(), + ..Account::default() + }); let accounts = vec![(&signer0_pubkey, true, &signer0_account)]; let keyed_accounts = create_keyed_is_signer_accounts(&accounts); assert_eq!( @@ -588,4 +599,35 @@ mod tests { Err(InstructionError::NotEnoughAccountKeys) ); } + + #[test] + fn test_config_bad_owner() { + let from_pubkey = solana_sdk::pubkey::new_rand(); + let config_pubkey = solana_sdk::pubkey::new_rand(); + let new_config = MyConfig::new(84); + let signer0_pubkey = solana_sdk::pubkey::new_rand(); + let signer0_account = RefCell::new(Account::default()); + let config_account = RefCell::new(Account::default()); + let keys = vec![ + (from_pubkey, false), + (signer0_pubkey, true), + (config_pubkey, true), + ]; + + let instruction = config_instruction::store(&config_pubkey, true, keys, &new_config); + let accounts = vec![ + (&config_pubkey, true, &config_account), + (&signer0_pubkey, true, &signer0_account), + ]; + let keyed_accounts = create_keyed_is_signer_accounts(&accounts); + assert_eq!( + process_instruction( + &id(), + &keyed_accounts, + &instruction.data, + &mut MockInvokeContext::default() + ), + Err(InstructionError::InvalidAccountOwner) + ); + } } diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 173ea7b12..58af042a1 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -496,7 +496,11 @@ pub fn process_instruction( let me = &next_keyed_account(keyed_accounts)?; if me.owner()? != id() { - return Err(InstructionError::IncorrectProgramId); + if invoke_context.is_feature_active(&feature_set::check_program_owner::id()) { + return Err(InstructionError::InvalidAccountOwner); + } else { + return Err(InstructionError::IncorrectProgramId); + } } match limited_deserialize(data)? { @@ -802,7 +806,7 @@ mod tests { &Authorized::default(), &Lockup::default() )), - Err(InstructionError::IncorrectProgramId), + Err(InstructionError::InvalidAccountOwner), ); assert_eq!( process_instruction(&authorize( @@ -812,7 +816,7 @@ mod tests { StakeAuthorize::Staker, None, )), - Err(InstructionError::IncorrectProgramId), + Err(InstructionError::InvalidAccountOwner), ); assert_eq!( process_instruction( @@ -823,7 +827,7 @@ mod tests { &Pubkey::default(), )[1] ), - Err(InstructionError::IncorrectProgramId), + Err(InstructionError::InvalidAccountOwner), ); assert_eq!( process_instruction( @@ -844,7 +848,7 @@ mod tests { &Pubkey::default(), )[0] ), - Err(InstructionError::IncorrectProgramId), + Err(InstructionError::InvalidAccountOwner), ); assert_eq!( process_instruction( @@ -867,7 +871,7 @@ mod tests { "seed" )[1] ), - Err(InstructionError::IncorrectProgramId), + Err(InstructionError::InvalidAccountOwner), ); assert_eq!( process_instruction(&delegate_stake( @@ -875,7 +879,7 @@ mod tests { &Pubkey::default(), &Pubkey::default(), )), - Err(InstructionError::IncorrectProgramId), + Err(InstructionError::InvalidAccountOwner), ); assert_eq!( process_instruction(&withdraw( @@ -885,14 +889,14 @@ mod tests { 100, None, )), - Err(InstructionError::IncorrectProgramId), + Err(InstructionError::InvalidAccountOwner), ); assert_eq!( process_instruction(&deactivate_stake( &spoofed_stake_state_pubkey(), &Pubkey::default() )), - Err(InstructionError::IncorrectProgramId), + Err(InstructionError::InvalidAccountOwner), ); assert_eq!( process_instruction(&set_lockup( @@ -900,7 +904,7 @@ mod tests { &LockupArgs::default(), &Pubkey::default() )), - Err(InstructionError::IncorrectProgramId), + Err(InstructionError::InvalidAccountOwner), ); } diff --git a/programs/vest/src/vest_processor.rs b/programs/vest/src/vest_processor.rs index bd40f31ae..5e8134609 100644 --- a/programs/vest/src/vest_processor.rs +++ b/programs/vest/src/vest_processor.rs @@ -8,6 +8,7 @@ use solana_config_program::date_instruction::DateConfig; use solana_config_program::get_config_data; use solana_sdk::{ account::Account, + feature_set, instruction::InstructionError, keyed_account::{next_keyed_account, KeyedAccount}, process_instruction::InvokeContext, @@ -60,10 +61,15 @@ pub fn process_instruction( _program_id: &Pubkey, keyed_accounts: &[KeyedAccount], data: &[u8], - _invoke_context: &mut dyn InvokeContext, + invoke_context: &mut dyn InvokeContext, ) -> Result<(), InstructionError> { let keyed_accounts_iter = &mut keyed_accounts.iter(); let contract_account = &mut next_keyed_account(keyed_accounts_iter)?.try_account_ref_mut()?; + if invoke_context.is_feature_active(&feature_set::check_program_owner::id()) + && contract_account.owner != crate::id() + { + return Err(InstructionError::InvalidAccountOwner); + } let instruction = limited_deserialize(data)?; diff --git a/programs/vote/src/vote_instruction.rs b/programs/vote/src/vote_instruction.rs index e505018da..649d8384e 100644 --- a/programs/vote/src/vote_instruction.rs +++ b/programs/vote/src/vote_instruction.rs @@ -289,6 +289,12 @@ pub fn process_instruction( let keyed_accounts = &mut keyed_accounts.iter(); let me = &mut next_keyed_account(keyed_accounts)?; + if invoke_context.is_feature_active(&feature_set::check_program_owner::id()) + && me.owner()? != id() + { + return Err(InstructionError::InvalidAccountOwner); + } + match limited_deserialize(data)? { VoteInstruction::InitializeAccount(vote_init) => { verify_rent_exemption(me, next_keyed_account(keyed_accounts)?)?; @@ -341,6 +347,7 @@ mod tests { rent::Rent, }; use std::cell::RefCell; + use std::str::FromStr; // these are for 100% coverage in this file #[test] @@ -368,8 +375,16 @@ mod tests { account::create_account(&SlotHashes::default(), 1) } else if sysvar::rent::check_id(&meta.pubkey) { account::create_account(&Rent::free(), 1) + } else if meta.pubkey == invalid_vote_state_pubkey() { + Account { + owner: invalid_vote_state_pubkey(), + ..Account::default() + } } else { - Account::default() + Account { + owner: id(), + ..Account::default() + } }) }) .collect(); @@ -393,8 +408,25 @@ mod tests { } } + fn invalid_vote_state_pubkey() -> Pubkey { + Pubkey::from_str("BadVote111111111111111111111111111111111111").unwrap() + } + + #[test] + fn test_spoofed_vote() { + assert_eq!( + process_instruction(&vote( + &invalid_vote_state_pubkey(), + &Pubkey::default(), + Vote::default(), + )), + Err(InstructionError::InvalidAccountOwner), + ); + } + #[test] fn test_vote_process_instruction() { + solana_logger::setup(); let instructions = create_account( &Pubkey::default(), &Pubkey::default(), diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7acfd2b70..8dbcf5d17 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -109,7 +109,7 @@ impl ExecuteTimings { } type BankStatusCache = StatusCache>; -#[frozen_abi(digest = "9wmxRM64shxBcCCjQtRMwCGkWun4VeUufNoHgLXwwFfz")] +#[frozen_abi(digest = "3ZaEt781qwhfQSE4DZPBHhng2S6MuimchRjkR9ZWzDFs")] pub type BankSlotDelta = SlotDelta>; type TransactionAccountRefCells = Vec>>; type TransactionAccountDepRefCells = Vec<(Pubkey, RefCell)>; @@ -10466,7 +10466,7 @@ pub(crate) mod tests { bank.process_transaction(&tx), Err(TransactionError::InstructionError( 0, - InstructionError::InvalidAccountData + InstructionError::InvalidAccountOwner )) ); assert_eq!(bank.get_balance(&mint_keypair.pubkey()), 498); // transaction fee charged @@ -10488,7 +10488,7 @@ pub(crate) mod tests { bank.process_transaction(&tx), Err(TransactionError::InstructionError( 0, - InstructionError::InvalidAccountData + InstructionError::InvalidAccountOwner )) ); assert_eq!(bank.get_balance(&mint_keypair.pubkey()), 496); // transaction fee charged diff --git a/sdk/program/src/instruction.rs b/sdk/program/src/instruction.rs index 8714fc8fe..abdc988a9 100644 --- a/sdk/program/src/instruction.rs +++ b/sdk/program/src/instruction.rs @@ -195,6 +195,9 @@ pub enum InstructionError { #[error("An account does not have enough lamports to be rent-exempt")] AccountNotRentExempt, + + #[error("Invalid account owner")] + InvalidAccountOwner, } #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index c66ccbbd9..8a2ab2cdb 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -123,6 +123,10 @@ pub mod check_init_vote_data { solana_sdk::declare_id!("3ccR6QpxGYsAbWyfevEtBNGfWV4xBffxRj2tD6A9i39F"); } +pub mod check_program_owner { + solana_sdk::declare_id!("5XnbR5Es9YXEARRuP6mdvoxiW3hx5atNNeBmwVd8P3QD"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -153,7 +157,8 @@ lazy_static! { (full_inflation::mainnet::certusone::vote::id(), "Community vote allowing Certus One to enable full inflation"), (warp_timestamp_again::id(), "warp timestamp again, adjust bounding to 25% fast 80% slow #15204"), (per_byte_logging_cost::id(), "charge the compute budget per byte for logging"), - (check_init_vote_data::id(), "check initialized Vote data") + (check_init_vote_data::id(), "check initialized Vote data"), + (check_program_owner::id(), "limit programs to operating on accounts owned by itself") /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/storage-proto/proto/solana.storage.transaction_by_addr.rs b/storage-proto/proto/solana.storage.transaction_by_addr.rs index 4d1763d57..49ca4a9f1 100644 --- a/storage-proto/proto/solana.storage.transaction_by_addr.rs +++ b/storage-proto/proto/solana.storage.transaction_by_addr.rs @@ -116,4 +116,5 @@ pub enum InstructionErrorType { IncorrectAuthority = 43, BorshIoError = 44, AccountNotRentExempt = 45, + InvalidAccountOwner = 46, } diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index aa78ef633..6117fb976 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -483,6 +483,9 @@ impl TryFrom for TransactionError { 41 => InstructionError::ProgramFailedToCompile, 42 => InstructionError::Immutable, 43 => InstructionError::IncorrectAuthority, + 44 => InstructionError::BorshIoError(String::new()), + 45 => InstructionError::AccountNotRentExempt, + 46 => InstructionError::InvalidAccountOwner, _ => return Err("Invalid InstructionError"), }; @@ -706,6 +709,9 @@ impl From for tx_by_addr::TransactionError { InstructionError::AccountNotRentExempt => { tx_by_addr::InstructionErrorType::AccountNotRentExempt } + InstructionError::InvalidAccountOwner => { + tx_by_addr::InstructionErrorType::InvalidAccountOwner + } } as i32, custom: match instruction_error { InstructionError::Custom(custom) => { diff --git a/storage-proto/src/transaction_by_addr.proto b/storage-proto/src/transaction_by_addr.proto index cd8319927..ca46b0960 100644 --- a/storage-proto/src/transaction_by_addr.proto +++ b/storage-proto/src/transaction_by_addr.proto @@ -95,6 +95,7 @@ enum InstructionErrorType { INCORRECT_AUTHORITY = 43; BORSH_IO_ERROR = 44; ACCOUNT_NOT_RENT_EXEMPT = 45; + INVALID_ACCOUNT_OWNER = 46; } message UnixTimestamp {