code-423n4 / 2024-04-panoptic-findings

9 stars 4 forks source link

InteractionHelper.computeDecimals returns incorrect default value if ERC20 doesn't suport metadata #554

Closed c4-bot-10 closed 5 months ago

c4-bot-10 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/libraries/InteractionHelper.sol#L112-L114

Vulnerability details

InteractionHelper.computeDecimals (which is used in CollateralTracker contract) tries to get decimals amount of token to set it as shares token decimals, but if this collateral token doesn't support ERC20 this function would return 0. But if token doesn't support metadata decimals should be 18, as it's a commonly accepted default value for maintaining compatibility with other tokens.

Impact

If some contract use decimals from CollateralTracker (if corresponding token doesn't support decimals) erroneous calculations may occur during operations with shares tokens as they have incorrect decimals amount.

Recommended Mitigation Steps

Change default value from 0 to 18.

    function computeDecimals(address token) external view returns (uint8) {
        // not guaranteed that token supports metadada extension
        // so we need to let call fail and return placeholder if not
        try IERC20Metadata(token).decimals() returns (uint8 _decimals) {
            return _decimals;
        } catch {
-           return 0;
+           return 18;
        }
    }

Assessed type

Decimal

Picodes commented 5 months ago

Unless proven otherwise this is only for front ends so doesn't change much in the context of panoptic ?

c4-judge commented 5 months ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 5 months ago

Picodes marked the issue as grade-c