hals - `UniswapAdapter._swapInSingle()` & `UniswapAdapter._swapInPath()` don't implement a slippage check

[sherlock-admin3]

hals

medium

UniswapAdapter._swapInSingle() & UniswapAdapter._swapInPath() don't implement a slippage check

Summary

UniswapAdapter._swapInSingle() & UniswapAdapter._swapInPath() don't implement a slippage check.

Vulnerability Detail

The protocol integrates with Uniswap V3 pools as an underlying DEX to swap side tokens for base tokens of vaults (via Vault._sellSideTokens()) in:

• Vault.emergencyRebalance(): to sell all side tokens if the vault is stuck in InsufficientLiquidity error.

• Vault._adjustBalances(): to ddjust the balances in order to cover the liquidity locked for pending operations and obtain an equal weight portfolio.

• Vault._beforeRollEpoch()

• Vault._deltaHedge() and the swapping is done by calling UniswapAdapter.swapIn() function, where it will swap the given amount of tokenIn tokens in exchange for some tokenOut tokens, and will return the amount of output token given by the exchange.

    function swapIn(address tokenIn, address tokenOut, uint256 amountIn) public returns (uint256 tokenOutAmount) {
            _zeroAddressCheck(tokenIn);
            _zeroAddressCheck(tokenOut);
  
            TransferHelper.safeTransferFrom(tokenIn, msg.sender, address(this), amountIn);
            TransferHelper.safeApprove(tokenIn, address(_swapRouter), amountIn);
  
            TimeLockedSwapPath storage path = _swapPaths[_encodePair(tokenIn, tokenOut)];
            tokenOutAmount = path.exists.get()
                ? _swapInPath(path.data.get(), amountIn)
                : _swapInSingle(tokenIn, tokenOut, amountIn);
        }

  where:

    function _swapInPath(bytes memory path, uint256 amountIn) private returns (uint256 tokenOutAmount) {
        ISwapRouter.ExactInputParams memory params = ISwapRouter.ExactInputParams(
            path,
            msg.sender,
            block.timestamp,
            amountIn,
            0 //! amountOutMinimum  @audit-issue : doesn't protect against slippage
        );
  
        tokenOutAmount = _swapRouter.exactInput(params);
    }

  and

    function _swapInSingle(
        address tokenIn,
        address tokenOut,
        uint256 amountIn
    ) private returns (uint256 tokenOutAmount) {
        ISwapRouter.ExactInputSingleParams memory params = ISwapRouter.ExactInputSingleParams(
            tokenIn,
            tokenOut,
            _DEFAULT_FEE,
            msg.sender,
            block.timestamp,
            amountIn,
            0, //! amountOutMinimum @audit-issue : doesn't protect against slippage
            _SQRTPRICELIMITX96
        );
  
        tokenOutAmount = _swapRouter.exactInputSingle(params);
    }

• As can be noticed; the amountOutMinimum parameter is set to zero for the swap, indicating that any returned amount will be accepted.

Impact

Since there's no check in the returned tokenOutAmount wherever swapIn function is used in Vault contract (the aforementioned functions); this would result in the protocol (or any user using this contract to swap) to lose their tokens by accepting any returned amount from swapping.

Code Snippet

UniswapAdapter.swapIn function

    function swapIn(address tokenIn, address tokenOut, uint256 amountIn) public returns (uint256 tokenOutAmount) {
        _zeroAddressCheck(tokenIn);
        _zeroAddressCheck(tokenOut);

        TransferHelper.safeTransferFrom(tokenIn, msg.sender, address(this), amountIn);
        TransferHelper.safeApprove(tokenIn, address(_swapRouter), amountIn);

        TimeLockedSwapPath storage path = _swapPaths[_encodePair(tokenIn, tokenOut)];
        tokenOutAmount = path.exists.get()
            ? _swapInPath(path.data.get(), amountIn)
            : _swapInSingle(tokenIn, tokenOut, amountIn);
    }

UniswapAdapter._swapInPath function

function _swapInPath(bytes memory path, uint256 amountIn) private returns (uint256 tokenOutAmount) {
        ISwapRouter.ExactInputParams memory params = ISwapRouter.ExactInputParams(
            path,
            msg.sender,
            block.timestamp,
            amountIn,
            0
        );

        tokenOutAmount = _swapRouter.exactInput(params);
    }

UniswapAdapter._swapInSingle function

function _swapInSingle(
        address tokenIn,
        address tokenOut,
        uint256 amountIn
    ) private returns (uint256 tokenOutAmount) {
        ISwapRouter.ExactInputSingleParams memory params = ISwapRouter.ExactInputSingleParams(
            tokenIn,
            tokenOut,
            _DEFAULT_FEE,
            msg.sender,
            block.timestamp,
            amountIn,
            0,
            _SQRTPRICELIMITX96
        );

        tokenOutAmount = _swapRouter.exactInputSingle(params);
    }

Tool used

Manual Review

Recommendation

Check the tokenOutAmount value returned from calling swapIn function in the aforementioned Vault contract functions against an acceptable value (slippage, being a percentage of the swapped amount).

[sherlock-admin3]

3 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, slippage check is done downstream in exchange and upstream in DVP when the actual premium is compared to user's provided max premium value.

tsvetanovv commented:

Slippage related issue is known issues. Check Readme.

takarez commented:

invalid; DEX SWAP: issues related to impacts of fees and slippage are known and mitigated