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

IV3SwapRouter of the swap-router-contracts lib lacks deadline checks which can cause timing execution issues #601

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

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/011f62059f3a0b1f3577c8ccd1140f0cf3e7bb29/src/swappers/UniSwapper.sol#L129-L135
https://github.com/code-423n4/2024-01-decent/blob/011f62059f3a0b1f3577c8ccd1140f0cf3e7bb29/src/swappers/UniSwapper.sol#L149-L156
https://github.com/code-423n4/2024-01-decent/blob/011f62059f3a0b1f3577c8ccd1140f0cf3e7bb29/src/swappers/SwapParams.sol#L9-L20

Vulnerability details

Impact

The protocol uses the IV3SwapRouter of the swap-router-contracts lib rather than using the swap functions defined in the SwapRouter contarct of the v3-periphery lib of UniswapV3 protocol. The key difference between both, is that UniswapV3 uses the modifier checkDeadline before calling any swap functions CheckDeadline. Plus, the UniswapV3 has a deadline parameter defined for the struct ExactInputSingleParams unlike the swap-router-contracts.

However, the absence of the checkDeadline modifier and the deadline parameter, can lead to issues related to the timing of swap executions. In Automated Market Maker (AMM) systems, front-running and transaction timing are critical. Without deadline checks, transactions could be manipulated by validators or miners to occur at times that are unfavorable to users or the protocol, potentially causing maximum slippage or other forms of transaction manipulation.

Proof of Concept

In the UniSwapper contract, the library used lacks the checkDeadline modifier present in UniswapV3's SwapRouter contract. This modifier is crucial for enforcing a deadline on swap transactions:

modifier checkDeadline(uint256 deadline) {
    require(_blockTimestamp() <= deadline, 'Transaction too old');
    _;
}

Without this check, swap transactions lack a crucial safeguard against timing manipulation.

Tools Used

Manual review

Recommended Mitigation Steps

Use the functions defined in the SwapRouter contract of the v3-periphery lib of UniswapV3 protocol instead. With this library used, the SwapParams struct needs to be updated to account for the deadline parameter which should also be chosen by the user or detemined off-chain.

Assessed type

Uniswap

@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-6 added a commit that referenced this issue Jan 23, 2024
@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 insufficient quality report

@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