code-423n4 / 2023-05-maia-findings

20 stars 12 forks source link

MALICIOUS USER CAN CALL THE `FlywheelBribeRewards.setRewardsDepot()` FUNCTION INDEFINITELY TO PUSH `ethereum` INTO `STATE BLOAT` #906

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/rewards/rewards/FlywheelBribeRewards.sol#L37-L42

Vulnerability details

Impact

The FlywheelBribeRewards.setRewardsDepot() function is an external permissionless function. Any malicious user can create as many ERC20 compatible contracts as possible and can call this function to set themselves as strategies in the FlywheelBribeRewards.rewardsDepots mapping. Hence this could be used to add as many instances as possible to the rewardsDepots state variable. Hence this can be used by a malicious user to increase the storage used by the smart contract which could lead to state bloat in the ethereum state.

Proof of Concept

    function setRewardsDepot(RewardsDepot rewardsDepot) external {
        /// @dev Anyone can call this, whitelisting is handled in FlywheelCore
        rewardsDepots[ERC20(msg.sender)] = rewardsDepot;

        emit AddRewardsDepot(msg.sender, rewardsDepot);
    }

https://github.com/code-423n4/2023-05-maia/blob/main/src/rewards/rewards/FlywheelBribeRewards.sol#L37-L42

Tools Used

VSCode and Manual Review

Recommended Mitigation Steps

It is recommended to keep an upperbound to the number of rewardsDepot that can be added to the rewardsDepots mapping via the FlywheelBribeRewards.setRewardsDepot function. This can be done by maintaining an incrementer inside the FlywheelBribeRewards.setRewardsDepot function, which will increment each time a new rewardsDepot is added to the mapping. A check should performed inside the setRewardsDepot to make sure this incrementer value is less than the upperbound set. upperbound can be set as a constant and the incrementer can be set as a state variable. The setRewardsDepot transaction should revert if the incrementer exceeds the upperbound. This will prevent state mapping expanding unnecessarily and forcing the ethereum into state bloat.

Assessed type

Other

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid