dimulski - Rewards are calculated as distributed even if there are no stakers, locking the rewards forever

[sherlock-admin3]

dimulski

medium

Rewards are calculated as distributed even if there are no stakers, locking the rewards forever

Summary

The ZivoeRewards.sol and the ZivoeRewardsVesting.sol contracts are a fork of the Synthetix rewards distribution contract, with slight modifications. The contract logic keeps track of the total time passed, as well as the cumulative rate of rewards that have been generated for each token used for rewards, so that for every user, it just has to keep track of the last timestamp and last rate at the time of the last reward withdrawal, stake, or unstake in order to calculate the rewards owed since the user began staking. The code special-cases the scenario where there are no users, by not updating the cumulative rate when the _totalSupply is zero, but it does not include such a condition for the tracking of the timestamp. Because of this, even when there are no users staking, the accounting logic still thinks funds were being dispersed during that timeframe (because the starting timestamp is updated), which means the funds effectively are distributed to nobody, rather than being saved for when there is someone to receive them. And will be locked in the contract forever. One of the modifications is this line in the depositReward() function.

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

Contrary to the Synthetix implementation, Zivo requires each time reward is deposited to the contact, the reward amount to be transferred in the same transaction. However if a reward is deposited but there are no stakers, the reward that should have been distributed for the period until the first user stakes, will be locked in the contract forever, and won't be able to be added to the rewards for the next reward period because of the above code snippet.

Vulnerability Detail

Gist After following the steps in the above linked gist add the following test to the AuditorTests.t.sol contract:

    function test_LockedRewards() public {
        vm.startPrank(ZVL);
        zivoeToken.transfer(alice, 5_000_000e18);
        uint256 duration = 30 days;
        stZVE.addReward(address(mockUSDC), duration);
        vm.stopPrank();

        vm.startPrank(simulateYLD);
        mockUSDC.mint(simulateYLD, 50_000e6); // this represent 50_000 USDC tokens to be distributed in the next 30 days
        mockUSDC.approve(address(stZVE), type(uint256).max);
        stZVE.depositReward(address(mockUSDC), 50_000e6);
        skip(172_800); /// @notice two days pass, before anybody stakes in the contract
        vm.stopPrank();

        vm.startPrank(alice);
        console2.log("USDC balance of alice before staking: ", mockUSDC.balanceOf(alice));
        console2.log("USDC balance of stZVE contract: ", mockUSDC.balanceOf(address(stZVE)));
        zivoeToken.approve(address(stZVE), type(uint256).max);
        stZVE.stake(5_000_000e18);
        skip(duration);
        stZVE.fullWithdraw();
        console2.log("USDC balance of alice after staking for 30 days: ", mockUSDC.balanceOf(alice));
        console2.log("USDC balance of stZVE contract, when users have withdrawn everything for the first period: ", mockUSDC.balanceOf(address(stZVE)));
        console2.log("USDC balance of stZVE contract, when users have withdrawn everything for the first period normalized: ", mockUSDC.balanceOf(address(stZVE)) / 1e6);
        console2.log("zivoToken balance of alice after unstaking: ", zivoeToken.balanceOf(alice));
        vm.stopPrank();
    }

Logs:
  USDC balance of alice before staking:  0
  USDC balance of stZVE contract:  50000000000
  USDC balance of alice after staking for 30 days:  46665000000
  USDC balance of stZVE contract, when users have withdrawn everything for the first period:  3335000000
  USDC balance of stZVE contract, when users have withdrawn everything for the first period normalized:  3335
  zivoToken balance of alice after unstaking:  5000000000000000000000000

As can be seen from the above logs 3335 USDC tokens will be locked forever.

To run the test use: forge test -vvv --mt test_LockedRewards

Impact

If the depositReward() function is called prior to there being any users staking, the funds that should have gone to the first stakers will instead accrue to nobody, and be locked in the contract forever.

Code Snippet

https://github.com/sherlock-audit/2024-03-zivoe/blob/main/zivoe-core-foundry/src/ZivoeRewards.sol#L228-L243

Tool used

Manual Review & Foundry

Recommendation

Remove the safeTransfer() from the depositReward() function, and instead check if there are enough reward tokens already in the contract. Something similar to the Synthetix implementation.

[sherlock-admin4]

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

panprog commented:

borderline medium/low. If there are no stakers and depositReward is called, the deposit amount might be lost since it isn't distributed to anybody.

[panprog]

Keeping this medium as the loss of funds can happen, although the probability of this is very low, but still possible.

[spacegliderrrr]

Escalate

It would be an admin/ user mistake to deposit rewards when there are no stakers. Issue should be low.

[sherlock-admin3]

Escalate

It would be an admin/ user mistake to deposit rewards when there are no stakers. Issue should be low.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[panprog]

The rewards are sent from the lockers into ZivoeYDL, which then calls depositReward in the distributeYield function, which can be called by any user, thus the rewards might be lost without any admin interaction, only from (any) user interaction. And it's not user depositing rewards, it's user forcing accumulated reward to be deposited.

[WangSecurity]

Need clarification how it can distribute rewards if there are no stakers. As I know the users deposit into tranches and their funds are used for either loans or investing in other protocols, correct? For depositing into tranches, the users receive ZVE token and then can stake it.

But in the edge case, no one staked their ZVE token, but the funds from tranches were invested in some protocol and accrued yield. That yield will be distributed among users, but since no one staked their ZVE, the funds are stuck inside the contract and cannot be withdraw by anyone?

[panprog]

@WangSecurity The following are ZivoeRewards contracts (from ZivoeGlobals):

    address public stJTT;       /// @dev The ZivoeRewards ($stJTT) contract.
    address public stSTT;       /// @dev The ZivoeRewards ($stSTT) contract.
    address public stZVE;       /// @dev The ZivoeRewards ($stZVE) contract.

When user deposits into tranche, he's given tranche tokens (STT or JTT). But in order to get the rewards, user will have to stake his token into stSTT/stJTT, which are ZivoeRewards. When the ZivoeYDL distributes yield via distributeYield, it deposits this yield as reward into stSTT/stJTT based on emaSTT/emaJTT, which are averages of the adjusted balances (not staked tokens). So if for whatever reason all users unstake their stSTT or stJTT tokens, it is possible that there is some distribution accumulated, which is deposited into stSTT or stJTT, but there are no stakers.

This is not a concern with stZVE, because it splits the yield between stZVE / vestZVE based on staked tokens total supply, so if stZVE total supply == 0, it will receive 0 reward.

[WangSecurity]

In that case, the likelihood of that scenario is extremely low, but no guarantee that it doesn't happen, leading to a loss of funds if the issue occurs.

Planning to reject the escalations and leave the issue as it is.

[Evert0x]

Result: Medium Has Duplicates

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• spacegliderrrr: rejected