sherlock-audit / 2024-03-zivoe-judging

8 stars 6 forks source link

Ironsidesec - `ZivoeRewardsVesting` : reward rate can be deflated #343

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

Ironsidesec

high

ZivoeRewardsVesting : reward rate can be deflated

Summary

Issue happens on ZivoeRewardsVesting contract's depositReward

root cause: missing access control to depositReward Impact :

  1. make reward emission very slow by decreasing reward rate,
  2. causing DOS to yield distribution on ZivoeYDL.distributeYield

Vulnerability Detail

https://github.com/sherlock-audit/2024-03-zivoe/blob/01e00e6f27b58392a6fa0b82c84a46a783a0df3c/zivoe-core-foundry/src/ZivoeRewardsVesting.sol#L352


File: 2024-03-zivoe\zivoe-core-foundry\src\ZivoeRewardsVesting.sol

355: >>>  function depositReward( address _rewardsToken, uint256 reward) external updateReward(address(0)) nonReentrant {
354:         IERC20(_rewardsToken).safeTransferFrom(_msgSender(), address(this), reward);
355: 
356:         // Update vesting accounting for reward (if existing rewards being distributed, increase proportionally).
357:         if (block.timestamp >= rewardData[_rewardsToken].periodFinish) {
358:             rewardData[_rewardsToken].rewardRate = reward.div(rewardData[_rewardsToken].rewardsDuration);
359:         } else {
360:             uint256 remaining = rewardData[_rewardsToken].periodFinish.sub(block.timestamp);
361:             uint256 leftover = remaining.mul(rewardData[_rewardsToken].rewardRate);
362:             rewardData[_rewardsToken].rewardRate = reward.add(leftover).div(rewardData[_rewardsToken].rewardsDuration);
363:         }
364:         
365:         rewardData[_rewardsToken].lastUpdateTime = block.timestamp;
366:         rewardData[_rewardsToken].periodFinish = block.timestamp.add(rewardData[_rewardsToken].rewardsDuration);
367:         emit RewardDeposited(_rewardsToken, reward, _msgSender());
368:     }

reward rate = reward balance / reward duration.

Attack path: 1

  1. Currently, block.timestamp  <  rewardData[_rewardsToken].periodFinish, meaning for a 30-day reward period with 1000 tokens per day reward period, currently we are at the 20th day, so 10 * 1000 tokens are yet to stream.

  2. Now call depositReward with 0 rewards, which will keep the reward amount same, but it will increase the periodFinish to now + 30 days, so reward rate becomes (10 * 1000 + 0 ) / 30 = 333 tokens per day.

  3. So there is a sudden 1/3 rate decrease and stakers will leave if this happens. Also, you can repeatedly call everyday and keep on deflating the reward rate.

  4. so the same rewards that were about to stream for the remaining 10 days were elongated to 30 more days, so stakers have to wait a long time to claim their rewards.

This is an issue, deflating reward rates, so stakers might exit seeing the reward APRs. Even if after 10 days a new reward is deposited by YDL which is done every month end, those 10 days rewards are made 1/3 rd slower by elongating it to 30 days.

Attack path: 2

  1. It is 5 days away from distribution in YDL. So the keeper will call to deposit rewards in 5 more days by calling ZivoeYDL.distributeYield.
  2. Now, an attacker can call  deposit reward with a very low value and it will deflate the reward rate due to same numerator but big denominator, After 5 days, YDL will distribute rewards further, making the reward duration very high. So genuinely, it should be x + 30 days, but now it's 5 + x + 30 days, the same rewards but spread out in extra days, discouraging stakers to exit.

This cannot be fixed by reward rate and blance checks, or input validation to be rewards > 0, atatcker will still do this attack even if the cost is 1000$. So the recommendation is to make this callable only by YDL contract or ZVL .

Another impact of causing DOS  on YDL reward distribution is,  1. There's a missing check that reward rate <= rewardbalance/reward period that will be added by Zivoe related to other issue ex: synthetix/contracts/StakingRewards.sol#L127.

 And if this is added, someone can call deposit reward legitimately with 0 as reward, and it will decrese the reward rate the same reward token balance, and it will trigger this revert, and whole ZivoeYDL.distributeYield will fail making the YDL distribution revrting to cause DOS.

Impact

DOS to yield distribution. And reward emissions can be made very slow by increasing the reward duration.

Code Snippet

https://github.com/sherlock-audit/2024-03-zivoe/blob/01e00e6f27b58392a6fa0b82c84a46a783a0df3c/zivoe-core-foundry/src/ZivoeRewardsVesting.sol#L352

Tool used

Manual Review

Recommendation

Add access control, depositReward to only callable by YDL contract.

https://github.com/sherlock-audit/2024-03-zivoe/blob/01e00e6f27b58392a6fa0b82c84a46a783a0df3c/zivoe-core-foundry/src/ZivoeRewardsVesting.sol#L352

    function depositReward(address _rewardsToken, uint256 reward) external updateReward(address(0)) nonReentrant {
+       require(
+           _msgSender() == IZivoeGlobals_ZivoeRewards(GBL).ZVL() || _msgSender() == IZivoeGlobals_ZivoeRewards(GBL).YDL(),
+           "_msgSender() != ZVL || YDL");

        IERC20(_rewardsToken).safeTransferFrom(_msgSender(), address(this), reward);

        // Update vesting accounting for reward (if existing rewards being distributed, increase proportionally).
        if (block.timestamp >= rewardData[_rewardsToken].periodFinish) {
            rewardData[_rewardsToken].rewardRate = reward.div(rewardData[_rewardsToken].rewardsDuration);
        } else {
            uint256 remaining = rewardData[_rewardsToken].periodFinish.sub(block.timestamp);
            uint256 leftover = remaining.mul(rewardData[_rewardsToken].rewardRate);
            rewardData[_rewardsToken].rewardRate = reward.add(leftover).div(rewardData[_rewardsToken].rewardsDuration);
        }

        rewardData[_rewardsToken].lastUpdateTime = block.timestamp;
        rewardData[_rewardsToken].periodFinish = block.timestamp.add(rewardData[_rewardsToken].rewardsDuration);
        emit RewardDeposited(_rewardsToken, reward, _msgSender());
    }

Duplicate of #11

sherlock-admin3 commented 6 months ago

1 comment(s) were left on this issue during the judging contest.

panprog commented:

high, dup of #11, allows anyone to extend reward finish time indefinitely and decrease reward rate.