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

2 stars 2 forks source link

0xeix - Anybody can claim rebate fees #41

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

0xeix

High

Anybody can claim rebate fees

Summary

In the current version of the claim_rebate_fee instruction, anybody can claim rebate fees as there is no constraints on claim_fee_to_account.

Vulnerability Detail

Take a look at the ClaimRebateFee struct and its constraints:

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/rebate_manager/src/instructions/claim_rebate_fee.rs#L8-35

#[derive(Accounts)]
pub struct ClaimRebateFee<'info> {
    pub quote_token_mint: Account<'info, Mint>,

    pub rebate_authority: Signer<'info>,

    #[account(mut,
        has_one = quote_token_mint,
    )]
    pub rebate_manager: Account<'info, RebateManager>,

    #[account(mut,
        address = rebate_manager.token_vault
    )]
    token_vault: Box<Account<'info, TokenAccount>>,

    #[account(mut,
        has_one = rebate_manager,
        has_one = rebate_authority,
        constraint = rebate_info.authority == rebate_manager.authority
    )]
    pub rebate_info: Account<'info, RebateInfo>,

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

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

As you can see here, the only requirement is for claim_fee_to_account.mint == quote_token_mint.key(). The other important requirement is for rebate_info.authority == rebate_manager.authority. The problem is that these checks do not make sure that the signer is actually an authority, they just verify if the two instructions have the same authority and if the mint field of Account corresponds to the quote_token_mint.key(). As per spec:

Functions need admin authority: claim_fee claim_rebate_fee create_oracle create_pool create_rebate_pool deposit set_pool_admin set_pool_state (all handlers in this file) set_woo_admin set_woo_state(all handlers in this file)

Impact

This basically allows anybody to claim the fees instead of admin.

Code Snippet

Provided above.

Tool used

Manual Review

Recommendation

Introduce the following check (or another constraint):

 constraint = rebate_manager.authority == authority.key()
toprince commented 1 month ago

Not valid here.

Please check below code. the has_one = rebate_authority is doing the check.

#[account(mut,
    has_one = rebate_manager,
    has_one = rebate_authority,
    constraint = rebate_info.authority == rebate_manager.authority
)]
pub rebate_info: Account<'info, RebateInfo>,