sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

Jeiwan - Using `block.timestamp` as deadline exposes users to sandwich attacks #380

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Jeiwan

medium

Using block.timestamp as deadline exposes users to sandwich attacks

Summary

The deadline parameter is set to block.timestamp when calling Perpetual Protocol's ClearingHouse.openPosition and Uniswap's SwapRouter.exactInputSingle, which basically disables the transaction execution deadline. As a result, users may lose funds due to sandwich attacks when there's positive slippage.

Vulnerability Detail

When opening a position in Perpetual Protocol or swapping tokens in Uniswap, the deadline parameter is set to block.timestamp (PerpDepository.sol#L362, Uniswapper.sol#L39). This means that the deadline parameter will be set to the block at which the transaction will be mined, which makes the deadline check useless. This exposes users to sandwich attacks in situations when:

  1. their transactions is delayed due to increased gas prices or a too low gas price set when the transactions was created;
  2. the exchange rate in Perpetual Protocol or the Uniswap V3 pool changes for the benefit of the user (e.g. the price of the asset they're buying/longing lowers).

The above situation results in a positive slippage, which may be stolen by MEV bots. Consider this exploit scenario:

  1. A user deposits USDC to the controller. The transaction gets delayed due to rapidly increased gas prices or due to a too low gas price set by the user.
  2. While the transaction is pending, the price of ETH lowers. If the transaction is minted at this moment, the user will get more ETH than they initially expected, due to the lowered ETH price.
  3. Since the ETH price is lower, sandwich attacks are more profitable because attackers may steal the positive change in ETH amount.
  4. A MEV bot sandwiches the user's transaction. The user gets the amount of ETH as set by their slippage tolerance, which was set at the price when the transaction was created and which is lower than the current price. The MEV bot gets the slippage tolerance + the extra ETH amount appeared due to the lowered ETH price.

    Impact

    Users funds may be stolen by MEV bots as a result of sandwich attacks.

    Code Snippet

    Uniswapper.sol#L39 PerpDepository.sol#L362

    Tool used

    Manual Review

    Recommendation

    Consider letting users set the deadline parameter and consider using a default value when it's not set. For example, Uniswap sets deadline to 5 minutes on L2 networks.

WarTech9 commented 1 year ago

Not a security vulnerability.

IAm0x52 commented 1 year ago

Invalid anyways. PerpDepository is deployed on OP which doesn't have a mempool and each transaction is it's own block, making sandwich attacks impossible.

hrishibhat commented 1 year ago

Closing based on above comments