sherlock-audit / 2024-06-makerdao-endgame-judging

1 stars 1 forks source link

hash - Lack of time gap restrictions on the `distribute` call allows for sizeable loss on the rewarded distribution #108

Closed sherlock-admin4 closed 1 month ago

sherlock-admin4 commented 1 month ago

hash

Medium

Lack of time gap restrictions on the distribute call allows for sizeable loss on the rewarded distribution

Summary

Lack of time gap restrictions on the distribute call allows for sizeable loss on the rewarded distribution

Vulnerability Detail

The distribute function which distributes the reward to the staking contract (from the vest) doesn't enforce any time gap b/w calls and is callable by the public

    function distribute() external returns (uint256 amount) {
        require(vestId != INVALID_VEST_ID, "VestedRewardsDistribution/invalid-vest-id");

        amount = dssVest.unpaid(vestId);
        require(amount > 0, "VestedRewardsDistribution/no-pending-amount");

        lastDistributedAt = block.timestamp;
        dssVest.vest(vestId, amount);

        require(gem.transfer(address(stakingRewards), amount), "VestedRewardsDistribution/transfer-failed");
        stakingRewards.notifyRewardAmount(amount);

        emit Distribute(amount);
    }

This allows a user to invoke the distribute function in the lowest possible interval (ie. block period == 12s) which would yield the lowest reward which could result to a reward rate of 0 (or a significant loss) depending on the other configurations like reward duration, total staked amount and the vesting parameters

Example POC

The following test shows a possible loss of > 0.5% for the same:

    function testRewardNullify() public {
        // 10million totalSupply, 5% return, 500k worth of mkr. 500k worth mkr = 500k/ 3k == 166mkr  
        uint reward = 250e18;
        uint yearTime = 365 days;
        // for 12 second, reward
        uint perBlock = reward * 12 / yearTime;
        uint rewardRate = perBlock / 7 days;

        uint daiSupply = 1e25;

        uint rewardPerToken = rewardRate * 12 * 1e18 / daiSupply;

        assert(rewardPerToken == 125);
        // == 125, possible loss is 1 due to rounding, so loss % = 1/125 * 100 > 0.5%

    }

Impact

A sizeable portion of the reward tokens can be lost

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/dba30d7a676c20dfed3bda8c52fd6702e2e85bb1/endgame-toolkit/src/VestedRewardsDistribution.sol#L152

Tool used

Manual Review

Recommendation

Enforce a timegap b/w the distributions

telome commented 1 month ago

Known issue. See Section 5.1 of https://github.com/makerdao/endgame-toolkit/blob/5dc625fd6a07c7c24a97a45553c2287f38807e44/audits/ChainSecurity_Maker_EndGame_Toolkit_audit_v3.pdf