Governance: validate holding account (#2157)

* fix: validate governing token holding account for deposits

* fix: validate governing token holding account for withdrawals

governance/program/src/error.rs

@@ -295,6 +295,10 @@ pub enum GovernanceError {
     /// Realm authority must sign
     #[error("Realm authority must sign")]
     RealmAuthorityMustSign,
+
+    /// Invalid governing token holding account
+    #[error("Invalid governing token holding account")]
+    InvalidGoverningTokenHoldingAccount,
 }
 
 impl PrintProgramError for GovernanceError {

governance/program/src/processor/process_deposit_governing_tokens.rs

@@ -50,7 +50,12 @@ pub fn process_deposit_governing_tokens(
     let realm_data = get_realm_data(program_id, realm_info)?;
     let governing_token_mint = get_spl_token_mint(governing_token_holding_info)?;
 
-    realm_data.assert_is_valid_governing_token_mint(&governing_token_mint)?;
+    realm_data.assert_is_valid_governing_token_mint_and_holding(
+        program_id,
+        realm_info.key,
+        &governing_token_mint,
+        governing_token_holding_info.key,
+    )?;
 
     let amount = get_spl_token_amount(governing_token_source_info)?;
 

governance/program/src/processor/process_withdraw_governing_tokens.rs

@@ -39,6 +39,13 @@ pub fn process_withdraw_governing_tokens(
     let realm_data = get_realm_data(program_id, realm_info)?;
     let governing_token_mint = get_spl_token_mint(governing_token_holding_info)?;
 
+    realm_data.assert_is_valid_governing_token_mint_and_holding(
+        program_id,
+        realm_info.key,
+        &governing_token_mint,
+        governing_token_holding_info.key,
+    )?;
+
     let token_owner_record_address_seeds = get_token_owner_record_address_seeds(
         realm_info.key,
         &governing_token_mint,

governance/program/src/state/realm.rs

@@ -68,6 +68,26 @@ impl Realm {
 
         Err(GovernanceError::InvalidGoverningTokenMint.into())
     }
+
+    /// Asserts the given governing token mint and holding accounts are valid for the realm
+    pub fn assert_is_valid_governing_token_mint_and_holding(
+        &self,
+        program_id: &Pubkey,
+        realm: &Pubkey,
+        governing_token_mint: &Pubkey,
+        governing_token_holding: &Pubkey,
+    ) -> Result<(), ProgramError> {
+        self.assert_is_valid_governing_token_mint(governing_token_mint)?;
+
+        let governing_token_holding_address =
+            get_governing_token_holding_address(program_id, realm, governing_token_mint);
+
+        if governing_token_holding_address != *governing_token_holding {
+            return Err(GovernanceError::InvalidGoverningTokenHoldingAccount.into());
+        }
+
+        Ok(())
+    }
 }
 
 /// Checks whether realm account exists, is initialized and  owned by Governance program

governance/program/tests/process_deposit_governing_tokens.rs

@@ -270,3 +270,53 @@ async fn test_deposit_initial_community_tokens_with_invalid_owner_error() {
     // Assert
     assert_eq!(error, GovernanceError::GoverningTokenOwnerMustSign.into());
 }
+
+#[tokio::test]
+async fn test_deposit_community_tokens_with_malicious_holding_account_error() {
+    // Arrange
+    let mut governance_test = GovernanceProgramTest::start_new().await;
+    let realm_cookie = governance_test.with_realm().await;
+
+    let token_owner_record_cookie = governance_test
+        .with_community_token_deposit(&realm_cookie)
+        .await;
+
+    governance_test
+        .mint_tokens(
+            &realm_cookie.account.community_mint,
+            &realm_cookie.community_mint_authority,
+            &token_owner_record_cookie.token_source,
+            50,
+        )
+        .await;
+
+    let mut deposit_ix = deposit_governing_tokens(
+        &governance_test.program_id,
+        &realm_cookie.address,
+        &token_owner_record_cookie.token_source,
+        &token_owner_record_cookie.token_owner.pubkey(),
+        &token_owner_record_cookie.token_owner.pubkey(),
+        &governance_test.context.payer.pubkey(),
+        &realm_cookie.account.community_mint,
+    );
+
+    // Try to maliciously deposit to the source
+    deposit_ix.accounts[1].pubkey = token_owner_record_cookie.token_source;
+
+    // Act
+
+    let err = governance_test
+        .process_transaction(
+            &[deposit_ix],
+            Some(&[&token_owner_record_cookie.token_owner]),
+        )
+        .await
+        .err()
+        .unwrap();
+
+    // Assert
+    assert_eq!(
+        err,
+        GovernanceError::InvalidGoverningTokenHoldingAccount.into()
+    );
+}

governance/program/tests/process_withdraw_governing_tokens.rs

@@ -262,3 +262,52 @@ async fn test_withdraw_governing_tokens_after_relinquishing_vote() {
         source_account.amount
     );
 }
+
+#[tokio::test]
+async fn test_withdraw_tokens_with_malicious_holding_account_error() {
+    // Arrange
+    let mut governance_test = GovernanceProgramTest::start_new().await;
+    let realm_cookie = governance_test.with_realm().await;
+
+    let token_owner_record_cookie = governance_test
+        .with_community_token_deposit(&realm_cookie)
+        .await;
+
+    // Try to maliciously withdraw from other token account owned by realm
+
+    let realm_token_account_cookie = governance_test
+        .with_token_account(
+            &realm_cookie.account.community_mint,
+            &realm_cookie.address,
+            &realm_cookie.community_mint_authority,
+            200,
+        )
+        .await;
+
+    let mut instruction = withdraw_governing_tokens(
+        &governance_test.program_id,
+        &realm_cookie.address,
+        &token_owner_record_cookie.token_source,
+        &token_owner_record_cookie.token_owner.pubkey(),
+        &realm_cookie.account.community_mint,
+    );
+
+    instruction.accounts[1].pubkey = realm_token_account_cookie.address;
+
+    // Act
+    let err = governance_test
+        .process_transaction(
+            &[instruction],
+            Some(&[&token_owner_record_cookie.token_owner]),
+        )
+        .await
+        .err()
+        .unwrap();
+
+    // Assert
+
+    assert_eq!(
+        err,
+        GovernanceError::InvalidGoverningTokenHoldingAccount.into()
+    );
+}

governance/program/tests/program_test/cookies.rs

@@ -145,3 +145,8 @@ pub struct ProposalInstructionCookie {
     pub account: ProposalInstruction,
     pub instruction: Instruction,
 }
+
+#[derive(Debug)]
+pub struct TokenAccountCookie {
+    pub address: Pubkey,
+}

governance/program/tests/program_test/mod.rs

@@ -62,7 +62,7 @@ use self::{
     cookies::{
         GovernanceCookie, GovernedAccountCookie, GovernedMintCookie, GovernedProgramCookie,
         GovernedTokenCookie, ProposalCookie, ProposalInstructionCookie, RealmCookie,
-        TokeOwnerRecordCookie, VoteRecordCookie,
+        TokeOwnerRecordCookie, TokenAccountCookie, VoteRecordCookie,
     },
     tools::NopOverride,
 };
@@ -1907,6 +1907,32 @@ impl GovernanceProgramTest {
         .unwrap();
     }
 
+    #[allow(dead_code)]
+    pub async fn with_token_account(
+        &mut self,
+        token_mint: &Pubkey,
+        owner: &Pubkey,
+        token_mint_authority: &Keypair,
+        amount: u64,
+    ) -> TokenAccountCookie {
+        let token_account_keypair = Keypair::new();
+
+        self.create_empty_token_account(&token_account_keypair, token_mint, owner)
+            .await;
+
+        self.mint_tokens(
+            token_mint,
+            token_mint_authority,
+            &token_account_keypair.pubkey(),
+            amount,
+        )
+        .await;
+
+        return TokenAccountCookie {
+            address: token_account_keypair.pubkey(),
+        };
+    }
+
     #[allow(dead_code)]
     pub async fn create_token_account_with_transfer_authority(
         &mut self,