sherlock-audit / 2024-06-boost-aa-wallet-judging

3 stars 1 forks source link

lola - CGDAIncentive: The logic for calculating cgdaParams.currentReward implies a range that can be bypassed in certain situations #426

Open sherlock-admin4 opened 2 months ago

sherlock-admin4 commented 2 months ago

lola

Medium

CGDAIncentive: The logic for calculating cgdaParams.currentReward implies a range that can be bypassed in certain situations

Summary

The logic for calculating cgdaParams.currentReward implies a range that can be bypassed in certain situations. Here is the claim function:

CGDAIncentive.sol#L85C1-L100C6

   function claim(address claimTarget, bytes calldata) external virtual override onlyOwner returns (bool) {
        if (!_isClaimable(claimTarget)) revert NotClaimable();
        claims++;

        // Calculate the current reward and update the state
        uint256 reward = currentReward();
        cgdaParams.lastClaimTime = block.timestamp;
        cgdaParams.currentReward =
            reward > cgdaParams.rewardDecay ? reward - cgdaParams.rewardDecay : cgdaParams.rewardDecay;

        // Transfer the reward to the recipient
        asset.safeTransfer(claimTarget, reward);

        emit Claimed(claimTarget, abi.encodePacked(asset, claimTarget, reward));
        return true;
    }

There is a bug in all situations that end up like this example of CGDAParameters:

    reward = 7
    struct CGDAParameters {
            uint256 rewardDecay = 5;
            uint256 rewardBoost = 2;
            uint256 lastClaimTime = does not matter;
            uint256 currentReward = does not matter;
     }

Steps:

  1. reward = 7.

  2. Remember:

    cgdaParams.currentReward =
            reward > cgdaParams.rewardDecay ? reward - cgdaParams.rewardDecay : cgdaParams.rewardDecay;
  3. This implies through code that we want cgdaParams.currentReward to stay between cgdaParams.rewardDecay and totalBudget (inclusive). TLDR; we want cgdaParams.rewardDecay <= cgdaParams.currentReward <= totalBudget.

  4. But in the situation above currentReward will be set to reward - rewardDecay. currentReward = 7 - 5 = 2. currentReward is now less than rewardDecay which breaks the implied range as stated. The next claimer gets this amount of tokens which may be lesser than all other users ever get in a worst-case claim. However, after this mishap, the next claim gets reset to rewardDecay but this particular claimer in this specific condition would always get less than rewardDecay.

Root Cause

No response

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

No response

PoC

No response

Mitigation

No response