Skip to content

Commit

Permalink
Reject faked stake/vote accounts in stake mgmt. (bp #13615) (#13620)
Browse files Browse the repository at this point in the history 
* Reject faked stake/vote accounts in stake mgmt. (#13615)

* Reject faked stake/vote accounts in stake mgmt.

* Use clearer name

(cherry picked from commit 2b3faa1)

# Conflicts:
#	programs/stake/src/stake_instruction.rs

* Fix conflict

Co-authored-by: Ryo Onodera <ryoqun@gmail.com>
  • Loading branch information
@mergify @ryoqun
mergify[bot] and ryoqun committed Nov 16, 2020
1 parent 29ac8f5 commit e12cb45
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 5 deletions.
33 changes: 28 additions & 5 deletions programs/stake/src/stake_instruction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Account> {
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
Expand All @@ -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()
})
Expand Down Expand Up @@ -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),
);
Expand All @@ -604,7 +625,7 @@ mod tests {
&Pubkey::default(),
&Pubkey::default(),
100,
&Pubkey::default(),
&invalid_stake_state_pubkey(),
&Pubkey::default(),
"seed"
)[1]
Expand All @@ -615,7 +636,7 @@ mod tests {
process_instruction(&delegate_stake(
&Pubkey::default(),
&Pubkey::default(),
&Pubkey::default()
&invalid_vote_state_pubkey(),
)),
Err(InstructionError::InvalidAccountData),
);
Expand Down Expand Up @@ -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,
Expand Down
106 changes: 106 additions & 0 deletions programs/stake/src/stake_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,10 @@ impl<'a> StakeAccount for KeyedAccount<'a> {
config: &Config,
signers: &HashSet<Pubkey>,
) -> 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)?;
Expand Down Expand Up @@ -881,6 +885,10 @@ impl<'a> StakeAccount for KeyedAccount<'a> {
split: &KeyedAccount,
signers: &HashSet<Pubkey>,
) -> Result<(), InstructionError> {
if split.owner()? != id() {
return Err(InstructionError::IncorrectProgramId);
}

if let StakeState::Uninitialized = split.state()? {
// verify enough account lamports
if lamports > self.lamports()? {
Expand Down Expand Up @@ -980,6 +988,10 @@ impl<'a> StakeAccount for KeyedAccount<'a> {
stake_history: &StakeHistory,
signers: &HashSet<Pubkey>,
) -> 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
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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::<StakeState>(),
&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::<StakeState>(),
&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();
Expand Down Expand Up @@ -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::<StakeState>(),
&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::<StakeState>(),
&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();
Expand Down

0 comments on commit e12cb45

Please sign in to comment.