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

0 stars 0 forks source link

Formal Charcoal Boa - The pool owner can claim all `unclaimed_fee` that has accrued in the pool, resulting in no protocol fee #63

Open sherlock-admin2 opened 13 hours ago

sherlock-admin2 commented 13 hours ago

Formal Charcoal Boa

High

The pool owner can claim all unclaimed_fee that has accrued in the pool, resulting in no protocol fee

Summary

The ClaimFee struct in claim_fee.rs is designed to allow the pool owner to act as the fee claimer. This enables the pool owner to claim all unclaimed_fee accrued in the pool, leaving no protocol fee.

Root Cause

As indicated in claim_fee.rs:16, the pool owner (woopool.authority) can serve as the claimer (pub authority: Signer<'info>). This means that the pool owner can claim all unclaimed_fee accrued in the pool, resulting in no protocol fee.

#[derive(Accounts)]
pub struct ClaimFee<'info> {
    pub wooconfig: Box<Account<'info, WooConfig>>,
    pub token_mint: Account<'info, Mint>,

    pub authority: Signer<'info>,

    #[account(mut,
        has_one = wooconfig,
16      constraint = woopool.authority == authority.key()
                  || wooconfig.fee_authority.contains(authority.key),
        constraint = woopool.token_mint == token_mint.key()
    )]
    pub woopool: Box<Account<'info, WooPool>>,

    #[account(mut,
        address = woopool.token_vault,
        constraint = token_vault.mint == token_mint.key()
      )]
    pub token_vault: Box<Account<'info, TokenAccount>>,

    #[account(mut, constraint = claim_fee_to_account.mint == token_mint.key())]
    pub claim_fee_to_account: Box<Account<'info, TokenAccount>>,

    #[account(address = token::ID)]
    pub token_program: Program<'info, Token>,
}

Internal pre-conditions

External pre-conditions

Attack Path

  1. Alice, a malicious user, creates pools.
  2. Users swap tokens through Alice's pools, accumulating fees in her quote pool.
  3. Alice then claims all accrued fees, leaving no protocol fee.

Impact

Pool owners can claim all unclaimed_fee accrued in the pool, resulting in no protocol fee.

PoC

Mitigation

Can fix as follows.

#[derive(Accounts)]
pub struct ClaimFee<'info> {
    pub wooconfig: Box<Account<'info, WooConfig>>,
    pub token_mint: Account<'info, Mint>,

    pub authority: Signer<'info>,

    #[account(mut,
        has_one = wooconfig,
-       constraint = woopool.authority == authority.key()
-                 || wooconfig.fee_authority.contains(authority.key),
+       constraint = wooconfig.fee_authority.contains(authority.key),
        constraint = woopool.token_mint == token_mint.key()
    )]
    pub woopool: Box<Account<'info, WooPool>>,

    #[account(mut,
        address = woopool.token_vault,
        constraint = token_vault.mint == token_mint.key()
      )]
    pub token_vault: Box<Account<'info, TokenAccount>>,

    #[account(mut, constraint = claim_fee_to_account.mint == token_mint.key())]
    pub claim_fee_to_account: Box<Account<'info, TokenAccount>>,

    #[account(address = token::ID)]
    pub token_program: Program<'info, Token>,
}