Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delayed Transaction Execution Due to Lack of Deadline in Swap Functions in UniSwapper Contract #253

Closed
c4-bot-9 opened this issue Jan 22, 2024 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-24 insufficient quality report This report is not of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-9
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/swappers/UniSwapper.sol#L130-L135
https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/swappers/UniSwapper.sol#L150-L156

Vulnerability details

Delayed Transaction Execution Due to Lack of Deadline in Swap Functions in UniSwapper Contract

The swapExactIn and swapExactOut functions within the UniSwapper contract are used to perform token swaps, potentially interacting with Uniswap V3. These functions currently do not include a deadline parameter, which is a standard feature in decentralized exchange protocols to ensure that swaps are executed within a certain timeframe. The absence of a deadline can lead to delayed transaction execution, resulting in swaps being executed under market conditions that may have changed unfavorably since the transaction was initiated or the transaction will wait a long time to reach a favorable condition.

Impact

The lack of a deadline parameter can have several adverse effects:

  1. Delayed Execution: Without a deadline, transactions can remain pending for an extended period, especially during times of network congestion. This delay increases the risk of price movements that can result in unfavorable execution rates.

  2. Market Volatility Exposure: Cryptocurrency markets are highly volatile. A swap executed much later than intended can be subject to significant price slippage, potentially leading to a trade at a rate that is worse than the user's expectations.

  3. Ineffective Trading Strategies: Automated trading strategies that rely on timely execution may become ineffective if transactions are not executed within the expected timeframe.

Proof of Concept

The issue is present in the swapExactIn and swapExactOut functions:

Both ExactInputParams at line 130 and ExactOutputParams at line 150 are lacking the inclusion of the deadline parameter.

File: src/swappers/UniSwapper.sol

123:    function swapExactIn(
124:        SwapParams memory swapParams, // SwapParams is a struct
125:        address receiver
126:    ) public payable routerIsSet returns (uint256 amountOut) {
            // ... other code ...
129:        IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter
130:            .ExactInputParams({
131:                path: swapParams.path,
132:                recipient: address(this),
133:                amountIn: swapParams.amountIn,
134:                amountOutMinimum: swapParams.amountOut
135:            });
            // ... other code ...    

143:    function swapExactOut(
144:        SwapParams memory swapParams,
145:        address receiver,
146:        address refundAddress
147:    ) public payable routerIsSet returns (uint256 amountIn) {
            // ... other code ...
149:        IV3SwapRouter.ExactOutputParams memory params = IV3SwapRouter
150:            .ExactOutputParams({
151:                path: swapParams.path,
152:                recipient: address(this),
153: @>             //deadline: block.timestamp,
154:                amountOut: swapParams.amountOut,
155:                amountInMaximum: swapParams.amountIn
156:            });
            // ... other code ...

https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/swappers/UniSwapper.sol#L130C1-L135C16
https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/swappers/UniSwapper.sol#L150C13-L156C16

PoC

  1. Alice initiates a swap of ETH for USDC using the contract's swapExactIn function.
  2. Due to network congestion, Alice's transaction remains pending and is not immediately mined.
  3. The market price of ETH experiences significant volatility during the pending period.
  4. After a substantial delay, Alice's transaction is finally mined.
  5. The swap is executed at the current market rate, which has moved unfavorably compared to when Alice initiated the swap.

Alice's swap is executed with maximum slippage due to the absence of a deadline, resulting in her receiving less USDC than she expected at the time of initiating the swap.

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate this issue, it is recommended to add a deadline parameter to the ExactInputParams and ExactOutputParams when calling Uniswap V3's swap functions. The deadline should be set to block.timestamp that represents the latest time by which the transaction should be executed.

Here is an example modification:

    function swapExactIn(
        SwapParams memory swapParams, // SwapParams is a struct
        address receiver
    ) public payable routerIsSet returns (uint256 amountOut) {
        swapParams = _receiveAndWrapIfNeeded(swapParams);

        IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter
            .ExactInputParams({
                path: swapParams.path,
                recipient: address(this),
+               deadline: block.timestamp,
                amountIn: swapParams.amountIn,
                amountOutMinimum: swapParams.amountOut
            });

        IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn);
        amountOut = IV3SwapRouter(uniswap_router).exactInput(params);

        _sendToRecipient(receiver, swapParams.tokenOut, amountOut);
    }

    function swapExactOut(
        SwapParams memory swapParams,
        address receiver,
        address refundAddress
    ) public payable routerIsSet returns (uint256 amountIn) {
        swapParams = _receiveAndWrapIfNeeded(swapParams);
        IV3SwapRouter.ExactOutputParams memory params = IV3SwapRouter
            .ExactOutputParams({
                path: swapParams.path,
                recipient: address(this),
-               //deadline: block.timestamp,
+               deadline: block.timestamp,
                amountOut: swapParams.amountOut,
                amountInMaximum: swapParams.amountIn
            });

        IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn);
        amountIn = IV3SwapRouter(uniswap_router).exactOutput(params);

        // refund sender
        _refundUser(
            refundAddress,
            swapParams.tokenIn,
            params.amountInMaximum - amountIn
        );

        _sendToRecipient(receiver, swapParams.tokenOut, swapParams.amountOut);
    }

By implementing a deadline, the UniSwapper contract will ensure that swaps are executed within a reasonable and expected timeframe, providing users with protection against unfavorable market movements and enhancing the overall reliability of the trading operation.

Assessed type

Uniswap

@c4-bot-9 c4-bot-9 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jan 22, 2024
c4-bot-1 added a commit that referenced this issue Jan 22, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as insufficient quality report

@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Jan 24, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #24

@c4-judge
Copy link

c4-judge commented Feb 1, 2024

alex-ppg marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-24 insufficient quality report This report is not of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants