Lines of code

https://github.com/code-423n4/2024-05-predy/blob/2fb1e0ec7a52fc06c2e9c8e561bccba84302e4bb/src/settlements/UniswapSettlement.sol#L34 https://github.com/code-423n4/2024-05-predy/blob/2fb1e0ec7a52fc06c2e9c8e561bccba84302e4bb/src/settlements/UniswapSettlement.sol#L50

Vulnerability details

Impact

The protocol is using block.timestamp as the deadline argument while interacting with the Uniswap swap router, which completely defeats the purpose of using a deadline.

Actions in the Uniswap SwapRouter contract are protected by a deadline parameter to limit the execution of pending transactions. Functions that modify the liquidity of the pool check this parameter against the current block timestamp in order to discard expired actions.

However, UniswapSettlement::swapExactIn and UniswapSettlement::swapExactout functions provide block.timestamp as the argument for the deadline parameter in their call to the corresponding underlying Uniswap SwapRouter contract. Using block.timestamp as the deadline is effectively a no-operation that has no effect nor protection. Since block.timestamp will take the timestamp value when the transaction gets mined, the check will end up comparing block.timestamp against the same value (see https://github.com/Uniswap/v3-periphery/blob/697c2474757ea89fec12a4e6db16a574fe259610/contracts/base/PeripheryValidation.sol#L7).

Failure to provide a proper deadline value enables pending transactions to be maliciously executed at a later point. Transactions that provide an insufficient amount of gas such that they are not mined within a reasonable amount of time, can be picked by malicious actors or MEV bots and executed later in detriment of the submitter.

Proof of Concept

UniswapSettlement.sol 34 & 50

File: src/settlements/UniswapSettlement.sol

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

        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

Add a deadline parameter to the UniswapSettlement::swapExactIn and UniswapSettlement::swapExactOut functions and forward this parameter to the corresponding underlying call to the Uniswap SwapRouter contract.

Assessed type

Uniswap

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

Dangerous use of deadline parameter #170