No Deadline check in `UlyssesRouter`, allowing outdated slippage and allow pending transaction to be unexpected executed

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-amm/UlyssesRouter.sol#L73

Vulnerability details

Proof of Concept

AMMs provide their users with an option to limit the execution of their pending actions, such as swaps or adding and removing liquidity. The most common solution is to include a deadline timestamp as a parameter (for example: Uniswap V2 and Uniswap V3). If such an option is not present, users can unknowingly perform bad trades.

Currently there are 3 external methods in UlyssesRouter:

1. addLiquidity
2. removeLiquidity
3. swap

2 things are most important while interacting with AMMs:

1. Slippage parameter: Which is controlled here by user by passing the value of minOutput as a function parameter.
2. Deadline of Execution: In case, the transaction is submitted to the mempool and transaction fee is too low for miners to be interested in including the transaction in a block immediately, then the transaction can stay in the mempool for some time. If it is executed at a later time, there is high chance that the slippage parameter earlier passed might result in higher slippage than what user originally anticipated. This can result in loss of funds.

A MEV bot can detect the pending transactions. Since the outdated maximum slippage value now allows for high slippage, the bot sandwiches the transaction, resulting in significant profit for the bot and significant loss for user.

This second part is not handled in UlyssesRouter.

Impact

Potential loss of funds

Tools Used

VS Code

Recommended Mitigation Steps

Add a deadline check exactly how uniswap does in functions related to adding liquidity, removing liquidity and swapping.

Assessed type

MEV

[c4-judge]

trust1995 marked the issue as duplicate of #171

[c4-judge]

trust1995 marked the issue as satisfactory