code-423n4 / 2024-05-munchables-findings

3 stars 1 forks source link

`getLockedWeightedValue()` does not support tokens with more than 18 decimals #245

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L461

Vulnerability details

Impact

In the contest page we can see that the protocol team says that we should assume that they can add more ERC20 tokens in the future for use in LockManager.sol and that the ERC20 tokens can be with more than 18 decimals.

Proof of Concept

The problem is that getLockedWeightedValue() does not support tokens with more than 18 decimals because of the logic implemented in it. As we can see here the expected result is a token with 18 decimals


 function getLockedWeightedValue(address _player) external view returns (uint256 _lockedWeightedValue) {
        uint256 lockedWeighted = 0;
        uint256 configuredTokensLength = configuredTokenContracts.length;
        for (uint256 i; i < configuredTokensLength; i++) {
            if (
                lockedTokens[_player][configuredTokenContracts[i]].quantity > 0
                    && configuredTokens[configuredTokenContracts[i]].active
            ) {               
>>              uint256 deltaDecimal = 10 ** (18 - configuredTokens[configuredTokenContracts[i]].decimals);
>>              lockedWeighted += (
                    deltaDecimal * lockedTokens[_player][configuredTokenContracts[i]].quantity
                        * configuredTokens[configuredTokenContracts[i]].usdPrice
                ) / 1e18;
            }
        }

        _lockedWeightedValue = lockedWeighted;
    }

If they add a configured token with more than 18 decimals the deltaDecimal will overflow which will result in a revert and DoS of the getLockedWeightedValue() function because once a token is in the configuredTokensLength it can no longer be removed.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider changing the deltaDecimal so it can handle tokens with more than 18 decimals and at the end of the lockedWeighted variable you will have to change the division to not be a constant value of 1e18 as it is now.

Assessed type

Other

c4-judge commented 3 months ago

alex-ppg marked the issue as unsatisfactory: Out of scope