sherlock-audit / 2024-02-smilee-finance-judging

2 stars 1 forks source link

hals - `UniswapAdapter` uses a hardcoded `_DEFAULT_FEE` for UNIV3 pools #123

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 9 months ago

hals

medium

UniswapAdapter uses a hardcoded _DEFAULT_FEE for UNIV3 pools

Summary

UniswapAdapter uses a hardcoded _DEFAULT_FEE for UNIV3 pools

Vulnerability Detail

The protocol integrates with Uniswap V3 as an underlying DEX to swap side tokens for base tokens of vaults via UniswapAdapter contract, where it uses a 0.05% fee pool regardless of the swapped asset, and by knowing that there are multiple pool tiers for the same side/base tokens pair; then it's possible that there are other pools (pools with fees different than 0.05%) where majority of the liquidity lies instead.

Impact

Also, it could be possible that pools with 0.05% fee for specific side/base token pairs are not created (knowing that the protocol is supposed to be deployed initially on Arbitrum, then later on other L2s), so any attempts to swap via the specified path will fail, breaking base tokens swapping for liquidity and payoffs.

Code Snippet

UniswapAdapter._DEFAULT_FEE

    uint24 constant _DEFAULT_FEE = 500; // 0.05% (hundredths of basis points)

UniswapAdapter.getPath function

path = abi.encodePacked(
  reversed ? tokenOut : tokenIn,
  _DEFAULT_FEE,
  reversed ? tokenIn : tokenOut
);

UniswapAdapter._swapOutSingle function

    function _swapOutSingle(
        address tokenIn,
        address tokenOut,
        uint256 amountOut,
        uint256 amountMaximumIn
    ) private returns (uint256 amountIn) {
        ISwapRouter.ExactOutputSingleParams memory params = ISwapRouter.ExactOutputSingleParams(
            tokenIn,
            tokenOut,
            _DEFAULT_FEE,
            msg.sender,
            block.timestamp,
            amountOut,
            amountMaximumIn,
            _SQRTPRICELIMITX96
        );

        amountIn = _swapRouter.exactOutputSingle(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

Add a customizable pool fee for each side/base token pairs.

sherlock-admin2 commented 8 months ago

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

panprog commented:

invalid, UniswapAdapter allows to set arbitrary path (setPath), each segment in the path also specifies fee. If needed, admin can set such path from 1 segment with the needed fee. The usage of default fee seems to be intended functionality.

tsvetanovv commented:

Known issue. Check readme: "DEX SWAP: issues related to impacts of fees and slippage are known and mitigated"

takarez commented:

invalid