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

0 stars 0 forks source link

Little Aquamarine Jellyfish - Unrestricted Access to Claim Rebate Fee Function #85

Open sherlock-admin3 opened 13 hours ago

sherlock-admin3 commented 13 hours ago

Little Aquamarine Jellyfish

Medium

Unrestricted Access to Claim Rebate Fee Function

Summary

The claim_rebate_fee function in the rebate manager system lacks proper access control mechanisms. As currently implemented, any user can call this function to claim rebates to any specified account. This is a significant security vulnerability as it bypasses intended authorization checks, potentially allowing unauthorized users to drain rebate funds. The issue stems from the absence of caller verification within the function and over-reliance on account validation in the Context struct without runtime checks. This vulnerability could lead to financial losses and compromise the integrity of the rebate distribution system.

Relevant links

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

Details

The claim_rebate_fee function is designed to be called by a rebate manager to claim rebates to a specified account (claim_fee_to_account). However, the current implementation has a severe access control flaw:

Impact

This access control vulnerability can lead to:

  1. Unauthorized claiming of rebates by any user, not just the intended rebate manager.

  2. Potential draining of the entire rebate pool by malicious actors.

  3. Redirection of rebates to arbitrary accounts, resulting in loss of funds for the protocol or legitimate rebate recipients.

  4. Complete compromise of the rebate distribution system.

Code snippet

    pub fn handler(ctx: Context<ClaimRebateFee>) -> Result<()> {
    let rebate_manager = &mut ctx.accounts.rebate_manager;
    let token_vault = &mut ctx.accounts.token_vault;
    let rebate_info = &mut ctx.accounts.rebate_info;
    let claim_fee_to_account = &ctx.accounts.claim_fee_to_account;

    require!(
        token_vault.amount as u128 >= rebate_info.pending_rebate,
        ErrorCode::RebateFeeNotEnough
    );

    let claim_amount = rebate_info.pending_rebate;
    rebate_info.clear_pending_rebate()?;

    transfer_from_vault_to_owner(
        rebate_manager,
        token_vault,
        claim_fee_to_account,
        &ctx.accounts.token_program,
        claim_amount as u64,
    )?;

    Ok(())
}

Recommendation

To address this critical access control issue:

Implement an explicit check to verify the caller's authority as the rebate manager:

require!(
    ctx.accounts.authority.key() == rebate_manager.authority,
    ErrorCode::UnauthorizedRebateManager
);

Ensure the ClaimRebateFee Context struct includes the authority account and proper constraints:

    pub struct ClaimRebateFee<'info> {
    pub authority: Signer<'info>,
    #[account(
        constraint = rebate_manager.authority == authority.key()
    )]
    pub rebate_manager: Account<'info, RebateManager>,
    // ... other accounts ...
}

Add a check to ensure the claim_fee_to_account is an authorized recipient:

    require!(
    rebate_manager.is_authorized_recipient(claim_fee_to_account.key()),
    ErrorCode::UnauthorizedRecipient
);