From e12cb457fb0eae15e815da4c116a01ddcca671e1 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 16 Nov 2020 22:05:31 +0000 Subject: [PATCH] Reject faked stake/vote accounts in stake mgmt. (bp #13615) (#13620) * Reject faked stake/vote accounts in stake mgmt. (#13615) * Reject faked stake/vote accounts in stake mgmt. * Use clearer name (cherry picked from commit 2b3faa1947c1fe7b16bfc372c006f93ac88647f6) # Conflicts: # programs/stake/src/stake_instruction.rs * Fix conflict Co-authored-by: Ryo Onodera --- programs/stake/src/stake_instruction.rs | 33 ++++++-- programs/stake/src/stake_state.rs | 106 ++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 5 deletions(-) diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 03e6a06626..09cf77c619 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -525,11 +525,20 @@ mod tests { use bincode::serialize; use solana_sdk::{account::Account, rent::Rent, sysvar::stake_history::StakeHistory}; use std::cell::RefCell; + use std::str::FromStr; fn create_default_account() -> RefCell { RefCell::new(Account::default()) } + fn invalid_stake_state_pubkey() -> Pubkey { + Pubkey::from_str("BadStake11111111111111111111111111111111111").unwrap() + } + + fn invalid_vote_state_pubkey() -> Pubkey { + Pubkey::from_str("BadVote111111111111111111111111111111111111").unwrap() + } + fn process_instruction(instruction: &Instruction) -> Result<(), InstructionError> { let accounts: Vec<_> = instruction .accounts @@ -545,6 +554,14 @@ mod tests { config::create_account(0, &config::Config::default()) } else if sysvar::rent::check_id(&meta.pubkey) { sysvar::rent::create_account(1, &Rent::default()) + } else if meta.pubkey == invalid_stake_state_pubkey() { + let mut account = Account::default(); + account.owner = id(); + account + } else if meta.pubkey == invalid_vote_state_pubkey() { + let mut account = Account::default(); + account.owner = solana_vote_program::id(); + account } else { Account::default() }) @@ -587,14 +604,18 @@ mod tests { &Pubkey::default(), &Pubkey::default(), 100, - &Pubkey::default() + &invalid_stake_state_pubkey(), )[1] ), Err(InstructionError::InvalidAccountData), ); assert_eq!( process_instruction( - &merge(&Pubkey::default(), &Pubkey::default(), &Pubkey::default(),)[0] + &merge( + &Pubkey::default(), + &invalid_stake_state_pubkey(), + &Pubkey::default(), + )[0] ), Err(InstructionError::InvalidAccountData), ); @@ -604,7 +625,7 @@ mod tests { &Pubkey::default(), &Pubkey::default(), 100, - &Pubkey::default(), + &invalid_stake_state_pubkey(), &Pubkey::default(), "seed" )[1] @@ -615,7 +636,7 @@ mod tests { process_instruction(&delegate_stake( &Pubkey::default(), &Pubkey::default(), - &Pubkey::default() + &invalid_vote_state_pubkey(), )), Err(InstructionError::InvalidAccountData), ); @@ -746,12 +767,14 @@ mod tests { ); // gets the check non-deserialize-able account in delegate_stake + let mut bad_vote_account = create_default_account(); + bad_vote_account.get_mut().owner = solana_vote_program::id(); assert_eq!( super::process_instruction( &Pubkey::default(), &[ KeyedAccount::new(&Pubkey::default(), true, &create_default_account()), - KeyedAccount::new(&Pubkey::default(), false, &create_default_account()), + KeyedAccount::new(&Pubkey::default(), false, &bad_vote_account), KeyedAccount::new( &sysvar::clock::id(), false, diff --git a/programs/stake/src/stake_state.rs b/programs/stake/src/stake_state.rs index 582415b91d..a8c50d815d 100644 --- a/programs/stake/src/stake_state.rs +++ b/programs/stake/src/stake_state.rs @@ -820,6 +820,10 @@ impl<'a> StakeAccount for KeyedAccount<'a> { config: &Config, signers: &HashSet, ) -> Result<(), InstructionError> { + if vote_account.owner()? != solana_vote_program::id() { + return Err(InstructionError::IncorrectProgramId); + } + match self.state()? { StakeState::Initialized(meta) => { meta.authorized.check(signers, StakeAuthorize::Staker)?; @@ -881,6 +885,10 @@ impl<'a> StakeAccount for KeyedAccount<'a> { split: &KeyedAccount, signers: &HashSet, ) -> Result<(), InstructionError> { + if split.owner()? != id() { + return Err(InstructionError::IncorrectProgramId); + } + if let StakeState::Uninitialized = split.state()? { // verify enough account lamports if lamports > self.lamports()? { @@ -980,6 +988,10 @@ impl<'a> StakeAccount for KeyedAccount<'a> { stake_history: &StakeHistory, signers: &HashSet, ) -> Result<(), InstructionError> { + if source_stake.owner()? != id() { + return Err(InstructionError::IncorrectProgramId); + } + let meta = match self.state()? { StakeState::Stake(meta, stake) => { // stake must be fully de-activated @@ -1479,6 +1491,21 @@ mod tests { ) .is_ok()); + // signed but faked vote account + let faked_vote_account = vote_account.clone(); + faked_vote_account.borrow_mut().owner = solana_sdk::pubkey::new_rand(); + let faked_vote_keyed_account = KeyedAccount::new(&vote_pubkey, false, &faked_vote_account); + assert_eq!( + stake_keyed_account.delegate( + &faked_vote_keyed_account, + &clock, + &StakeHistory::default(), + &Config::default(), + &signers, + ), + Err(solana_sdk::instruction::InstructionError::IncorrectProgramId) + ); + // verify that delegate() looks right, compare against hand-rolled let stake = StakeState::stake_from(&stake_keyed_account.account.borrow()).unwrap(); assert_eq!( @@ -3557,6 +3584,40 @@ mod tests { } } + #[test] + fn test_split_fake_stake_dest() { + let stake_pubkey = solana_sdk::pubkey::new_rand(); + let stake_lamports = 42; + + let split_stake_pubkey = solana_sdk::pubkey::new_rand(); + let signers = vec![stake_pubkey].into_iter().collect(); + + let split_stake_account = Account::new_ref_data_with_space( + 0, + &StakeState::Uninitialized, + std::mem::size_of::(), + &solana_sdk::pubkey::new_rand(), + ) + .expect("stake_account"); + + let split_stake_keyed_account = + KeyedAccount::new(&split_stake_pubkey, true, &split_stake_account); + + let stake_account = Account::new_ref_data_with_space( + stake_lamports, + &StakeState::Stake(Meta::auto(&stake_pubkey), Stake::just_stake(stake_lamports)), + std::mem::size_of::(), + &id(), + ) + .expect("stake_account"); + let stake_keyed_account = KeyedAccount::new(&stake_pubkey, true, &stake_account); + + assert_eq!( + stake_keyed_account.split(stake_lamports / 2, &split_stake_keyed_account, &signers), + Err(InstructionError::IncorrectProgramId), + ); + } + #[test] fn test_split_to_account_with_rent_exempt_reserve() { let stake_pubkey = solana_sdk::pubkey::new_rand(); @@ -4420,6 +4481,51 @@ mod tests { } } + #[test] + fn test_merge_fake_stake_source() { + let stake_pubkey = solana_sdk::pubkey::new_rand(); + let source_stake_pubkey = solana_sdk::pubkey::new_rand(); + let authorized_pubkey = solana_sdk::pubkey::new_rand(); + let stake_lamports = 42; + + let signers = vec![authorized_pubkey].into_iter().collect(); + + let stake_account = Account::new_ref_data_with_space( + stake_lamports, + &StakeState::Stake( + Meta::auto(&authorized_pubkey), + Stake::just_stake(stake_lamports), + ), + std::mem::size_of::(), + &id(), + ) + .expect("stake_account"); + let stake_keyed_account = KeyedAccount::new(&stake_pubkey, true, &stake_account); + + let source_stake_account = Account::new_ref_data_with_space( + stake_lamports, + &StakeState::Stake( + Meta::auto(&authorized_pubkey), + Stake::just_stake(stake_lamports), + ), + std::mem::size_of::(), + &solana_sdk::pubkey::new_rand(), + ) + .expect("source_stake_account"); + let source_stake_keyed_account = + KeyedAccount::new(&source_stake_pubkey, true, &source_stake_account); + + assert_eq!( + stake_keyed_account.merge( + &source_stake_keyed_account, + &Clock::default(), + &StakeHistory::default(), + &signers + ), + Err(InstructionError::IncorrectProgramId) + ); + } + #[test] fn test_merge_active_stake() { let stake_pubkey = solana_sdk::pubkey::new_rand();