Reject vote withdraws that create non-rent-exempt accounts (backport #21639) (#21645)

* Reject vote withdraws that create non-rent-exempt accounts (#21639)

* Reject vote withdraws that create non-rent-exempt accounts

* fix mocked instruction test

(cherry picked from commit e123883b26)

# Conflicts:
#	sdk/src/feature_set.rs

* resolve conflicts

Co-authored-by: Justin Starry <justin@solana.com>
This commit is contained in:
mergify[bot]
2021-12-07 00:42:01 +00:00
committed by GitHub
parent b3fa1e4550
commit 89d2f34a03
4 changed files with 172 additions and 34 deletions

View File

@ -192,12 +192,20 @@ impl<'a> InvokeContext<'a> {
pub fn new_mock( pub fn new_mock(
accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)], accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)],
builtin_programs: &'a [BuiltinProgram], builtin_programs: &'a [BuiltinProgram],
) -> Self {
Self::new_mock_with_sysvars(accounts, builtin_programs, &[])
}
pub fn new_mock_with_sysvars(
accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)],
builtin_programs: &'a [BuiltinProgram],
sysvars: &'a [(Pubkey, Vec<u8>)],
) -> Self { ) -> Self {
Self::new( Self::new(
Rent::default(), Rent::default(),
accounts, accounts,
builtin_programs, builtin_programs,
&[], sysvars,
Some(LogCollector::new_ref()), Some(LogCollector::new_ref()),
ComputeBudget::default(), ComputeBudget::default(),
Rc::new(RefCell::new(Executors::default())), Rc::new(RefCell::new(Executors::default())),
@ -915,11 +923,12 @@ pub fn with_mock_invoke_context<R, F: FnMut(&mut InvokeContext) -> R>(
callback(&mut invoke_context) callback(&mut invoke_context)
} }
pub fn mock_process_instruction( pub fn mock_process_instruction_with_sysvars(
loader_id: &Pubkey, loader_id: &Pubkey,
mut program_indices: Vec<usize>, mut program_indices: Vec<usize>,
instruction_data: &[u8], instruction_data: &[u8],
keyed_accounts: &[(bool, bool, Pubkey, Rc<RefCell<AccountSharedData>>)], keyed_accounts: &[(bool, bool, Pubkey, Rc<RefCell<AccountSharedData>>)],
sysvars: &[(Pubkey, Vec<u8>)],
process_instruction: ProcessInstructionWithContext, process_instruction: ProcessInstructionWithContext,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
let mut preparation = let mut preparation =
@ -927,7 +936,8 @@ pub fn mock_process_instruction(
let processor_account = AccountSharedData::new_ref(0, 0, &solana_sdk::native_loader::id()); let processor_account = AccountSharedData::new_ref(0, 0, &solana_sdk::native_loader::id());
program_indices.insert(0, preparation.accounts.len()); program_indices.insert(0, preparation.accounts.len());
preparation.accounts.push((*loader_id, processor_account)); preparation.accounts.push((*loader_id, processor_account));
let mut invoke_context = InvokeContext::new_mock(&preparation.accounts, &[]); let mut invoke_context =
InvokeContext::new_mock_with_sysvars(&preparation.accounts, &[], sysvars);
invoke_context.push( invoke_context.push(
&preparation.message, &preparation.message,
&preparation.message.instructions[0], &preparation.message.instructions[0],
@ -937,6 +947,23 @@ pub fn mock_process_instruction(
process_instruction(1, instruction_data, &mut invoke_context) process_instruction(1, instruction_data, &mut invoke_context)
} }
pub fn mock_process_instruction(
loader_id: &Pubkey,
program_indices: Vec<usize>,
instruction_data: &[u8],
keyed_accounts: &[(bool, bool, Pubkey, Rc<RefCell<AccountSharedData>>)],
process_instruction: ProcessInstructionWithContext,
) -> Result<(), InstructionError> {
mock_process_instruction_with_sysvars(
loader_id,
program_indices,
instruction_data,
keyed_accounts,
&[],
process_instruction,
)
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use { use {

View File

@ -408,7 +408,15 @@ pub fn process_instruction(
} }
VoteInstruction::Withdraw(lamports) => { VoteInstruction::Withdraw(lamports) => {
let to = keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?; let to = keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?;
vote_state::withdraw(me, lamports, to, &signers) let rent_sysvar = if invoke_context
.feature_set
.is_active(&feature_set::reject_non_rent_exempt_vote_withdraws::id())
{
Some(invoke_context.get_sysvar(&sysvar::rent::id())?)
} else {
None
};
vote_state::withdraw(me, lamports, to, &signers, rent_sysvar)
} }
VoteInstruction::AuthorizeChecked(vote_authorize) => { VoteInstruction::AuthorizeChecked(vote_authorize) => {
if invoke_context if invoke_context
@ -500,11 +508,15 @@ mod tests {
.zip(accounts.into_iter()) .zip(accounts.into_iter())
.map(|(meta, account)| (meta.is_signer, meta.is_writable, meta.pubkey, account)) .map(|(meta, account)| (meta.is_signer, meta.is_writable, meta.pubkey, account))
.collect(); .collect();
solana_program_runtime::invoke_context::mock_process_instruction(
let rent = Rent::default();
let rent_sysvar = (sysvar::rent::id(), bincode::serialize(&rent).unwrap());
solana_program_runtime::invoke_context::mock_process_instruction_with_sysvars(
&id(), &id(),
Vec::new(), Vec::new(),
&instruction.data, &instruction.data,
&keyed_accounts, &keyed_accounts,
&[rent_sysvar],
super::process_instruction, super::process_instruction,
) )
} }

View File

@ -890,20 +890,28 @@ pub fn withdraw<S: std::hash::BuildHasher>(
lamports: u64, lamports: u64,
to_account: &KeyedAccount, to_account: &KeyedAccount,
signers: &HashSet<Pubkey, S>, signers: &HashSet<Pubkey, S>,
rent_sysvar: Option<Rent>,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
let vote_state: VoteState = let vote_state: VoteState =
State::<VoteStateVersions>::state(vote_account)?.convert_to_current(); State::<VoteStateVersions>::state(vote_account)?.convert_to_current();
verify_authorized_signer(&vote_state.authorized_withdrawer, signers)?; verify_authorized_signer(&vote_state.authorized_withdrawer, signers)?;
match vote_account.lamports()?.cmp(&lamports) { let remaining_balance = vote_account
Ordering::Less => return Err(InstructionError::InsufficientFunds), .lamports()?
Ordering::Equal => { .checked_sub(lamports)
.ok_or(InstructionError::InsufficientFunds)?;
if remaining_balance == 0 {
// Deinitialize upon zero-balance // Deinitialize upon zero-balance
vote_account.set_state(&VoteStateVersions::new_current(VoteState::default()))?; vote_account.set_state(&VoteStateVersions::new_current(VoteState::default()))?;
} else if let Some(rent_sysvar) = rent_sysvar {
let min_rent_exempt_balance = rent_sysvar.minimum_balance(vote_account.data_len()?);
if remaining_balance < min_rent_exempt_balance {
return Err(InstructionError::InsufficientFunds);
} }
_ => (),
} }
vote_account vote_account
.try_account_ref_mut()? .try_account_ref_mut()?
.checked_sub_lamports(lamports)?; .checked_sub_lamports(lamports)?;
@ -1107,6 +1115,8 @@ mod tests {
} }
fn create_test_account() -> (Pubkey, RefCell<AccountSharedData>) { fn create_test_account() -> (Pubkey, RefCell<AccountSharedData>) {
let rent = Rent::default();
let balance = VoteState::get_rent_exempt_reserve(&rent);
let vote_pubkey = solana_sdk::pubkey::new_rand(); let vote_pubkey = solana_sdk::pubkey::new_rand();
( (
vote_pubkey, vote_pubkey,
@ -1114,7 +1124,7 @@ mod tests {
&vote_pubkey, &vote_pubkey,
&solana_sdk::pubkey::new_rand(), &solana_sdk::pubkey::new_rand(),
0, 0,
100, balance,
)), )),
) )
} }
@ -1875,35 +1885,120 @@ mod tests {
&RefCell::new(AccountSharedData::default()), &RefCell::new(AccountSharedData::default()),
), ),
&signers, &signers,
None,
); );
assert_eq!(res, Err(InstructionError::MissingRequiredSignature)); assert_eq!(res, Err(InstructionError::MissingRequiredSignature));
// insufficient funds // insufficient funds
let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)];
let lamports = vote_account.borrow().lamports();
let signers: HashSet<Pubkey> = get_signers(keyed_accounts); let signers: HashSet<Pubkey> = get_signers(keyed_accounts);
let res = withdraw( let res = withdraw(
&keyed_accounts[0], &keyed_accounts[0],
101, lamports + 1,
&KeyedAccount::new( &KeyedAccount::new(
&solana_sdk::pubkey::new_rand(), &solana_sdk::pubkey::new_rand(),
false, false,
&RefCell::new(AccountSharedData::default()), &RefCell::new(AccountSharedData::default()),
), ),
&signers, &signers,
None,
); );
assert_eq!(res, Err(InstructionError::InsufficientFunds)); assert_eq!(res, Err(InstructionError::InsufficientFunds));
// all good // non rent exempt withdraw, before feature activation
{
let (vote_pubkey, vote_account) = create_test_account();
let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)];
let lamports = vote_account.borrow().lamports();
let rent_sysvar = Rent::default();
let minimum_balance = rent_sysvar
.minimum_balance(vote_account.borrow().data().len())
.max(1);
assert!(minimum_balance <= lamports);
let signers: HashSet<Pubkey> = get_signers(keyed_accounts);
let res = withdraw(
&keyed_accounts[0],
lamports - minimum_balance + 1,
&KeyedAccount::new(
&solana_sdk::pubkey::new_rand(),
false,
&RefCell::new(AccountSharedData::default()),
),
&signers,
None,
);
assert_eq!(res, Ok(()));
}
// non rent exempt withdraw, after feature activation
{
let (vote_pubkey, vote_account) = create_test_account();
let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)];
let lamports = vote_account.borrow().lamports();
let rent_sysvar = Rent::default();
let minimum_balance = rent_sysvar
.minimum_balance(vote_account.borrow().data().len())
.max(1);
assert!(minimum_balance <= lamports);
let signers: HashSet<Pubkey> = get_signers(keyed_accounts);
let res = withdraw(
&keyed_accounts[0],
lamports - minimum_balance + 1,
&KeyedAccount::new(
&solana_sdk::pubkey::new_rand(),
false,
&RefCell::new(AccountSharedData::default()),
),
&signers,
Some(rent_sysvar),
);
assert_eq!(res, Err(InstructionError::InsufficientFunds));
}
// partial valid withdraw, after feature activation
{
let to_account = RefCell::new(AccountSharedData::default()); let to_account = RefCell::new(AccountSharedData::default());
let (vote_pubkey, vote_account) = create_test_account();
let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)];
let lamports = vote_account.borrow().lamports();
let rent_sysvar = Rent::default();
let minimum_balance = rent_sysvar
.minimum_balance(vote_account.borrow().data().len())
.max(1);
assert!(minimum_balance <= lamports);
let withdraw_lamports = lamports - minimum_balance;
let signers: HashSet<Pubkey> = get_signers(keyed_accounts);
let res = withdraw(
&keyed_accounts[0],
withdraw_lamports,
&KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account),
&signers,
Some(rent_sysvar),
);
assert_eq!(res, Ok(()));
assert_eq!(
vote_account.borrow().lamports(),
lamports - withdraw_lamports
);
assert_eq!(to_account.borrow().lamports(), withdraw_lamports);
}
// full withdraw, before/after activation
{
let rent_sysvar = Rent::default();
for rent_sysvar in [None, Some(rent_sysvar)] {
let to_account = RefCell::new(AccountSharedData::default());
let (vote_pubkey, vote_account) = create_test_account();
let lamports = vote_account.borrow().lamports(); let lamports = vote_account.borrow().lamports();
let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)];
let signers: HashSet<Pubkey> = get_signers(keyed_accounts); let signers: HashSet<Pubkey> = get_signers(keyed_accounts);
let pre_state: VoteStateVersions = vote_account.borrow().state().unwrap();
let res = withdraw( let res = withdraw(
&keyed_accounts[0], &keyed_accounts[0],
lamports, lamports,
&KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account),
&signers, &signers,
rent_sysvar,
); );
assert_eq!(res, Ok(())); assert_eq!(res, Ok(()));
assert_eq!(vote_account.borrow().lamports(), 0); assert_eq!(vote_account.borrow().lamports(), 0);
@ -1911,10 +2006,8 @@ mod tests {
let post_state: VoteStateVersions = vote_account.borrow().state().unwrap(); let post_state: VoteStateVersions = vote_account.borrow().state().unwrap();
// State has been deinitialized since balance is zero // State has been deinitialized since balance is zero
assert!(post_state.is_uninitialized()); assert!(post_state.is_uninitialized());
}
// reset balance and restore state, verify that authorized_withdrawer works }
vote_account.borrow_mut().set_lamports(lamports);
vote_account.borrow_mut().set_state(&pre_state).unwrap();
// authorize authorized_withdrawer // authorize authorized_withdrawer
let authorized_withdrawer_pubkey = solana_sdk::pubkey::new_rand(); let authorized_withdrawer_pubkey = solana_sdk::pubkey::new_rand();
@ -1943,6 +2036,7 @@ mod tests {
lamports, lamports,
withdrawer_keyed_account, withdrawer_keyed_account,
&signers, &signers,
None,
); );
assert_eq!(res, Ok(())); assert_eq!(res, Ok(()));
assert_eq!(vote_account.borrow().lamports(), 0); assert_eq!(vote_account.borrow().lamports(), 0);

View File

@ -263,6 +263,10 @@ pub mod reject_empty_instruction_without_program {
solana_sdk::declare_id!("9kdtFSrXHQg3hKkbXkQ6trJ3Ja1xpJ22CTFSNAciEwmL"); solana_sdk::declare_id!("9kdtFSrXHQg3hKkbXkQ6trJ3Ja1xpJ22CTFSNAciEwmL");
} }
pub mod reject_non_rent_exempt_vote_withdraws {
solana_sdk::declare_id!("7txXZZD6Um59YoLMF7XUNimbMjsqsWhc7g2EniiTrmp1");
}
lazy_static! { lazy_static! {
/// Map of feature identifiers to user-visible description /// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [ pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -323,6 +327,7 @@ lazy_static! {
(spl_token_v3_3_0_release::id(), "spl-token v3.3.0 release"), (spl_token_v3_3_0_release::id(), "spl-token v3.3.0 release"),
(leave_nonce_on_success::id(), "leave nonce as is on success"), (leave_nonce_on_success::id(), "leave nonce as is on success"),
(reject_empty_instruction_without_program::id(), "fail instructions which have native_loader as program_id directly"), (reject_empty_instruction_without_program::id(), "fail instructions which have native_loader as program_id directly"),
(reject_non_rent_exempt_vote_withdraws::id(), "fail vote withdraw instructions which leave the account non-rent-exempt"),
/*************** ADD NEW FEATURES HERE ***************/ /*************** ADD NEW FEATURES HERE ***************/
] ]
.iter() .iter()