code-423n4 / 2023-06-lybra-findings

8 stars 7 forks source link

reward ratio may return wrong ratio and even zero because of rounding down error #573

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/EUSDMiningIncentives.sol#L232-L235

Vulnerability details

Impact

the function notifyRewardAmount in the EUSDMiningIncentives contract doing a calculate to return the rewardRatio but this calculation may face round down error and cause returning wrong ratio for the reward or even return zero. this can happen if block.timestamp < finishAt.

Proof of Concept

the function notifyRewardAmount calculates the rewardRatio like this:

 function notifyRewardAmount(
        uint256 amount
    ) external onlyOwner updateReward(address(0)) {
        require(amount > 0, "amount = 0");
        if (block.timestamp >= finishAt) {
            rewardRatio = amount / duration;
        } else {
            //@audit

            uint256 remainingRewards = (finishAt - block.timestamp) * rewardRatio;
            rewardRatio = (amount + remainingRewards) / duration;
        }
    }

if the code runs the else so its possible to round down the result and setting the reward ratio to incorrect ratio. for example:

case 1: the remainingRewards will be equal to finishAt(set it to 2 month) - block.timestamp(20 days or 0.2 ) * rwardRatio(lets say its 1) this will return 2 - 0.2 * 1 = 1.8because of rounding down in solidity it will become 1.

case 2:

the remainingRewards will be equal to finishAt(set it to 2 month) - block.timestamp(2month -1 or 1.99 ) * rwardRatio(lets say its 1) this will return 2 - 1.99 * 1 = 0.1 because of rounding down in solidity it will become 0.

in both case the users get much less esLBR token as reward.

Tools Used

manual review

similar issue

https://code4rena.com/reports/2022-03-paladin#h-01-droppersecond-is-not-updated-homogeneously-the-rewards-emission-can-be-much-higher-than-expected-in-some-cases

Recommended Mitigation Steps

recommend to multiple the calculation by 1e18 to avoid round down error in solidity or using unhecked block to avoid this happen (with cautionof under/over flow)

i prefer something like this:

else {

            uint256 remainingRewards = ((finishAt - block.timestamp) * rewardRatio) * 1e18 / 1e18;
            rewardRatio = (amount + remainingRewards) / duration;
        }

Assessed type

Other

JeffCX commented 1 year ago

Loss of precision is captured by bot

c4-pre-sort commented 1 year ago

JeffCX marked the issue as low quality report

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Out of scope