sherlock-audit / 2024-08-woofi-solana-deployment-judging

2 stars 2 forks source link

g - Transfers from the rebate manager's token vault always fail due to lack of bump seed #18

Open sherlock-admin2 opened 1 month ago

sherlock-admin2 commented 1 month ago

g

High

Transfers from the rebate manager's token vault always fail due to lack of bump seed

Summary

The bump seed is not included in the signer seed for the transfer transaction which will cause all token transfers from the token vault to fail.

The Rebate Manager is a PDA that owns the Token Vault.

  #[account(
      init,
      payer = authority,
      token::mint = quote_token_mint,
      token::authority = rebate_manager
    )]
  // @audit token vault is owned by the rebate manager
  pub token_vault: Box<Account<'info, TokenAccount>>,

The rebate manager must sign the transaction when transferring tokens from the token vault. The signer seeds used for the transfer transaction are:

  pub fn seeds(&self) -> [&[u8]; 2] {
      [
          REBATEMANAGER_SEED.as_bytes(),
          self.quote_token_mint.as_ref(),
      ]
  }

All transactions signed by a PDA must include their bump seed like in Woopool's seeds.

Root Cause

In rebate_manager.rs:54-59, the bump seed is not included which causes all transfers/transactions signed with those seeds to fail.

  pub fn seeds(&self) -> [&[u8]; 2] {
      [
          REBATEMANAGER_SEED.as_bytes(),
          self.quote_token_mint.as_ref(),
      ]
  }

Internal pre-conditions

None

External pre-conditions

None

Attack Path

  1. Call any instruction in the rebate_manager program that calls transfer_from_vault_to_owner(). Instructions that call this transfer are:

Impact

Claiming rebate fees and withdrawing rebate fees will always fail due to this issue. Tokens meant for the rebate authority are stuck in the vault. This is a loss of funds.

PoC

No response

Mitigation

Consider adding the bump seed in rebate manager's seeds(). Woo Pool's seeds() can be used as a reference.

toprince commented 1 month ago

valid. will add.

sherlock-admin2 commented 1 month ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/woonetwork/WOOFi_Solana/pull/32

gjaldon commented 3 weeks ago

The rebate_manager's bump is now stored in the account and included in its seeds(). The issue is fixed and transfers from the rebate_manager will succeed.