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

* 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:
#	program-runtime/src/invoke_context.rs
#	programs/vote/src/vote_instruction.rs
#	sdk/src/feature_set.rs

* resolve conflicts

* fix old rust issue

Co-authored-by: Justin Starry <justin@solana.com>

programs/vote/src/vote_instruction.rs

@@ -16,7 +16,7 @@ use {
         hash::Hash,
         instruction::{AccountMeta, Instruction, InstructionError},
         keyed_account::{from_keyed_account, get_signers, keyed_account_at_index, KeyedAccount},
-        process_instruction::InvokeContext,
+        process_instruction::{get_sysvar, InvokeContext},
         program_utils::limited_deserialize,
         pubkey::Pubkey,
         system_instruction,
@@ -366,7 +366,14 @@ pub fn process_instruction(
         }
         VoteInstruction::Withdraw(lamports) => {
             let to = keyed_account_at_index(keyed_accounts, 1)?;
-            vote_state::withdraw(me, lamports, to, &signers)
+            let rent_sysvar = if invoke_context
+                .is_feature_active(&feature_set::reject_non_rent_exempt_vote_withdraws::id())
+            {
+                Some(get_sysvar(invoke_context, &sysvar::rent::id())?)
+            } else {
+                None
+            };
+            vote_state::withdraw(me, lamports, to, &signers, rent_sysvar)
         }
         VoteInstruction::AuthorizeChecked(vote_authorize) => {
             if invoke_context.is_feature_active(&feature_set::vote_stake_checked_instructions::id())
@@ -395,7 +402,7 @@ mod tests {
         bincode::serialize,
         solana_sdk::{
             account::{self, Account, AccountSharedData},
-            process_instruction::MockInvokeContext,
+            process_instruction::{mock_set_sysvar, MockInvokeContext},
             rent::Rent,
         },
         std::{cell::RefCell, str::FromStr},
@@ -454,11 +461,14 @@ mod tests {
                 .zip(accounts.iter())
                 .map(|(meta, account)| KeyedAccount::new(&meta.pubkey, meta.is_signer, account))
                 .collect();
-            super::process_instruction(
+            let mut invoke_context = MockInvokeContext::new(keyed_accounts);
-                &Pubkey::default(),
+            mock_set_sysvar(
-                &instruction.data,
+                &mut invoke_context,
-                &mut MockInvokeContext::new(keyed_accounts),
+                sysvar::rent::id(),
+                sysvar::rent::Rent::default(),
             )
+            .unwrap();
+            super::process_instruction(&Pubkey::default(), &instruction.data, &mut invoke_context)
         }
     }
 

programs/vote/src/vote_state/mod.rs

@@ -20,7 +20,6 @@ use {
     },
     std::{
         boxed::Box,
-        cmp::Ordering,
         collections::{HashSet, VecDeque},
     },
 };
@@ -682,20 +681,28 @@ pub fn withdraw<S: std::hash::BuildHasher>(
     lamports: u64,
     to_account: &KeyedAccount,
     signers: &HashSet<Pubkey, S>,
+    rent_sysvar: Option<Rent>,
 ) -> Result<(), InstructionError> {
     let vote_state: VoteState =
         State::<VoteStateVersions>::state(vote_account)?.convert_to_current();
 
     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
         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
         .try_account_ref_mut()?
         .checked_sub_lamports(lamports)?;
@@ -899,6 +906,8 @@ mod tests {
     }
 
     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();
         (
             vote_pubkey,
@@ -906,7 +915,7 @@ mod tests {
                 &vote_pubkey,
                 &solana_sdk::pubkey::new_rand(),
                 0,
-                100,
+                balance,
             )),
         )
     }
@@ -1667,35 +1676,120 @@ mod tests {
                 &RefCell::new(AccountSharedData::default()),
             ),
             &signers,
+            None,
         );
         assert_eq!(res, Err(InstructionError::MissingRequiredSignature));
 
         // insufficient funds
         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 res = withdraw(
             &keyed_accounts[0],
-            101,
+            lamports + 1,
             &KeyedAccount::new(
                 &solana_sdk::pubkey::new_rand(),
                 false,
                 &RefCell::new(AccountSharedData::default()),
             ),
             &signers,
+            None,
         );
         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 (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 keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)];
                 let signers: HashSet<Pubkey> = get_signers(keyed_accounts);
-        let pre_state: VoteStateVersions = vote_account.borrow().state().unwrap();
                 let res = withdraw(
                     &keyed_accounts[0],
                     lamports,
                     &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account),
                     &signers,
+                    *rent_sysvar,
                 );
                 assert_eq!(res, Ok(()));
                 assert_eq!(vote_account.borrow().lamports(), 0);
@@ -1703,10 +1797,8 @@ mod tests {
                 let post_state: VoteStateVersions = vote_account.borrow().state().unwrap();
                 // State has been deinitialized since balance is zero
                 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
         let authorized_withdrawer_pubkey = solana_sdk::pubkey::new_rand();
@@ -1735,6 +1827,7 @@ mod tests {
             lamports,
             withdrawer_keyed_account,
             &signers,
+            None,
         );
         assert_eq!(res, Ok(()));
         assert_eq!(vote_account.borrow().lamports(), 0);

sdk/src/feature_set.rs

@@ -273,6 +273,10 @@ pub mod reject_section_virtual_address_file_offset_mismatch {
     solana_sdk::declare_id!("5N4NikcJLEiZNqwndhNyvZw15LvFXp1oF7AJQTNTZY5k");
 }
 
+pub mod reject_non_rent_exempt_vote_withdraws {
+    solana_sdk::declare_id!("7txXZZD6Um59YoLMF7XUNimbMjsqsWhc7g2EniiTrmp1");
+}
+
 lazy_static! {
     /// Map of feature identifiers to user-visible description
     pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@@ -341,6 +345,7 @@ lazy_static! {
         (spl_token_v3_3_0_release::id(), "spl-token v3.3.0 release"),
         (reject_deployment_of_unresolved_syscalls::id(), "Reject deployment of programs with unresolved syscall symbols"),
         (reject_section_virtual_address_file_offset_mismatch::id(), "enforce section virtual addresses and file offsets in ELF to be equal"),
+        (reject_non_rent_exempt_vote_withdraws::id(), "fail vote withdraw instructions which leave the account non-rent-exempt"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]
     .iter()