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

2 stars 2 forks source link

Albort - Inconsistent Authority Constraints in `ClaimRebateFee` #79

Open sherlock-admin3 opened 2 months ago

sherlock-admin3 commented 2 months ago

Albort

Medium

Inconsistent Authority Constraints in ClaimRebateFee

Summary

Vulnerability Detail

In the ClaimRebateFee instruction, you have the following constraint:

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

This constraint requires that rebate_info.authority == rebate_manager.authority. However, when you create a RebateInfo account in the CreateRebateInfo instruction, the authority is set to the authority who calls the instruction, not necessarily the rebate_manager.authority:

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

    rebate_info.authority = ctx.accounts.authority.key();
    rebate_info.rebate_authority = ctx.accounts.rebate_authority.key();
    rebate_info.rebate_manager = ctx.accounts.rebate_manager.key();
    rebate_info.pending_rebate = 0;

    Ok(())
}

This mismatch means that unless the rebate_manager.authority itself creates the RebateInfo, any other users who create a RebateInfo will have an authority that doesn't match rebate_manager.authority, preventing them from claiming their rebates. This effectively blocks legitimate rebate claims from users who are supposed to be able to claim their pending rebates.

Impact

Code Snippet

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

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/rebate_manager/src/instructions/admin/create_rebate_info.rs#L38

Tool used

Manual Review

Recommendation

Modify the CreateRebateInfo instruction to set the rebate_info.authority to rebate_manager.authority instead of ctx.accounts.authority.key(). This ensures consistency and allows the ClaimRebateFee instruction to proceed as intended.

toprince commented 2 months ago

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/rebate_manager/src/instructions/admin/create_rebate_info.rs#L9-L19 constraint = rebate_manager.authority == authority.key() || rebate_manager.admin_authority.contains(authority.key),

Valid, due to above constraints, should be low impact.

sherlock-admin2 commented 1 month ago

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

gjaldon commented 1 month ago

Issue is fixed. This change fixes the issue by also allowing admin authorities to claim rebate fees.