search

sherlock-audit / 2023-12-flatmoney-judging

9 stars 8 forks source link

0xvj - An attacker can bypass the leverage position NFT locking mechanism #231

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

0xvj

high

An attacker can bypass the leverage position NFT locking mechanism

Summary

A leverage trader can still transfer his Position NFT eventhough he has a pending announced order by opening and closing a limit order which unlocks his NFT.

Vulnerability Detail

When a leverage trader announces an order like LeverageAdjust or LeverageClose his Position NFT should be locked so the owner cannot transfer it to others after creating an order.

But an attacker can still unlock his position NFT eventhough it has an open order waiting for execution , by opening a limit and instantly closing it. Because cancelLimitOrder() unlocks position NFT eventhough it has an open order created through DelayedOrders module.

First of all the Position NFT should be locked whenever a leverage trader opens an order, the margin will be transferred to the user who created the order instead of NFT owner at the time of order execution.

Attack steps

  1. Alice owns a leveraged position NFT.
  2. Alice calls announceLeverageClose() to close her leverage position, which will lock her position NFT to restrict transfers.
  3. Now Alice creates a fake limit order by calling announceLimitOrder() of LimitOrder and then immediately cancels it using cancelLimitOrder() function , which unlocks her leverage position NFT.
  4. Now the attacker can transfer her position NFT to Bob or some protocol which accepts this NFT as collateral(external protocol integration).
  5. When the pending order created in step 2 executes after delay Alice will receive collateral instead Bob receiving the collateral who is the actual owner of NFT.

Impact

An attacker can bypass the leverage position NFT locking mechanism

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/DelayedOrder.sol#L298

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/LimitOrder.sol#L76

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/LimitOrder.sol#L108

Tool used

Manual Review

Recommendation

Don't unlock the position NFT in cancelLimitOrder() function in LimitOrder module if the user already have a pending leverage adjust/close order.

Duplicate of #48

sherlock-admin commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: medium(4)