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

9 stars 7 forks source link

Ability to adjust _I_ creates sandwich opportunity #244

Closed c4-bot-5 closed 6 months ago

c4-bot-5 commented 6 months ago

Lines of code

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

Vulnerability details

Impact

Adjusting the value of "I" directly influences the price. This can be exploited by a MEV bot, simply by trading just before the "adjustPrice" function and exiting right after the price change. The profit gained from this operation essentially represents potential losses for the liquidity providers who supplied liquidity to the pool.

Proof of Concept

In a PMM, I is directly linked to pricing, changing it changes the price. Bots in mem pool can monitor it and front run and sell the base token and get the quote token and immediately after price change sell the quote tokens to get more of base token.

This problem arise in the following code snippet.

    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);
    }

Exact similar issue was reported in the dodo v3 sherlock competition.

https://solodit.xyz/issues/m-1-adjusting-_i_-will-create-a-sandwich-opportunity-because-of-price-changes-sherlock-dodo-gsp-git

Tools Used

Solodit

Recommended Mitigation Steps

This function is originally not present in the original implementation of DODO V2 and PMM don't need it to, let the free market decide the pricing of asset instead of controlling it via admin. So removing this function and still all the functionality will work fine.

Assessed type

Other

c4-pre-sort commented 6 months ago

141345 marked the issue as duplicate of #171

c4-judge commented 5 months ago

thereksfour marked the issue as satisfactory