If the NFT owner changes and there are still positions that have not been executed, it will result in financial losses for the user

Summary

Vulnerability Detail

When a user calls DelayedOrder.announceLeverageClose() to prepare for closing a position and LimitOrder.announceLimitOrder() for a limit order closure, the protocol locks the user's NFT, preventing it from being transferred. The NFT remains locked during the cancellation and execution of these requests. There are two attack scenarios:

Scenario 1:

1.User A calls DelayedOrder.announceLeverageOpen() to create a position and obtains an NFT. 2.User A calls LimitOrder.announceLimitOrder() to create a limit order with a price threshold that won't be executed in the short term. 3.User A sells the NFT on the secondary market. 4.User B purchases the NFT. 5.User A detects the transaction in the transaction pool and front-runs by executing DelayedOrder.announceLeverageClose() and LimitOrder.cancelLimitOrder() in the same transaction, unlocking the NFT. This allows User B to successfully purchase the NFT. 6.User B successfully purchased the NFT

_executeLeverageClose() is called. In this function, the protocol retrieves the order information based on _announcedOrder[account] and then calls LeverageModule.executeClose(). Within this function, the protocol settles the position based on the token ID, burns the NFT, and transfers funds to the account. However, at this point, the owner of the NFT has changed, resulting in financial loss for User B.



   // A position NFT has to be unlocked before burning otherwise, the transfer to address(0) will fail.
    _unlock(announcedClose.tokenId);
    _burn(announcedClose.tokenId);

    vault.updateStableCollateralTotal(int256(announcedClose.tradeFee)); // pay the trade fee to stable LPs

    // Settle the collateral.
    vault.sendCollateral({to: _keeper, amount: _order.keeperFee}); // pay the keeper their fee
    vault.sendCollateral({to: _account, amount: uint256(settledMargin) - totalFee}); // transfer remaining amount to the trader



Scenario 2:
1.User A calls `DelayedOrder.announceLeverageOpen()` to create a position and obtains an NFT.
2.User A calls `LimitOrder.announceLimitOrder()` to create a limit order with a price threshold that won't be executed in the short term.
3.User A calls `DelayedOrder.announceLeverageAdjust()` to adjust the position.
4.`DelayedOrder.executeOrder()` is called to execute the adjusted position, at which point the NFT has already been unlocked.
5.User A sells the NFT on the secondary market.
6.User B purchases the NFT, becoming the owner.
7.After some time, the limit order created by User A is mysteriously executed at a price not expected by User B, resulting in certain financial losses.

## Impact
User suffers financial losses.
## Code Snippet
https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/LimitOrder.sol#L119-L128
https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/LeverageModule.sol#L255-L317

## Tool used

Manual Review

## Recommendation
Check if the user has any unexecuted positions before transferring the NFT.

Duplicate of #48

sherlock-audit / 2023-12-flatmoney-judging

Bauer - If the NFT owner changes and there are still positions that have not been executed, it will result in financial losses for the user #86

If the NFT owner changes and there are still positions that have not been executed, it will result in financial losses for the user

Summary

Vulnerability Detail