Skip to content
This repository has been archived by the owner on Oct 27, 2024. It is now read-only.

SilverChariot - Time calculation issues with exponential decay #282

Open
sherlock-admin4 opened this issue Apr 25, 2024 · 3 comments
Open
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Apr 25, 2024

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
Copy link

forwardEmissions can be called prior to reset, not an issue

@github-actions github-actions bot added Medium A valid Medium severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels May 5, 2024
@sherlock-admin3
Copy link

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.

@sherlock-admin3 sherlock-admin3 added Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed labels May 9, 2024
@panprog
Copy link
Collaborator

panprog commented May 9, 2024

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

@sherlock-admin2 sherlock-admin2 changed the title Clumsy Cobalt Lion - Time calculation issues with exponential decay SilverChariot - Time calculation issues with exponential decay May 11, 2024
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label May 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

5 participants