code-423n4 / 2023-03-asymmetry-findings

14 stars 12 forks source link

Missing deadline checks allow pending transactions to be maliciously executed #25

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L83-L102

Vulnerability details

Impact

AMMs should 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 see Uniswap V2). If such an option is not present, users can unknowingly perform bad trades

Proof of Concept

Alice wants to swap 1 WETH for 1 RETH and later sell the 1 RETH for 1000 DAI. She signs the transaction calling Pair.sell with inputAmount = 1 WETH and minOutputAmount = 0.99 RETH to allow for some slippage. The transaction is submitted to the mempool, however, Alice chose a transaction fee that is too low for miners 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. When the average gas fee dropped far enough for Alice’s transaction to become interesting again for miners to include it, her swap will be executed. In the meantime, the price of RETH could have drastically changed. She will still at least get 0.99 RETH due to minOutputAmount, but 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 swap transaction is still pending in the mempool. Average fees are still too high for miners to be interested in it. The price of RETH has gone up significantly since the transaction was signed, meaning Alice would receive a lot more ETH when the swap is executed. But that also means that her minOutputAmount value is outdated and would allow for significant slippage. A MEV bot detects the pending transaction. Since the outdated minOutputAmount now allows for high slippage, the bot sandwiches Alice, resulting in significant profit for the bot and significant loss for Alice.

Tools Used

Manual review

Recommended Mitigation Steps

Introduce a deadline parameter to the mentioned functions.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #1087

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory