sherlock-audit / 2024-03-zivoe-judging

8 stars 6 forks source link

SilverChariot - Time calculation issues with exponential decay #282

Open sherlock-admin4 opened 6 months ago

sherlock-admin4 commented 6 months ago

SilverChariot

medium

Time calculation issues with exponential decay

Summary

OCE_ZVE is a locker that distributes rewards based on a exponential decay model. When a distribution starts, the whole balance of the contract should be unclaimable. As time moves forward, more and more of it should be unlocked for distributing. The calculations are made using the lastDistribution variable which is initially set in the constructor and then changed on each distribution. This is problematic because the "idle" periods where no distribution is happening will be considered as passed time when a real distribution starts.

Vulnerability Detail

In the constructor, the lastDistribution variable is set to block.timestamp.

        lastDistribution = block.timestamp;

When forwardEmissions() gets called, the calculation block.timestamp - lastDistribution will return a large value because the timer has started at the time of deployment.

    function forwardEmissions() external nonReentrant {
        uint zveBalance = IERC20(IZivoeGlobals_OCE_ZVE(GBL).ZVE()).balanceOf(address(this));
        _forwardEmissions(zveBalance - decay(zveBalance, block.timestamp - lastDistribution));
        lastDistribution = block.timestamp;
    }

As we can see in the Figma file, the OCE locker will be deployed at Phase One and funding will start after ITO ends, which is at least 30 days.

This results in a wrong calculation and instead of decaying, a big amount of the rewards can be forwarded as soon as the distribution starts.

The issue persists for further distributions also. If distribution 1 ends on 1 January and the Zivoe team decides to start distribution 2 on 1 July, the rewards for 6 months will be claimable from the very beginning. Clearing the timestamp before a distribution starts is not an option because it requires at least 100e18 assets to be forwarded.

        require(amount >= 100 ether, "OCE_ZVE::_forwardEmissions amount < 100 ether");

Impact

Instead of decaying, a big part of the rewards is claimable from the beginning.

Code Snippet

PoC for Test_OCE_ZVE

    function test_OCE_timer() public {
        hevm.warp(block.timestamp + 365 days);
        deal(address(ZVE), address(OCE_ZVE_Live), 20000e18);
        OCE_ZVE_Live.forwardEmissions();  
    }

Tool used

Foundry

Recommendation

Add a guarded function that start the distribution by updating the timestamp.

function startDistribution() external {
         require(
            _msgSender() == IZivoeGlobals_OCE_ZVE(GBL).TLC(), 
            "OCE_ZVE::startDistribution() _msgSender() != IZivoeGlobals_OCE_ZVE(GBL).TLC()"
        );
        lastDistribution = block.timestamp;
}
pseudonaut commented 6 months ago

forwardEmissions can be called prior to reset, not an issue

sherlock-admin3 commented 6 months ago

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

panprog commented:

medium, OCE_ZVE forwardEmission will start emission as if some large time has already passed due to absence of "start emission" function (to reset lastDistribution timestamp). While it's possible to reset it by calling forwardEmission, it requires a minimum of 100e18 tokens, which will have to be distributed to reset the timestamp, which is still a loss of funds. As this is mostly a 1-time action, the impact is limited.

panprog commented 5 months ago

Keeping this medium, because a distribution of 100e18 tokens is still required to reset the lastDistribution timestamp.