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

10 stars 9 forks source link

No expiration deadline leads to losing a lot of funds #116

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

The UniswapSettlement::swapExactIn() and UniswapSettlement::swapExactOut() does not set an expiration deadline, resulting in losing a lot of funds when swapping tokens.

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

    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

Recommended Mitigation Steps

I recommend setting the deadline parameter with a proper timestamp.

Assessed type

Context

c4-judge commented 4 months ago

alex-ppg marked the issue as unsatisfactory: Invalid