`MagicLp` contract: pool can be drained if any of the reserves are empty

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/libraries/PMMPricing.sol#L92-L95 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/libraries/PMMPricing.sol#L59-L63

Vulnerability details

Impact

• MagicLp contract has sellBase function that enables users to sell their base tokens for a quote token in return, and it has sellQuote function that enables users to sell their quote tokens and get base tokens in return (from the contract reserves), where users trade their tokens with the contract tokens reserves.

• But these functions (sellBase & sellQuote) allow sellers to purchase the contract reserves without a limit, where any user can empty the base or quote reserves as there's no limit on the amount of tokens being purchased from the contract.

• So for example:

  • if the quote token reserve is emptied, then _QUOTE_TARGET_ & _QUOTE_RESERVE_ will be zeros (state.Q0 & state.Q in getPMMState()),
  • then any malicious user can sell 0 quote token amount to withdraw the whole base token balance minus _BASE_TARGET_, by calling MagicLP.querySellQuote(trader,0).
  • this is caused as the edge case when the target is zero is not checked by the PMMPricing.sellQuoteToken:

function sellQuoteToken(PMMState memory state, uint256 payQuoteAmount) internal pure returns (uint256 receiveBaseAmount, RState newR) {
        if (state.R == RState.ONE) {
           //some code...
        } else if (state.R == RState.ABOVE_ONE) {
            //some code...
        } else {
            uint256 backToOnePayQuote = state.Q0 - state.Q; // << @audit-issue : when the quote reserve is emptied; then the reserve and target will be zero, thus backToOnePayQuote == 0
            uint256 backToOneReceiveBase = state.B - state.B0;
            if (payQuoteAmount < backToOnePayQuote) {

            //some code...

            } else if (payQuoteAmount == backToOnePayQuote) {// << @audit-issue : doesn't check if the payQuoteAmount || backToOnePayQuote == 0
                receiveBaseAmount = backToOneReceiveBase;
                newR = RState.ONE;
            } else {
               //some code...
            }
        }
    }

• This can be achieved by a flash-loan attack as well (via flashLoan() function).

Proof of Concept

PMMPricing.sellQuoteToken function/ L92-L95

 } else if (payQuoteAmount == backToOnePayQuote) {
                receiveBaseAmount = backToOneReceiveBase;
                newR = RState.ONE;
            } else {

PMMPricing.sellBaseToken function/L59-L63

  } else if (payBaseAmount == backToOnePayBase) {
                // case 2.2: R status changes to ONE
                receiveQuoteAmount = backToOneReceiveQuote;
                newR = RState.ONE;
            } else {

Tools Used

Manual Review.

Recommended Mitigation Steps

Update PMMPricing.sellQuoteToken() & PMMPricing.sellBaseToken() functions to revert if payBaseAmount == 0 || backToOnePayBase == 0:

  } else if (payBaseAmount == backToOnePayBase) {
                // case 2.2: R status changes to ONE
+               require(backToOnePayQuote !=0 && payBaseAmount != 0, "invalid purchase");
                receiveQuoteAmount = backToOneReceiveQuote;
                newR = RState.ONE;
            } else {

Assessed type

Context

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[141345]

empty reserve to drain the pool

seems invalid need to check POC, how to make target 0

[0xmDreamy]

We acknowledge that this is indeed the case, but dispute that it'd be possible to get reserves to 0 once liquidity has been added (this also burns some shares). The finding hints that it is possible to do through flashLoan(), but this call importantly only syncs reserves once everything has been validated, and it is nonReentrant.

[c4-sponsor]

0xmDreamy (sponsor) disputed

[thereksfour]

Invalid, flashloan can't do it, reserves are updated after the user repays them, not after the tokens are lent.

[c4-judge]

thereksfour marked the issue as unsatisfactory: Invalid