`MagicLp` contract: updating base/quote tokens price (`_I_`) can be sandwiched by arbitrageurs

[c4-bot-5]

Lines of code

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

Vulnerability details

Impact

• MagicLp contract uses _I_ variable as a replacemnt of oracles to get the price of base/quote tokens, and the variable _K_ is used to limit the price fluctuation of the pair tokens, and these values are used to price the tokens when users selling their base or quote tokens (pricing is done based on the state of the contract's reserves and balances extracted from getPMMState()) , where the owner of the contract can set these values via MagicLp.setParameters().

• Knowing that _I_ is in the range of 0 < _I_ < 10**36; updating _I_ will directly influence the price, and this can be exploited by any arbitrager by frontrunning the transaction and trading (selling base or quote tokens) just before this variable is adjusted via setParameters() function and trading again (selling again base or quote tokens but with a higher return) right after the price change (backrunning) , which would result in a loss for the share holders (LP providers) where their shares would hold a lesser value.

Proof of Concept

MagicLP.setParameters function

    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();
        }
        if (newI == 0 || newI > MAX_I) {
            revert ErrInvalidI();
        }
        if (newK > MAX_K) {
            revert ErrInvalidK();
        }
        if (newLpFeeRate < MIN_LP_FEE_RATE || newLpFeeRate > MAX_LP_FEE_RATE) {
            revert ErrInvalidLPFeeRate();
        }

        _LP_FEE_RATE_ = uint64(newLpFeeRate);
        _K_ = uint64(newK);
        _I_ = uint128(newI);

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

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

Tools Used

Manual Review.

Recommended Mitigation Steps

Update _I_ variable in multiple transactions over time so that it wouldn't create an opportunity for arbitrageurs to exploit the price change.

Assessed type

MEV

[c4-pre-sort]

141345 marked the issue as duplicate of #171

[c4-judge]

thereksfour marked the issue as satisfactory