Some functions in TokenisableRange contract does not allow user to supply slippage and deadline, which may cause swap revert

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/TokenisableRange.sol#L154
https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/TokenisableRange.sol#L200
https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/TokenisableRange.sol#L260
https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/TokenisableRange.sol#L301

Vulnerability details

Impact

In the following functions except TokenisableRange.claimFee(), the minimum slippage is still hardcoded to 0, not allowing user to specify their own slippage parameters. This can expose users to sandwich attacks due to unlimited slippage.

Additionally, it also does not allow users to supply their own deadline as the deadline parameter is simply passed in as current block.timestamp in which transaction occurs. This effectively means that transaction has no deadline, which means that swap transaction may be included anytime by validators and remain pending in mempool, potentially exposing users to sandwich attacks by attackers or MEV bots.

• TokenisableRange.claimFee()
• TokenisableRange.init()
• TokenisableRange.deposit()
• TokenisableRange.withdraw()

Proof of Concept

Consider the following scenario:

1. Alice wants to provide liquidity 300 BNB token for 30 ETH and later sell the 1 ETH for 3000 DAI. She signs the transaction.

2. The transaction is submitted to the mempool, however, Alice chose a transaction fee that is too low for validators to be interested in including her transaction in a block. The transaction stays pending in the mempool for extended periods, which could be hours, days, weeks, or even longer.

3. When the average gas fee dropped far enough for Alice's transaction to become interesting again for miners to include it, her trade will be executed. In the meantime, the price of ETH could have drastically decreased and the DAI value of that output might be significantly lower. She has unknowingly performed a bad trade due to the pending transaction she forgot about.

An even worse way this issue can be maliciously exploited is through MEV:

The transaction is still pending in the mempool. Average fees are still too high for validators to be interested in it. The price of ETH has gone up significantly since the transaction was signed, meaning Alice would receive a lot more when the trade is executed.

Tools Used

Manual review

Recommended Mitigation Steps

Allow users to supply their own deadline parameter within the mentioned functions above

Assessed type

Timing

[c4-pre-sort]

141345 marked the issue as duplicate of #78

[c4-pre-sort]

141345 marked the issue as duplicate of #260

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Insufficient proof

[c4-judge]

gzeon-c4 changed the severity to QA (Quality Assurance)