code-423n4 / 2023-07-moonwell-findings

1 stars 0 forks source link

Updating the end time for an emission campaign does not correspond with the available rewards and supply speed #279

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L775

Vulnerability details

Impact

The function _updateEndTime (lines 775-810 in MultiRewardDistributor.sol) does not validate if the rewards available in the contract would be enough after the extension of the campaign end time and the current emissions rate.

If the emissions speed per second set in the current configuration is X tokens to the end time of the current campaign, the needed available amount of X tokens in the form of rewards would be (End Time - block.timestamp) * Supply Emissions per Second. For example, if presumably the end of the campaign is extended by 50%, the needed available rewards would be 50% more than the current disposable one, which would mislead the user to think that he/she would receive rewards to the end of the campaign.

Proof of Concept

    function _updateEndTime(
        MToken _mToken,
        address _emissionToken,
        uint256 _newEndTime
    ) external onlyEmissionConfigOwnerOrAdmin(_mToken, _emissionToken) {
        MarketEmissionConfig
            storage emissionConfig = fetchConfigByEmissionToken(
                _mToken,
                _emissionToken
            );

        uint256 currentEndTime = emissionConfig.config.endTime;

        // Must be older than our existing end time AND the current block
        require(
            _newEndTime > currentEndTime,
            "_newEndTime MUST be > currentEndTime"
        );
        require(
            _newEndTime > block.timestamp,
            "_newEndTime MUST be > block.timestamp"
        );

        // Update both global indices before setting the new end time. If rewards are off this just updates the
        // global block timestamp to the current second
        updateMarketBorrowIndexInternal(_mToken);
        updateMarketSupplyIndexInternal(_mToken);

        emissionConfig.config.endTime = _newEndTime;
        emit NewRewardEndTime(
            _mToken,
            _emissionToken,
            currentEndTime,
            _newEndTime
        );
    }

Tools Used

Manual VS code

Recommended Mitigation Steps

Create a validation for the available funds based on the emissions per second and the end time of the campaign.

Assessed type

Other

0xSorryNotSorry commented 1 year ago

Technically valid, but the function is provided in another context as per the NATSPEC below;


     This is a set of APIs for external parties/emission config owners to update their configs. They're
     able to transfer ownership, update emission speeds, and update the end time for a campaign. Worth
     noting, if the endTime is hit, no more rewards will be accrued, BUT you can call `_updateEndTime`
     to extend the specified campaign - if the campaign has ended already, then rewards will start
     accruing from the time of reactivation.

So it's the emission config owner's responsibility to check for the available assets which makes the submission falling into [M‑04] The owner is a single point of failure and a centralization risk

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

alcueca commented 1 year ago

Also, I assume that the config owner can add assets to be distributed at any time.

c4-judge commented 1 year ago

alcueca marked the issue as unsatisfactory: Invalid