code423n4 commented 12 months ago

Lines of code

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

Vulnerability details

Impact

the emissionConfigOwner has the ability to manage emission speed and end time of emissions.

When changing end time there is a check that the new end time is further in the future than the current:

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

File: MultiRewardDistributor/MultiRewardDistributor.sol

789:        require(
790:            _newEndTime > currentEndTime,
791:            "_newEndTime MUST be > currentEndTime"
792:        );

_endTime is uint256 so the emissionConfigOwner could set the time to type(uint256).max. Even if admin (comptroller admin) changes the owner of the emissionConfig there is no way to lower the end time as any update is required to pass this check.

This could be done to grief by an owner that is being replaced as it would require micromanagement of the speed to manage emissions. Otherwise emissions will never end and can reach un-payable payout amounts.

Proof of Concept

Test in MultiRewardDistributor.t.sol, MultiRewardDistributorCommonUnitTest:

    function testEndTimeMax() public {
        distributor._addEmissionConfig(
            mToken,
            address(this),
            address(emissionToken),
            0.5e18,
            0.5e18,
            block.timestamp + 365 days
        );

        // end time set to max value
        distributor._updateEndTime(
            mToken,
            address(emissionToken),
            type(uint256).max
        );

        // cannot be changed back
        vm.expectRevert("_newEndTime MUST be > currentEndTime");
        distributor._updateEndTime(
            mToken,
            address(emissionToken),
            block.timestamp + 365 days
        );
    }