Insufficient slippage protection for swaps

Summary

Unitas contract provides insufficient slippage protection for swaps, that may cause a loss of funds because of fluctuation in exchange.

Vulnerability Detail

Unitas#swap(...) executes swaps for token pairs, users can pass AmountType to specify the amount they expected to get or the amount they are willing to spend:

    function swap(address tokenIn, address tokenOut, AmountType amountType, uint256 amount)
        external
        whenNotPaused
        nonReentrant
        returns (uint256 amountIn, uint256 amountOut)

During swapping, protocol gets the lastest price from oracle and conducts price check to ensure the price is in tolerance range:

    function _checkPrice(address quoteToken, uint256 price) internal view {
        (uint256 minPrice, uint256 maxPrice) = tokenManager.getPriceTolerance(quoteToken);

        _require(minPrice > 0 && maxPrice > 0, Errors.PRICE_TOLERANCE_INVALID);
        _require(minPrice <= price && price <= maxPrice, Errors.PRICE_INVALID);
    }

The price tolerance range is stored in _maxPriceTolerance and _minPriceTolerance mappings and could be updated based on currency exchange rate:

    function _setMinMaxPriceTolerance(address token, uint256 minPrice, uint256 maxPrice) internal {
        _require(maxPrice != 0, Errors.MAX_PRICE_INVALID);
        _require(minPrice != 0 && minPrice <= maxPrice, Errors.MIN_PRICE_INVALID);
        _maxPriceTolerance[token] = maxPrice;
        _minPriceTolerance[token] = minPrice;
    }

This provides slippage protection to some extent for swaps, however, it's insufficient and user may still suffer loss because of fluctuation in exchange. Imagine the following scenario:

Alice submits a transaction to swap USDEMC for USD1 and she is willing to spend 10000 USDEMC tokens;
At the time of submitting, the exchange rate for USD1/USDEMC is 10, protocol price tolerance range is [9, 11], so Alice is likely to get no less than 900 USD1 tokens, which is fair;
Alice is in no hurry so she pays low gas fee, her transaction gets stuck in pending;
Soon after, USDEMC depreciates and the exchange rate for USD1/USDEMC rises to 12, price tolerance range is updated accordingly to [11, 13] by protocol team;
Alice's transaction gets executed, as the swap is no longer protected by old tolerance range, Alice gets only 833 USD1 tokens in return and suffers a loss.

Impact

User may suffer loss of funds due to insufficient slippage protection.

Code Snippet

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/main/Unitas-Protocol/src/Unitas.sol#L208-L237

Tool used

Manual Review

Recommendation

Provide the necessary amountInMaximum and for amountOutMinimum for swaps.

-    function swap(address tokenIn, address tokenOut, AmountType amountType, uint256 amount)
+    function swap(address tokenIn, address tokenOut, AmountType amountType, uint256 amount, uint256 amountInMaximum, uint256 amountOutMinimum)

Duplicate of #88

sherlock-audit / 2023-04-unitasprotocol-judging

Juntao - Insufficient slippage protection for swaps #96