code-423n4 / 2024-03-abracadabra-money-findings

9 stars 7 forks source link

DoS in creation of certain pools using `Router` due to incorrect Validation #196

Closed c4-bot-4 closed 8 months ago

c4-bot-4 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/periphery/Router.sol#L598-L605

Vulnerability details

Impact

User will not be able to create any pool where baseDecimals is greater than quoteDecimals.

Proof of Concept

To Create new Pool, User can use Router contract's following functions:

  1. createPool
  2. createPoolEth

These functions first validate the decimals, ensuring that the difference between them does not exceed MAX_BASE_QUOTE_DECIMALS_DIFFERENCE, which is set to 12.


    function createPool(
        // SNIP
    ) external returns (address clone, uint256 shares) {
@->     _validateDecimals(IERC20Metadata(baseToken).decimals(), IERC20Metadata(quoteToken).decimals());

        clone = IFactory(factory).create(baseToken, quoteToken, lpFeeRate, i, k);

        // SNIP
    }

    function createPoolETH(
        // SNIP
    ) external payable returns (address clone, uint256 shares) {
        if (useTokenAsQuote) {
@->         _validateDecimals(18, IERC20Metadata(token).decimals());
        } else {
@->         _validateDecimals(IERC20Metadata(token).decimals(), 18);
        }

        clone = IFactory(factory).create(useTokenAsQuote ? address(weth) : token, useTokenAsQuote ? token : address(weth), lpFeeRate, i, k);

        // SNIP
    }

The _validateDecimals function performs the validation:


    function _validateDecimals(uint8 baseDecimals, uint8 quoteDecimals) internal pure {
        if (baseDecimals == 0 || quoteDecimals == 0) {
            revert ErrZeroDecimals();
        }
        if (quoteDecimals - baseDecimals > MAX_BASE_QUOTE_DECIMALS_DIFFERENCE) {
            revert ErrDecimalsDifferenceTooLarge();
        }
    }

However, the validation only accounts for scenarios where quoteDecimals > baseDecimals by 12 or more.

If baseDecimals > quoteDecimals, an underflow occurs, resulting in an immediate revert even if the difference between them is less than 12 decimals.

This leads to a DoS situation for creating pools where baseDecimals > quoteDecimals.

Tools Used

VS Code

Recommended Mitigation Steps

Refactor the validation as shown below which correctly handles scenarios where either baseDecimals or quoteDecimals is larger and both are within Max Decimal Difference allowed.


      function _validateDecimals(uint8 baseDecimals, uint8 quoteDecimals) internal pure {
        if (baseDecimals == 0 || quoteDecimals == 0) {
            revert ErrZeroDecimals();
        }
+       if(quoteDecimals >= baseDecimals){
            if (quoteDecimals - baseDecimals > MAX_BASE_QUOTE_DECIMALS_DIFFERENCE) {
                revert ErrDecimalsDifferenceTooLarge();
            }
+       } else {
+           if (baseDecimals - quoteDecimals > MAX_BASE_QUOTE_DECIMALS_DIFFERENCE) {
+               revert ErrDecimalsDifferenceTooLarge();
+           }
+       }
    }

Assessed type

Invalid Validation

c4-pre-sort commented 8 months ago

141345 marked the issue as duplicate of #25

c4-judge commented 8 months ago

thereksfour marked the issue as satisfactory

c4-judge commented 7 months ago

thereksfour changed the severity to QA (Quality Assurance)