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

2 stars 1 forks source link

smbv-1919 - No expiration deadline leads to losing of funds #146

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 9 months ago

smbv-1919

medium

No expiration deadline leads to losing of funds

Summary

The _swapInSingle() , _swapInPath(), _swapOutSingle(), _swapOutPath() in UniswapAdapter.sol does not set an expiration deadline, resulting in losing a lot of funds when swapping tokens.

Vulnerability Detail

The deadline parameter in the _swapInSingle() and in other 3 function is set to block.timestamp. That means the function will accept a token swap at any block number (i.e., no expiration deadline).

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

Impact

It may be more profitable for a miner to deny the transaction from being mined until the transaction incurs the maximum amount of slippage. A malicious miner can hold the transaction as deadline is set to block.timestamp which means that whenever the miner decides to include the transaction in a block, it will be valid at that time, since block.timestamp will be the current timestamp. The transaction might be left hanging in the mempool and be executed way later than the user wanted. The malicious miner can hold the transaction and execute the transaction only when he is profitable and no error would also be thrown as it will be valid at that time, since block.timestamp will be the current timestamp.

Code Snippet

https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/uniswap/UniswapAdapter.sol#L214 https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/uniswap/UniswapAdapter.sol#L232 https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/uniswap/UniswapAdapter.sol#L224 https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/uniswap/UniswapAdapter.sol#L170

Tool used

Manual Review

Recommendation

I recommend setting the deadline parameter with a proper timestamp.

sherlock-admin4 commented 8 months ago

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

panprog commented:

low, deadline can not be checked in uniswap, but might be beneficial to do in options mint and burn

tsvetanovv commented:

Invalid. Known issue. Check Readme.

takarez commented:

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