code-423n4 / 2024-03-abracadabra-money-findings

9 stars 7 forks source link

Griefing attack: small stakes reward griefing due to rounding down to 0 #166

Closed c4-bot-7 closed 7 months ago

c4-bot-7 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/staking/LockingMultiRewards.sol#L528-#L534

Vulnerability details

Vulnerability details

Whenever any operation with the given user as a beneficiary is performed, this user's rewards are checkpointed via function _updateRewardsGlobal(), which calculates the reward checkpoint by a call to function _rewardPerToken

function _rewardPerToken(address rewardToken, uint256 lastTimeRewardApplicable_, uint256 totalSupply_) public view returns (uint256) {
    if (totalSupply_ == 0) {
        return _rewardData[rewardToken].rewardPerTokenStored;
    }

    uint256 timeElapsed = lastTimeRewardApplicable_ - _rewardData[rewardToken].lastUpdateTime;
    uint256 pendingRewardsPerToken = (timeElapsed * _rewardData[rewardToken].rewardRate * 1e18) / totalSupply_;

    return _rewardData[rewardToken].rewardPerTokenStored + pendingRewardsPerToken;
}

rewardRate is set in notifyRewardAmount function as below:

    reward.rewardRate = amount / _remainingRewardTime;

It is clearly that pendingRewardsPerToken can be equal to 0. Consider this mock scenario:

So the result of the function is x + 12 * 2 * 1e18 / 1e22 Which is equal to x + 24e18 / 1e22 = x due to rounding down

Which is incorrect as the calculation does not account for the rewards accumulated since the last checkpoint.

Attacker can griefing attack by continously calling stake or lock function with dust amount, since they only revert when amount = 0:

function _stakeFor(address account, uint256 amount, bool lock_) internal {
    if (amount == 0) {
        revert ErrZeroAmount();  // <---
    }

function lock(uint256 amount) public whenNotPaused {
    if (amount == 0) {
        revert ErrZeroAmount();  // <--
    }

Impact

Incorrect checkpoint value being stored for the beneficiary, which will affect future reward calculations for this beneficiary.

Tools Used

Manual review

Recommended Mitigation Steps

Use validation checks to ensure that the required calculation doesn't round down to zero:

   require(pendingRewardsPerToken >= totalSupply_, "rounding down to 0")

Assessed type

Other

c4-pre-sort commented 8 months ago

141345 marked the issue as primary issue

141345 commented 8 months ago

low decimal with big totalSupply group several reports on the same block of code, but some dup only addresses low decimal, or only totalSupply

c4-pre-sort commented 8 months ago

141345 marked the issue as sufficient quality report

0xCalibur commented 8 months ago

no factor

c4-sponsor commented 8 months ago

0xCalibur (sponsor) disputed

thereksfour commented 8 months ago

I would think it's dust loss, the rewardRate in the example is very small

c4-judge commented 8 months ago

thereksfour changed the severity to QA (Quality Assurance)

c4-judge commented 7 months ago

This previously downgraded issue has been upgraded by thereksfour

c4-judge commented 7 months ago

thereksfour marked issue #222 as primary and marked this issue as a duplicate of 222

c4-judge commented 7 months ago

thereksfour marked the issue as partial-75