sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

0xGoodess - the swap function is subject to MEV bundle since it does not have slippage proportional to trrade size, but just a fixed fee. #51

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0xGoodess

medium

the swap function is subject to MEV bundle since it does not have slippage proportional to trrade size, but just a fixed fee.

Summary

the swap function is subject to MEV bundle since it does not have slippage proportional to trade size, but just a fixed fee.

Vulnerability Detail

The protocol allows swapping from USD1 to USDEMC, namely emerging market currencies like indian Rupee, whose prices would be updated in XOracle. Price volatility in these EM currencies are inherently higher. During the swap, USDEMC would be minted to the trader who send in USD1, and the output amount is calculated by _calculateSwapResultByAmountIn.

However, in this function, the output just consider a fixed fee and the price from the oracle, without considering ReserveSize relative to tradeSize to incur slippage. This is problematic since if there is a price update that is big enough to cover fee x 2, MEV searcher could bundle the oracle update into a sandwich attack and profit from the entire available swappable size..

_calculateSwapResultByAmountIn(SwapRequest memory request)

    function _calculateSwapResultByAmountIn(SwapRequest memory request)
        internal
        view
        virtual
        returns (uint256 amountIn, uint256 amountOut, uint256 fee)
    {
        amountIn = request.amount;

        if (request.tokenIn == request.feeToken) {
            // When tokenIn is feeToken, subtracts the fee before converting the amount
            fee = _getFeeByAmountWithFee(amountIn, request.feeNumerator, request.feeBase);
            amountOut = _convert(
                request.tokenIn,
                request.tokenOut,
                amountIn - fee,
                MathUpgradeable.Rounding.Down,
                request.price,
                request.priceBase,
                request.quoteToken
            );
        } else {
            // When tokenOut is feeToken, subtracts the fee after converting the amount
            amountOut = _convert(
                request.tokenIn,
                request.tokenOut,
                amountIn,
                MathUpgradeable.Rounding.Down,
                request.price,
                request.priceBase,
                request.quoteToken
            );
            fee = _getFeeByAmountWithFee(amountOut, request.feeNumerator, request.feeBase);
            amountOut -= fee;
        }
    }

Impact

TradingPairs with volatile price and centralized oracle are traded without slippage relative to pool size is subject to MEV.

Code Snippet

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/main/Unitas-Protocol/src/SwapFunctions.sol#L41-L75

Tool used

Manual Review

Recommendation

Incur dynamic slippage that is based on trade size as a percentage of total reserve.

Duplicate of #88