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

Execution of swaps in the before bridging operation and in the same-chain txs lacks deadline #608

Closed
c4-bot-1 opened this issue Jan 23, 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 edited-by-warden insufficient quality report This report is not of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-1
Copy link
Contributor

c4-bot-1 commented Jan 23, 2024

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L58-L77
https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L123-L169

Vulnerability details

Proof of Concept

In UniSwapper.sol, part of the Decent protocol, swaps are executed as part of the token exchange process. This contract interacts with Uniswap or similar decentralized exchanges (DEXs) to perform token swaps. However, the execution of these swaps, particularly before bridging operations and in same-chain transactions, does not include a deadline parameter. In typical DEX interactions, a deadline is used to specify the time by which a swap must be completed, providing a safeguard against unfavorable changes in market conditions or potential manipulation.

In the UniSwapper.sol contract, functions that interact with DEXs for swapping tokens (e.g., swapExactIn, swapExactOut) do not include a deadline parameter. This can be confirmed by reviewing the contract code and noting the absence of a time-bound condition for swap execution.

    function swap(
        bytes memory swapPayload
    )
        external
        onlyUtb
        returns (address tokenOut, uint256 amountOut)
    {
        (SwapParams memory swapParams, address receiver, address refund) = abi
            .decode(swapPayload, (SwapParams, address, address));
        tokenOut = swapParams.tokenOut;
        if (swapParams.path.length == 0) {
            return swapNoPath(swapParams, receiver, refund);
        }
        if (swapParams.direction == SwapDirection.EXACT_IN) {
            amountOut = swapExactIn(swapParams, receiver);
        } else {
            swapExactOut(swapParams, receiver, refund);
            amountOut = swapParams.amountOut;
        }
    }

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

			  //@audit missing deadline parameter
        IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter
            .ExactInputParams({
                path: swapParams.path,
                recipient: address(this),
                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);
				//@audit missing deadline parameter
        IV3SwapRouter.ExactOutputParams memory params = IV3SwapRouter
            .ExactOutputParams({
                path: swapParams.path,
                recipient: address(this),
                //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);
    }

Impact

The absence of a deadline in swap transactions can lead to several issues:

  1. Front-Running Vulnerability: Without a deadline, transactions are more susceptible to front-running, where malicious actors can observe a pending swap transaction and execute their own transactions first to affect the market price.
  2. Market Risk: Users are exposed to market risk for a longer duration, as the swap can be executed at any time, potentially leading to execution at less favorable prices.
  3. User Experience: Users have no control over the maximum duration for which their transaction can remain pending, which can lead to uncertainty and potential financial losses.

The issue presents a clear risk in terms of potential financial losses and exploitation through market manipulation. However, it does not directly lead to immediate loss of funds or critical contract failures. The severity could be higher in scenarios where the market is highly volatile, or the protocol is heavily utilized, increasing the likelihood of exploitation.

Tools Used

Manual code review

Recommendations

To mitigate the risks associated with the lack of a deadline in swap operations, consider the following recommendations:

  1. Implement Deadline Parameter: Modify the swap functions in UniSwapper.sol to include a deadline parameter. This parameter should be passed along to the DEX interface to ensure that swaps are only executed within the specified time frame.
  2. User-Defined Deadlines: Allow users to specify the deadline when initiating a swap. This gives users control over the timing of their transactions and reduces their exposure to market risks.

Assessed type

Other

@c4-bot-1 c4-bot-1 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 23, 2024
c4-bot-9 added a commit that referenced this issue Jan 23, 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 25, 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 edited-by-warden 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

4 participants