Audit fixes

README.md

@@ -33,7 +33,8 @@ Users can:
 # Development
 
 ## Rust
-* Built and developed using - rust stable(`rustc 1.57.0 (f1edd0429 2021-11-29)`)
+* Built and developed using - rust 1.66.1, see rust-toolchain.toml
+* Works with solana-release 1.14.20
 * Run rust based tests - `cargo test-sbf`
 * `run-generate-anchor-types.sh` generates latest anchor types file and writes to `./voter_stake_registry.ts`
 * To install the typescript client, do - `yarn add @blockworks-foundation/voter-stake-registry-client`

programs/voter-stake-registry/src/instructions/close_voter.rs

@@ -3,7 +3,7 @@ use std::ops::DerefMut;
 use crate::error::*;
 use crate::state::*;
 use anchor_lang::prelude::*;
-use anchor_spl::token::{self, CloseAccount, Token, TokenAccount};
+use anchor_spl::token::{self, CloseAccount, Token, TokenAccount, Transfer};
 use bytemuck::bytes_of_mut;
 
 // Remaining accounts must be all the token token accounts owned by voter, he wants to close,
@@ -20,6 +20,7 @@ pub struct CloseVoter<'info> {
         seeds = [voter.load()?.registrar.key().as_ref(), b"voter".as_ref(), voter_authority.key().as_ref()],
         bump = voter.load()?.voter_bump,
         has_one = voter_authority,
+        has_one = registrar,
         close = sol_destination
     )]
     pub voter: AccountLoader<'info, Voter>,
@@ -83,3 +84,83 @@ pub fn close_voter<'key, 'accounts, 'remaining, 'info>(
 
     Ok(())
 }
+
+/// Closes the voter account, transfers all funds from token accounts and closes vaults.
+/// Only accounts with no remaining lockups can be closed.
+/// remaining_accounts: All voter vaults followed by target token accounts, in order.
+pub fn close_voter_v2<'key, 'accounts, 'remaining, 'info>(
+    ctx: Context<'key, 'accounts, 'remaining, 'info, CloseVoter<'info>>,
+) -> Result<()> {
+    let registrar = ctx.accounts.registrar.load()?;
+    let curr_ts = registrar.clock_unix_timestamp();
+
+    {
+        let voter = ctx.accounts.voter.load()?;
+
+        let active_deposit_entries = voter.deposits.iter().filter(|d| d.is_used).count();
+        require_eq!(ctx.remaining_accounts.len(), active_deposit_entries * 2);
+
+        let any_locked = voter.deposits.iter().any(|d| d.amount_locked(curr_ts) > 0);
+        require!(!any_locked, VsrError::DepositStillLocked);
+
+        let voter_seeds = voter_seeds!(voter);
+
+        let active_deposits = voter.deposits.iter().filter(|d| d.is_used);
+        let deposit_vaults = &ctx.remaining_accounts[..active_deposit_entries];
+        let target_accounts = &ctx.remaining_accounts[active_deposit_entries..];
+
+        for ((deposit, deposit_vault), target_account) in
+            active_deposits.zip(deposit_vaults).zip(target_accounts)
+        {
+            let mint = &registrar.voting_mints[deposit.voting_mint_config_idx as usize].mint;
+
+            let token = Account::<TokenAccount>::try_from(&deposit_vault.clone()).unwrap();
+            require_keys_eq!(
+                token.owner,
+                ctx.accounts.voter.key(),
+                VsrError::InvalidAuthority
+            );
+            require_keys_eq!(token.mint, *mint);
+
+            // transfer to target_account
+            let cpi_transfer_accounts = Transfer {
+                from: deposit_vault.to_account_info(),
+                to: target_account.to_account_info(),
+                authority: ctx.accounts.voter.to_account_info(),
+            };
+            token::transfer(
+                CpiContext::new_with_signer(
+                    ctx.accounts.token_program.to_account_info(),
+                    cpi_transfer_accounts,
+                    &[voter_seeds],
+                ),
+                token.amount,
+            )?;
+
+            // close vault
+            let cpi_close_accounts = CloseAccount {
+                account: deposit_vault.to_account_info(),
+                destination: ctx.accounts.sol_destination.to_account_info(),
+                authority: ctx.accounts.voter.to_account_info(),
+            };
+            token::close_account(CpiContext::new_with_signer(
+                ctx.accounts.token_program.to_account_info(),
+                cpi_close_accounts,
+                &[voter_seeds],
+            ))?;
+
+            deposit_vault.exit(ctx.program_id)?;
+        }
+    }
+
+    // zero out voter account to prevent reinit attacks
+    // appease rust borrow checker
+    {
+        let mut voter = ctx.accounts.voter.load_mut()?;
+        let voter_dereffed = voter.deref_mut();
+        let voter_bytes = bytes_of_mut(voter_dereffed);
+        voter_bytes.fill(0);
+    }
+
+    Ok(())
+}

programs/voter-stake-registry/src/instructions/create_voter.rs

@@ -1,6 +1,7 @@
 use crate::error::*;
 use crate::state::*;
 use anchor_lang::prelude::*;
+use anchor_lang::solana_program::instruction::{get_stack_height, TRANSACTION_LEVEL_STACK_HEIGHT};
 use anchor_lang::solana_program::sysvar::instructions as tx_instructions;
 use std::mem::size_of;
 
@@ -39,6 +40,7 @@ pub struct CreateVoter<'info> {
     pub system_program: Program<'info, System>,
     pub rent: Sysvar<'info, Rent>,
 
+    /// NOTE: this account is currently unused
     /// CHECK: Address constraint is set
     #[account(address = tx_instructions::ID)]
     pub instructions: UncheckedAccount<'info>,
@@ -57,16 +59,10 @@ pub fn create_voter(
     // Forbid creating voter accounts from CPI. The goal is to make automation
     // impossible that weakens some of the limitations intentionally imposed on
     // locked tokens.
-    {
-        let ixns = ctx.accounts.instructions.to_account_info();
-        let current_index = tx_instructions::load_current_index_checked(&ixns)? as usize;
-        let current_ixn = tx_instructions::load_instruction_at_checked(current_index, &ixns)?;
-        require_keys_eq!(
-            current_ixn.program_id,
-            *ctx.program_id,
-            VsrError::ForbiddenCpi
-        );
-    }
+    require!(
+        get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT,
+        VsrError::ForbiddenCpi
+    );
 
     require_eq!(voter_bump, *ctx.bumps.get("voter").unwrap());
     require_eq!(

programs/voter-stake-registry/src/instructions/internal_transfer_locked.rs

@@ -37,6 +37,7 @@ pub fn internal_transfer_locked(
     target_deposit_entry_index: u8,
     amount: u64,
 ) -> Result<()> {
+    require_neq!(source_deposit_entry_index, target_deposit_entry_index);
     let registrar = &ctx.accounts.registrar.load()?;
     let voter = &mut ctx.accounts.voter.load_mut()?;
     let curr_ts = registrar.clock_unix_timestamp();

programs/voter-stake-registry/src/instructions/internal_transfer_unlocked.rs

@@ -31,6 +31,7 @@ pub fn internal_transfer_unlocked(
     target_deposit_entry_index: u8,
     amount: u64,
 ) -> Result<()> {
+    require_neq!(source_deposit_entry_index, target_deposit_entry_index);
     let registrar = &ctx.accounts.registrar.load()?;
     let voter = &mut ctx.accounts.voter.load_mut()?;
     let curr_ts = registrar.clock_unix_timestamp();

programs/voter-stake-registry/src/instructions/mod.rs

@@ -11,8 +11,6 @@ pub use internal_transfer_locked::*;
 pub use internal_transfer_unlocked::*;
 pub use log_voter_info::*;
 pub use reset_lockup::*;
-pub use set_time_offset::*;
-pub use update_max_vote_weight::*;
 pub use update_voter_weight_record::*;
 pub use withdraw::*;
 
@@ -29,7 +27,5 @@ mod internal_transfer_locked;
 mod internal_transfer_unlocked;
 mod log_voter_info;
 mod reset_lockup;
-mod set_time_offset;
-mod update_max_vote_weight;
 mod update_voter_weight_record;
 mod withdraw;

programs/voter-stake-registry/src/lib.rs

@@ -197,25 +197,23 @@ pub mod voter_stake_registry {
         instructions::update_voter_weight_record(ctx)
     }
 
-    pub fn update_max_vote_weight(ctx: Context<UpdateMaxVoteWeight>) -> Result<()> {
-        instructions::update_max_vote_weight(ctx)
-    }
-
     pub fn close_voter<'key, 'accounts, 'remaining, 'info>(
         ctx: Context<'key, 'accounts, 'remaining, 'info, CloseVoter<'info>>,
     ) -> Result<()> {
         instructions::close_voter(ctx)
     }
 
+    pub fn close_voter_v2<'key, 'accounts, 'remaining, 'info>(
+        ctx: Context<'key, 'accounts, 'remaining, 'info, CloseVoter<'info>>,
+    ) -> Result<()> {
+        instructions::close_voter_v2(ctx)
+    }
+
     pub fn log_voter_info(
         ctx: Context<LogVoterInfo>,
         deposit_entry_begin: u8,
         deposit_entry_count: u8,
     ) -> Result<()> {
         instructions::log_voter_info(ctx, deposit_entry_begin, deposit_entry_count)
     }
-
-    pub fn set_time_offset(ctx: Context<SetTimeOffset>, time_offset: i64) -> Result<()> {
-        instructions::set_time_offset(ctx, time_offset)
-    }
 }

programs/voter-stake-registry/tests/program_test/addin.rs

@@ -1,3 +1,4 @@
+use std::cell::RefCell;
 use std::sync::Arc;
 
 use solana_sdk::pubkey::Pubkey;
@@ -13,6 +14,7 @@ use crate::*;
 pub struct AddinCookie {
     pub solana: Arc<solana::SolanaCookie>,
     pub program_id: Pubkey,
+    pub time_offset: RefCell<i64>,
 }
 
 pub struct RegistrarCookie {
@@ -537,6 +539,47 @@ impl AddinCookie {
             .await
     }
 
+    #[allow(dead_code)]
+    pub async fn close_voter_v2(
+        &self,
+        registrar: &RegistrarCookie,
+        voter: &VoterCookie,
+        voting_mint: &VotingMintConfigCookie,
+        voter_authority: &Keypair,
+        token_address: Pubkey,
+    ) -> std::result::Result<(), BanksClientError> {
+        let vault = voter.vault_address(&voting_mint);
+
+        let data =
+            anchor_lang::InstructionData::data(&voter_stake_registry::instruction::CloseVoterV2 {});
+
+        let mut accounts = anchor_lang::ToAccountMetas::to_account_metas(
+            &voter_stake_registry::accounts::CloseVoter {
+                registrar: registrar.address,
+                voter: voter.address,
+                voter_authority: voter_authority.pubkey(),
+                sol_destination: voter_authority.pubkey(),
+                token_program: spl_token::id(),
+            },
+            None,
+        );
+        accounts.push(anchor_lang::prelude::AccountMeta::new(vault, false));
+        accounts.push(anchor_lang::prelude::AccountMeta::new(token_address, false));
+
+        let instructions = vec![Instruction {
+            program_id: self.program_id,
+            accounts,
+            data,
+        }];
+
+        // clone the secrets
+        let signer = Keypair::from_base58_string(&voter_authority.to_base58_string());
+
+        self.solana
+            .process_transaction(&instructions, Some(&[&signer]))
+            .await
+    }
+
     pub fn update_voter_weight_record_instruction(
         &self,
         registrar: &RegistrarCookie,
@@ -774,36 +817,25 @@ impl AddinCookie {
     #[allow(dead_code)]
     pub async fn set_time_offset(
         &self,
-        registrar: &RegistrarCookie,
-        authority: &Keypair,
+        _registrar: &RegistrarCookie,
+        _authority: &Keypair,
         time_offset: i64,
     ) {
-        let data =
-            anchor_lang::InstructionData::data(&voter_stake_registry::instruction::SetTimeOffset {
-                time_offset,
-            });
+        let old_offset = *self.time_offset.borrow();
+        *self.time_offset.borrow_mut() = time_offset;
 
-        let accounts = anchor_lang::ToAccountMetas::to_account_metas(
-            &voter_stake_registry::accounts::SetTimeOffset {
-                registrar: registrar.address,
-                realm_authority: authority.pubkey(),
-            },
-            None,
-        );
-
-        let instructions = vec![Instruction {
-            program_id: self.program_id,
-            accounts,
-            data,
-        }];
-
-        // clone the secrets
-        let signer = Keypair::from_base58_string(&authority.to_base58_string());
-
-        self.solana
-            .process_transaction(&instructions, Some(&[&signer]))
+        let old_clock = self
+            .solana
+            .context
+            .borrow_mut()
+            .banks_client
+            .get_sysvar::<solana_program::clock::Clock>()
             .await
             .unwrap();
+
+        let mut new_clock = old_clock.clone();
+        new_clock.unix_timestamp += time_offset - old_offset;
+        self.solana.context.borrow_mut().set_sysvar(&new_clock);
     }
 }
 

programs/voter-stake-registry/tests/program_test/mod.rs

@@ -228,6 +228,7 @@ impl TestContext {
             addin: AddinCookie {
                 solana: solana.clone(),
                 program_id: addin_program_id,
+                time_offset: RefCell::new(0),
             },
             mints,
             users,