Summary

Nft is not burnt when position is closed. Because position ids can be reused, this can cost position for user.

Vulnerability Details

When user creates short position then he has ability to mint nft. This will set tokenId to short record and also will store nftMapping for owner which means that user owns position. This mapping also tracks short position id that token is assigned to.

As with usual nft, user can provide approve on it for some specific address.

The problem now is that when user closes position, or it's closed on behalf of it, then this nft isn't burnt and LibShortRecord.burnNFT is not called. So when position will be closed, then LibShortRecord.deleteShortRecord function will be called and this function will mark id as canceled, so next order can reuse this id. But nftMapping inside ERC721Facet is not removed and still assigned to canceled short id. Also all approves are not removed.

So what can happen?
1.User has short position with id 5 and he minted nft with id 10 for it.
2.User provided approve for that position for some address and that approve was not used.
3.User's position got closed and order id 5 is marked as canceled for future use. Approve for token 10 was not removed.
4.User later create another short position and id 5 is assigned to it.
5.User that has approve for token 10 now can transfer this position and steal funds.

Impact

User lost funds

Tools Used

VsCode

Recommendations

When position is closed, then remove nft and all approves for nft.
This is already done inside ShortRecordFacet.combineShorts.