`MagicLp.flashLoan` function can be exploited to empty the contract base and quote tokens balances

[c4-bot-4]

Lines of code

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

Vulnerability details

Impact

• MagicLp.flashLoan function enabes users to utilize the contract quote and base balances, but due to a missing balance check; the contracts balances can be drained, let's see the following scenario:

  function flashLoan(uint256 baseAmount, uint256 quoteAmount, address assetTo, bytes calldata data) external nonReentrant {
  
    // 1. @audit-issue : first the baseAmount & quoteAmount are transferred to the attacker contract, and these amounts are equal to the balance - 1wei , where 1 wei will be left in the contract for each token
        _transferBaseOut(assetTo, baseAmount);
        _transferQuoteOut(assetTo, quoteAmount);
  
    // 2. @audit-issue : the attacker contract calls `MagicLP.sync()` to update the reserves of the contract to the current balance; which is the 1 wei left
        if (data.length > 0) {
            ICallee(assetTo).FlashLoanCall(msg.sender, baseAmount, quoteAmount, data);
        }
  
    // 3. @audit-issue : now the baseBalance = 1 wei, and quoteBalance = 1 wei
        uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this));
        uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this));
  
    // 4. @audit-issue : the baseBalance ==_BASE_RESERVE_ == 1 wei, and quoteBalance == _QUOTE_RESERVE_ == 1 wei, so the txn will not revert
  
        // no input -> pure loss
        if (baseBalance < _BASE_RESERVE_ && quoteBalance < _QUOTE_RESERVE_) {
            revert ErrFlashLoanFailed();
        }
  
   // 5. @audit-issue : this block will not be entered since baseBalance ==_BASE_RESERVE_ == 1 wei
  
        // sell quote case
        // quote input + base output
        if (baseBalance < _BASE_RESERVE_) {
            uint256 quoteInput = quoteBalance - uint256(_QUOTE_RESERVE_);
            (uint256 receiveBaseAmount, uint256 mtFee, PMMPricing.RState newRState, uint256 newQuoteTarget) = querySellQuote(
                tx.origin,
                quoteInput
            );
  
            if (uint256(_BASE_RESERVE_) - baseBalance > receiveBaseAmount) {
                revert ErrFlashLoanFailed();
            }
  
            _transferBaseOut(_MT_FEE_RATE_MODEL_.maintainer(), mtFee);
            if (_RState_ != uint32(newRState)) {
                _QUOTE_TARGET_ = newQuoteTarget.toUint112();
                _RState_ = uint32(newRState);
                emit RChange(newRState);
            }
            emit Swap(address(_QUOTE_TOKEN_), address(_BASE_TOKEN_), quoteInput, receiveBaseAmount, msg.sender, assetTo);
        }
  
   // 6. @audit-issue : this block will not be entered since quoteBalance == _QUOTE_RESERVE_ == 1 wei
  
        // sell base case
        // base input + quote output
        if (quoteBalance < _QUOTE_RESERVE_) {
            uint256 baseInput = baseBalance - uint256(_BASE_RESERVE_);
            (uint256 receiveQuoteAmount, uint256 mtFee, PMMPricing.RState newRState, uint256 newBaseTarget) = querySellBase(
                tx.origin,
                baseInput
            );
  
            if (uint256(_QUOTE_RESERVE_) - quoteBalance > receiveQuoteAmount) {
                revert ErrFlashLoanFailed();
            }
  
            _transferQuoteOut(_MT_FEE_RATE_MODEL_.maintainer(), mtFee);
            if (_RState_ != uint32(newRState)) {
                _BASE_TARGET_ = newBaseTarget.toUint112();
                _RState_ = uint32(newRState);
                emit RChange(newRState);
            }
            emit Swap(address(_BASE_TOKEN_), address(_QUOTE_TOKEN_), baseInput, receiveQuoteAmount, msg.sender, assetTo);
        }
  
        _sync();
  
        emit FlashLoan(msg.sender, assetTo, baseAmount, quoteAmount);
    }

1. A malicious user calls flashLoan function with:

   • baseAmount = _BASE_TOKEN_.balanceOf(address(magicLp)) - 1
   • quoteAmount = _QUOTE_TOKEN_.balanceOf(address(magicLp)) - 1

2. Then a call is made to the attacker contract (assetTo address), where it will call MagicLp.sync() function to set the reserves to the current balance of tokens, knowing that the current balances are 1 wei & 1 wei:

   function sync() external nonReentrant {
           _sync();
       }
   
       //----
   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();
       }

3. Now, when the flashLoan() function checks for the balances and reserves after the external call, it will pass since the reserves == balances == 1 wei, and the function will proceed.

4. The next blocks that check for extra quote or base tokens reserves to sell them will be passed since balances == reserves.

Proof of Concept

MagicLP.flashLoan function

function flashLoan(uint256 baseAmount, uint256 quoteAmount, address assetTo, bytes calldata data) external nonReentrant {
        _transferBaseOut(assetTo, baseAmount);
        _transferQuoteOut(assetTo, quoteAmount);

        if (data.length > 0) {
            ICallee(assetTo).FlashLoanCall(msg.sender, baseAmount, quoteAmount, data);
        }

        uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this));
        uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this));

        // no input -> pure loss
        if (baseBalance < _BASE_RESERVE_ && quoteBalance < _QUOTE_RESERVE_) {
            revert ErrFlashLoanFailed();
        }

        // sell quote case
        // quote input + base output
        if (baseBalance < _BASE_RESERVE_) {
            uint256 quoteInput = quoteBalance - uint256(_QUOTE_RESERVE_);
            (uint256 receiveBaseAmount, uint256 mtFee, PMMPricing.RState newRState, uint256 newQuoteTarget) = querySellQuote(
                tx.origin,
                quoteInput
            );

            if (uint256(_BASE_RESERVE_) - baseBalance > receiveBaseAmount) {
                revert ErrFlashLoanFailed();
            }

            _transferBaseOut(_MT_FEE_RATE_MODEL_.maintainer(), mtFee);
            if (_RState_ != uint32(newRState)) {
                _QUOTE_TARGET_ = newQuoteTarget.toUint112();
                _RState_ = uint32(newRState);
                emit RChange(newRState);
            }
            emit Swap(address(_QUOTE_TOKEN_), address(_BASE_TOKEN_), quoteInput, receiveBaseAmount, msg.sender, assetTo);
        }

        // sell base case
        // base input + quote output
        if (quoteBalance < _QUOTE_RESERVE_) {
            uint256 baseInput = baseBalance - uint256(_BASE_RESERVE_);
            (uint256 receiveQuoteAmount, uint256 mtFee, PMMPricing.RState newRState, uint256 newBaseTarget) = querySellBase(
                tx.origin,
                baseInput
            );

            if (uint256(_QUOTE_RESERVE_) - quoteBalance > receiveQuoteAmount) {
                revert ErrFlashLoanFailed();
            }

            _transferQuoteOut(_MT_FEE_RATE_MODEL_.maintainer(), mtFee);
            if (_RState_ != uint32(newRState)) {
                _BASE_TARGET_ = newBaseTarget.toUint112();
                _RState_ = uint32(newRState);
                emit RChange(newRState);
            }
            emit Swap(address(_BASE_TOKEN_), address(_QUOTE_TOKEN_), baseInput, receiveQuoteAmount, msg.sender, assetTo);
        }

        _sync();

        emit FlashLoan(msg.sender, assetTo, baseAmount, quoteAmount);
    }

Tools Used

Manual Review.

Recommended Mitigation Steps

Update MagicLP.flashLoan() function to first sync the reserves with the balances, and then cache the tokens reserves before the call, and check they are <= reserves after the call, so that the attacker can't drain the contract funds:

function flashLoan(uint256 baseAmount, uint256 quoteAmount, address assetTo, bytes calldata data) external nonReentrant {

+       _sync();
+       uint256 baseReservesBefore = _BASE_RESERVE_;// will be equal to the base balance
+       uint256 quoteReservesBefore = _QUOTE_RESERVE_;// will be equal to the quote balance
        _transferBaseOut(assetTo, baseAmount);
        _transferQuoteOut(assetTo, quoteAmount);

        if (data.length > 0) {
            ICallee(assetTo).FlashLoanCall(msg.sender, baseAmount, quoteAmount, data);
        }

+       // to ensure that the resrves haven't been decreased due to a manipulation:
+       if (baseReservesBefore > _BASE_RESERVE_ || quoteReservesBefore > _QUOTE_RESERVE_) {
+           revert ErrFlashLoanFailed();
+       }

        uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this));
        uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this));

        // no input -> pure loss
        if (baseBalance < _BASE_RESERVE_ && quoteBalance < _QUOTE_RESERVE_) {
            revert ErrFlashLoanFailed();
        }

        // sell quote case
        // quote input + base output
        if (baseBalance < _BASE_RESERVE_) {
            uint256 quoteInput = quoteBalance - uint256(_QUOTE_RESERVE_);
            (uint256 receiveBaseAmount, uint256 mtFee, PMMPricing.RState newRState, uint256 newQuoteTarget) = querySellQuote(
                tx.origin,
                quoteInput
            );

            if (uint256(_BASE_RESERVE_) - baseBalance > receiveBaseAmount) {
                revert ErrFlashLoanFailed();
            }

            _transferBaseOut(_MT_FEE_RATE_MODEL_.maintainer(), mtFee);
            if (_RState_ != uint32(newRState)) {
                _QUOTE_TARGET_ = newQuoteTarget.toUint112();
                _RState_ = uint32(newRState);
                emit RChange(newRState);
            }
            emit Swap(address(_QUOTE_TOKEN_), address(_BASE_TOKEN_), quoteInput, receiveBaseAmount, msg.sender, assetTo);
        }

        // sell base case
        // base input + quote output
        if (quoteBalance < _QUOTE_RESERVE_) {
            uint256 baseInput = baseBalance - uint256(_BASE_RESERVE_);
            (uint256 receiveQuoteAmount, uint256 mtFee, PMMPricing.RState newRState, uint256 newBaseTarget) = querySellBase(
                tx.origin,
                baseInput
            );

            if (uint256(_QUOTE_RESERVE_) - quoteBalance > receiveQuoteAmount) {
                revert ErrFlashLoanFailed();
            }

            _transferQuoteOut(_MT_FEE_RATE_MODEL_.maintainer(), mtFee);
            if (_RState_ != uint32(newRState)) {
                _BASE_TARGET_ = newBaseTarget.toUint112();
                _RState_ = uint32(newRState);
                emit RChange(newRState);
            }
            emit Swap(address(_BASE_TOKEN_), address(_QUOTE_TOKEN_), baseInput, receiveQuoteAmount, msg.sender, assetTo);
        }

        _sync();

        emit FlashLoan(msg.sender, assetTo, baseAmount, quoteAmount);
    }

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as primary issue

[141345]

invalid

_sync() func all have nonReentrant modifier

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-pre-sort]

141345 marked the issue as insufficient quality report

[c4-judge]

thereksfour marked the issue as unsatisfactory: Invalid