Missing deadline check in UlyssesRouter

[code423n4]

Lines of code

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

Vulnerability details

Impact

addLiquidity(), removeLiquidity(), swap() don't have deadline parameter. It means that transaction can be pending in mempool so long, such that minOutput is too low comparing to current price at execution, and therefore high slippage action is performed.

Proof of Concept

Suppose current scenario:

1. User wants to deposit 1000 tokens. token : share are traded as 1 : 1.
2. User accepts slippage of 1%, therefore specifies minOutput = 1000 * 1 * (100% - 1%) = 990
3. Transaction is pending long time in mempool. token/price share changed a lot, now 1 token is equal to 2 shares.
4. Fair price with slippage 1% should be minOutput = 1000 * 2 * (100% - 1%) = 1980, but user specified 990 and can perform bad trade because of slippage

Tools Used

Manual Review

Recommended Mitigation Steps

Add deadline argument and check it, like UniswapV2Router does:

    function addLiquidityETH(
        address token,
        uint amountTokenDesired,
        uint amountTokenMin,
        uint amountETHMin,
        address to,
        uint deadline
    ) external virtual override payable ensure(deadline) returns (uint amountToken, uint amountETH, uint liquidity) {
        ...
    }

    modifier ensure(uint deadline) {
        require(deadline >= block.timestamp, 'UniswapV2Router: EXPIRED');
        _;
    }

Assessed type

MEV

[c4-judge]

trust1995 marked the issue as duplicate of #171

[c4-judge]

trust1995 marked the issue as satisfactory