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

1 stars 0 forks source link

`safe32` WILL RESTRICT THE DURATION OF `MultiRewardDistributor._addEmissionConfig` FUNCTION USABILITY #364

Closed code423n4 closed 11 months ago

code423n4 commented 11 months ago

Lines of code

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

Vulnerability details

Impact

The MultiRewardDistributor._addEmissionConfig function is used to add a new emission configuration for a specific market. The _addEmissionConfig function constructs the MarketConfig configuration struct using the input parameters to the function. The supplyGlobalTimestamp and borrowGlobalTimestamp are set as follows:

        supplyGlobalTimestamp: safe32(
            block.timestamp,
            "block timestamp exceeds 32 bits"
        ),

        borrowGlobalTimestamp: safe32(
            block.timestamp,
            "block timestamp exceeds 32 bits"
        ),

The both above parameter assignment use the safe32 to make sure that block.timestamp does not exceed 32 bits. If the block.timestamp >= 2**32 the _addEmissionConfig transaction will revert thus preventing onlyComptrollersAdmin from adding a new emission configuration for a specific market. Hence the maximum value block.timestamp can hold will be upto 2106-02-07 06:28:15 UTC. After this point in time the _addEmissionConfig functinality will be unusable.

Proof of Concept

            supplyGlobalTimestamp: safe32(
                block.timestamp,
                "block timestamp exceeds 32 bits"
            ),

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

            borrowGlobalTimestamp: safe32(
                block.timestamp,
                "block timestamp exceeds 32 bits"
            ),

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

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to allow more bits for the block.timestamp to hold its value in. Hence safe32 can be replaced with safe48 in the MultiRewardDistributor._addEmissionConfig function, thus allowing 2**48 for the block.timestamp which is enough duration for the protocol to operate without any broken functionality due to time contraints.

Assessed type

Other

0xSorryNotSorry commented 11 months ago

Could be QA.

Inflated submission as there's no risk to the funds.

c4-pre-sort commented 11 months ago

0xSorryNotSorry marked the issue as low quality report

alcueca commented 11 months ago

To be quite frank, by 2106 there will be either a Moonwell v2+, or no Moonwell at all. Just because an scenario is possible, it doesn't mean it is probable.

c4-judge commented 11 months ago

alcueca marked the issue as unsatisfactory: Overinflated severity