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

2 stars 2 forks source link

chinepun - Withdraw Instruction can withdraw total amount in pool(Including unclaimed_fees) #75

Open sherlock-admin2 opened 2 months ago

sherlock-admin2 commented 2 months ago

chinepun

High

Withdraw Instruction can withdraw total amount in pool(Including unclaimed_fees)

Summary

The admin gated withdraw instruction could withdraw the entire pool balance without leaving any amount left in the pool to collect the fees to be callled by the claim_fee instruction.

Root Cause

In deposit_withdraw.rs, the variable has an underscore hence the rust compiler ignores the variable been unused.

Internal pre-conditions

Admin calls withdraw instruction where amount + woopool.unclaimed_fees > woopool.amount.

Admin calls claim_fee instruction but cannot withdraw any fees because the pool is currently empty

Impact

These prevents the protocol from been able to appropriately withdraw it's unclaimed fees in some scenarios.

Mitigation

Add this below

    require!( amount + woopool.unclaimed_fee >= token_vault.amount, ErrorCode::AdminWithdrawsTooMuchFromPool )

       transfer_from_vault_to_owner(
        woopool,
        token_vault,
        token_owner_account,
        &ctx.accounts.token_program,
        amount as u64,
    )?;

to the deposit_withdraw.rs

and comment out or delete this line

toprince commented 2 months ago

Need investigation further. Admin calls, low impact. This related how we define withdraw's logic.