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

0 stars 0 forks source link

Passive Indigo Dolphin - Pending Rebate and Woopool Unclaimed fee is of inapppropriate type #22

Open sherlock-admin3 opened 15 hours ago

sherlock-admin3 commented 15 hours ago

Passive Indigo Dolphin

High

Pending Rebate and Woopool Unclaimed fee is of inapppropriate type

Summary

The field pending_rebate in rebate_info.rs and unclaimed_fee in woopol.rs should be of type u64 rather than u128. This is to easily prevent undefined behaviour when casting to u64 later on which happens very often in the codebase.

Root Cause

    let claim_amount: u128 = 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,
    )?;

This is an example of such a scenario in claim_rebate_fee.rs where claim_amount is of type u128 and it is later casted to u64 in the transfer_from_vault_to_owner function, this casting could lead to precision errors or undefined behaviour when the pending_rebate > u64::MAX.

This is similar to the code in claim_fee.rs where unclaimed_fee > u64::MAX

Internal pre-conditions

If the instruction add_rebate is called, it calls add_pending_rebate and this could result in a value where u128::MAX > value > u64::MAX

Hence if this happens and the code above in claim_rebate_fee.rs is called, the claim_amount is casted to a u64 but the value is above the range for u64

Similarly this occurs when add_unclaimed_fee is called resulting in a value where u128::MAX > value > u64::MAX hence affecting claim_amount in claim_fee.rs

Impact

This could lead to a loss of value to the protocol as the expected behaviour is undefined.

Mitigation

use pub unclaimed_fee: u64 // 8 in woopool.rs and pub pending_rebate: u64 // 8 in rebate_info.rs

toprince commented 13 hours ago

Need further investigation.