Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/settlements/UniswapSettlement.sol#L22-L56

Vulnerability details

Description

In the UniswapSettlement.sol contract, the methods swapExactIn and swapExactOut use block.timestamp as the deadline argument while interacting with the Uniswap V3 Protocol via the exactInput() method. This practice fails to provide adequate protection against transaction delays and potential price swings.

The deadline parameter in exact input swaps is designed to mitigate risks associated with transaction delays and resultant price volatility. However, using block.timestamp as the deadline offers no such protection, as it merely reflects the current timestamp, which means the transaction will be valid whenever the miner includes it in a block.

Impact

An attacker could intentionally delay the transaction in the mempool, allowing it to be processed much later. During this period, the Uniswap pool's price may change significantly and become unfavorable.

Proof of Concept

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/settlements/UniswapSettlement.sol#L22-L56

    function swapExactIn(
        address,
        address baseToken,
        bytes memory data,
        uint256 amountIn,
        uint256 amountOutMinimum,
        address recipient
    ) external override returns (uint256 amountOut) {
        ERC20(baseToken).safeTransferFrom(msg.sender, address(this), amountIn);
        ERC20(baseToken).approve(address(_swapRouter), amountIn);

        // @audit block.timestamp as deadline means that whenever the miner decides to include the txn in a block, it will be valid at that time, since block.timestamp will be the current timestamp.
        amountOut = _swapRouter.exactInput(
            ISwapRouter.ExactInputParams(data, recipient, block.timestamp, amountIn, amountOutMinimum)
        );
    }

    function swapExactOut(
        address quoteToken,
        address,
        bytes memory data,
        uint256 amountOut,
        uint256 amountInMaximum,
        address recipient
    ) external override returns (uint256 amountIn) {
        ERC20(quoteToken).safeTransferFrom(msg.sender, address(this), amountInMaximum);
        ERC20(quoteToken).approve(address(_swapRouter), amountInMaximum);

        amountIn = _swapRouter.exactOutput(
            ISwapRouter.ExactOutputParams(data, recipient, block.timestamp, amountOut, amountInMaximum)
        );

        if (amountInMaximum > amountIn) {
            ERC20(quoteToken).safeTransfer(msg.sender, amountInMaximum - amountIn);
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended to pass valid deadline value into swapExactIn and swapExactOut, e.g., add a new field in settlementParams as the deadline value just like the slippage parameter.

Assessed type

Timing

code-423n4 / 2024-05-predy-findings

Dangerous use of deadline parameter in swapExactIn and swapExactOut #242