check program owners (#15495)

* check program owners

* BankSlotDelta should change because InstructionError variant added

Co-authored-by: Tyera Eulberg <tyera@solana.com>

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)
+        );
+    }
 }

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),
         );
     }
 

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)?;
 

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(),

runtime/src/bank.rs

@@ -109,7 +109,7 @@ impl ExecuteTimings {
 }
 
 type BankStatusCache = StatusCache<Result<()>>;
-#[frozen_abi(digest = "9wmxRM64shxBcCCjQtRMwCGkWun4VeUufNoHgLXwwFfz")]
+#[frozen_abi(digest = "3ZaEt781qwhfQSE4DZPBHhng2S6MuimchRjkR9ZWzDFs")]
 pub type BankSlotDelta = SlotDelta<Result<()>>;
 type TransactionAccountRefCells = Vec<Rc<RefCell<Account>>>;
 type TransactionAccountDepRefCells = Vec<(Pubkey, RefCell<Account>)>;
@@ -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

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)]

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<Pubkey, &'static str> = [
@@ -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()

storage-proto/proto/solana.storage.transaction_by_addr.rs

@@ -116,4 +116,5 @@ pub enum InstructionErrorType {
     IncorrectAuthority = 43,
     BorshIoError = 44,
     AccountNotRentExempt = 45,
+    InvalidAccountOwner = 46,
 }

storage-proto/src/convert.rs

@@ -483,6 +483,9 @@ impl TryFrom<tx_by_addr::TransactionError> 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<TransactionError> 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) => {

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 {