Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L360-L410

Vulnerability details

Impact

The owner of the MagicLp contract can reset the reserves and target values of the base and quote tokens via setParameters(), where the owner can totally reset these value to zeros if the quote and base balances are emptied (transferred to an address determined by the owner):

function setParameters(
      address assetTo,
      uint256 newLpFeeRate,
      uint256 newI,
      uint256 newK,
      uint256 baseOutAmount,
      uint256 quoteOutAmount,
      uint256 minBaseReserve,
      uint256 minQuoteReserve
  ) public nonReentrant onlyImplementationOwner {

      // some code...

      _transferBaseOut(assetTo, baseOutAmount);
      _transferQuoteOut(assetTo, quoteOutAmount);
      (uint256 newBaseBalance, uint256 newQuoteBalance) = _resetTargetAndReserve(); //<< @audit :this function resets the reserves and targets to be equal to the base and quote balances of the contract
      emit ParametersChanged(newLpFeeRate, newI, newK, newBaseBalance, newQuoteBalance);
  }

function _resetTargetAndReserve() internal returns (uint256 baseBalance, uint256 quoteBalance) {
      baseBalance = _BASE_TOKEN_.balanceOf(address(this));
      quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this));

      if (baseBalance > type(uint112).max || quoteBalance > type(uint112).max) {
          revert ErrOverflow();
      }

      _BASE_RESERVE_ = uint112(baseBalance);
      _QUOTE_RESERVE_ = uint112(quoteBalance);
      _BASE_TARGET_ = uint112(baseBalance);
      _QUOTE_TARGET_ = uint112(quoteBalance);
      _RState_ = uint32(PMMPricing.RState.ONE);

      _twapUpdate();
  }

So as can be noticed, the reserves and targets could be zeros (knowing that the minimum reserves checks are made before updating the reserves, while this should be done after _resetTargetAndReserve()), but how this could be exploited by a malicious user? let's see the following scenario:

The owner empties the reserves (_QUOTE_RESERVE_ & _QUOTE_RESERVE_ are zeros).
A malicious user directly transfers 1 wei of quote token and 1 wei of base token, and calls sync() function to set the reserves values to 1 wei:

  function _sync() internal {
      uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this));
      uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this));
      if (baseBalance != _BASE_RESERVE_) {
          _BASE_RESERVE_ = baseBalance.toUint112();
      }
      if (quoteBalance != _QUOTE_RESERVE_) {
          _QUOTE_RESERVE_ = quoteBalance.toUint112();
      }

      _twapUpdate();
  }

Then this malicious user transfers another 2 wei of base token and 2 wei of quote token to the contract and calls buyShares function:

function buyShares(address to) external nonReentrant returns (uint256 shares, uint256 baseInput, uint256 quoteInput) {
    uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this));
    uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this));
    uint256 baseReserve = _BASE_RESERVE_;
    uint256 quoteReserve = _QUOTE_RESERVE_;

    baseInput = baseBalance - baseReserve;
    quoteInput = quoteBalance - quoteReserve;

    if (baseInput == 0) {
        revert ErrNoBaseInput();
    }

    // Round down when withdrawing. Therefore, never be a situation occurring balance is 0 but totalsupply is not 0
    // But May Happen，reserve >0 But totalSupply = 0
    if (totalSupply() == 0) {
        // case 1. initial supply
        if (quoteBalance == 0) {
            revert ErrZeroQuoteAmount();
        }

        shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_) ? DecimalMath.divFloor(quoteBalance, _I_) : baseBalance;
        _BASE_TARGET_ = shares.toUint112();
        _QUOTE_TARGET_ = DecimalMath.mulFloor(shares, _I_).toUint112();

        if (_QUOTE_TARGET_ == 0) {
            revert ErrZeroQuoteTarget();
        }

        if (shares <= 2001) {
            revert ErrMintAmountNotEnough();
        }

        _mint(address(0), 1001);
        shares -= 1001;
    } else if (baseReserve > 0 && quoteReserve > 0) {
        // case 2. normal case
        uint256 baseInputRatio = DecimalMath.divFloor(baseInput, baseReserve);
        uint256 quoteInputRatio = DecimalMath.divFloor(quoteInput, quoteReserve);
        uint256 mintRatio = quoteInputRatio < baseInputRatio ? quoteInputRatio : baseInputRatio;
        shares = DecimalMath.mulFloor(totalSupply(), mintRatio);

        _BASE_TARGET_ = (uint256(_BASE_TARGET_) + DecimalMath.mulFloor(uint256(_BASE_TARGET_), mintRatio)).toUint112();
        _QUOTE_TARGET_ = (uint256(_QUOTE_TARGET_) + DecimalMath.mulFloor(uint256(_QUOTE_TARGET_), mintRatio)).toUint112();
    }

    _mint(to, shares);
    _setReserve(baseBalance, quoteBalance);

    emit BuyShares(to, shares, balanceOf(to));
}

So now: baseInput = baseBalance - baseReserve = 2wei - 1wei = 1wei, and quoteInput = quoteBalance - quoteReserve = 2wei - 1wei = 1wei, and the totalSupply is already > 0, so the second if-block will be executed to calculate the user shares:

function buyShares(address to) external nonReentrant returns (uint256 shares, uint256 baseInput, uint256 quoteInput) {
     //some code...
    } else if (baseReserve > 0 && quoteReserve > 0) {
        // case 2. normal case

        //! 1. @audit-issue : this would be: (1wei * 1e18) / 1wei = 1e18
        uint256 baseInputRatio = DecimalMath.divFloor(baseInput, baseReserve);

        //! 2.  @audit-issue : this would be: (1wei * 1e18) / 1wei = 1e18
        uint256 quoteInputRatio = DecimalMath.divFloor(quoteInput, quoteReserve);

        //! 3. @audit-issue : this would be: 1e18
        uint256 mintRatio = quoteInputRatio < baseInputRatio ? quoteInputRatio : baseInputRatio;

        //! 4. @audit-issue : this would be: (totalSupply() * 1e18)/ 1e18 = totalSupply()
        shares = DecimalMath.mulFloor(totalSupply(), mintRatio);

        //some code...
    }

    //! 5. @audit-issue : this will mint the current `totalSupply()` amount of shares to the malicious user
    _mint(to, shares);
    _setReserve(baseBalance, quoteBalance);

    emit BuyShares(to, shares, balanceOf(to));
}

As can be noticed from the follow-up comments above; the malicious user will be able to mint himself the current totalSupply of the shares with only 2 wei of base and quote tokens.

The explained attack will result in the following:
- The same attack can be repeatedly done as long as the reserves are low, where users will be able to buy shares with a very discounted amounts of base and quote (dust amounts).
- this would harm the old LP poviders where their share tokens value will drop.

Proof of Concept

MagicLP.buyShares function

function buyShares(address to) external nonReentrant returns (uint256 shares, uint256 baseInput, uint256 quoteInput) {
        uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this));
        uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this));
        uint256 baseReserve = _BASE_RESERVE_;
        uint256 quoteReserve = _QUOTE_RESERVE_;

        baseInput = baseBalance - baseReserve;
        quoteInput = quoteBalance - quoteReserve;

        if (baseInput == 0) {
            revert ErrNoBaseInput();
        }

        // Round down when withdrawing. Therefore, never be a situation occurring balance is 0 but totalsupply is not 0
        // But May Happen，reserve >0 But totalSupply = 0
        if (totalSupply() == 0) {
            // case 1. initial supply
            if (quoteBalance == 0) {
                revert ErrZeroQuoteAmount();
            }

            shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_) ? DecimalMath.divFloor(quoteBalance, _I_) : baseBalance;
            _BASE_TARGET_ = shares.toUint112();
            _QUOTE_TARGET_ = DecimalMath.mulFloor(shares, _I_).toUint112();

            if (_QUOTE_TARGET_ == 0) {
                revert ErrZeroQuoteTarget();
            }

            if (shares <= 2001) {
                revert ErrMintAmountNotEnough();
            }

            _mint(address(0), 1001);
            shares -= 1001;
        } else if (baseReserve > 0 && quoteReserve > 0) {
            // case 2. normal case
            uint256 baseInputRatio = DecimalMath.divFloor(baseInput, baseReserve);
            uint256 quoteInputRatio = DecimalMath.divFloor(quoteInput, quoteReserve);
            uint256 mintRatio = quoteInputRatio < baseInputRatio ? quoteInputRatio : baseInputRatio;
            shares = DecimalMath.mulFloor(totalSupply(), mintRatio);

            _BASE_TARGET_ = (uint256(_BASE_TARGET_) + DecimalMath.mulFloor(uint256(_BASE_TARGET_), mintRatio)).toUint112();
            _QUOTE_TARGET_ = (uint256(_QUOTE_TARGET_) + DecimalMath.mulFloor(uint256(_QUOTE_TARGET_), mintRatio)).toUint112();
        }

        _mint(to, shares);
        _setReserve(baseBalance, quoteBalance);

        emit BuyShares(to, shares, balanceOf(to));
    }

Tools Used

Manual Review.

Recommended Mitigation Steps

Prevent token reserves from reaching zeros when the owner updates the contract parameters (via setParameters) function, this can be ensured by updating setParameters() function to check for minimum tokens reserves after the update:

function setParameters(
        address assetTo,
        uint256 newLpFeeRate,
        uint256 newI,
        uint256 newK,
        uint256 baseOutAmount,
        uint256 quoteOutAmount,
        uint256 minBaseReserve,
        uint256 minQuoteReserve
    ) public nonReentrant onlyImplementationOwner {

-       if (_BASE_RESERVE_ < minBaseReserve || _QUOTE_RESERVE_ < minQuoteReserve) {
-           revert ErrReserveAmountNotEnough();
-       }

        // some code...

        _transferBaseOut(assetTo, baseOutAmount);
        _transferQuoteOut(assetTo, quoteOutAmount);
        (uint256 newBaseBalance, uint256 newQuoteBalance) = _resetTargetAndReserve();

+       if (_BASE_RESERVE_ < minBaseReserve || _QUOTE_RESERVE_ < minQuoteReserve) {
+           revert ErrReserveAmountNotEnough();
+       }

        emit ParametersChanged(newLpFeeRate, newI, newK, newBaseBalance, newQuoteBalance);
    }

Assessed type

Context

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

`MagicLp.buyShares()`: a malicious actor can mint himself the `totalSupply` of the shares token with dust amounts if the reserves are empty #178