code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

The protocol is not ready for Position Transfers as declared #165

Closed c4-bot-7 closed 5 months ago

c4-bot-7 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PositionNFTs.sol#L11

Vulnerability details

Impact

Loss of funds

Proof of Concept

Wiselending declares in their docs that the Position NFTs are transferable, hence it´s a value transfer as well as the NFT transfer.

The quoted text below is referred from the docs link;

Each user mints an NFT for their account when they first interact. This allows for transferrable positions, as well as modular features to be built on top of the base protocol, such as an options market.

However, the protocol doesn´t provide an implementation of this feature. In fact. there are only simple ERC721 functions such as transfer or transferFrom to transfer the NFT and the position. But these actions are prone to phishing the users.

Accordingly;

  1. A position holder can deal to sell his position NFT
  2. In one batch, the owner can close their position, withdraw all funds & transfer the Position to the new owner
  3. The new owner receives the NFT without holding any value

OR

The owner can leverage their position close to the liquidation level, transfer the NFT, and liquidate the new owner.

By promoting the existing situation without the validation of the NFTs holding value, the transfer of the Position NFTs could be a phishing tool.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend overriding _beforeTokenTransfer & _afterTokenTransfer hooks of ERC721 with functionality that checks the position doesn´t have debt, and that the position holds the same value or more with 1 block earlier.

Assessed type

MEV

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

GalloDaSballo commented 5 months ago

OOS

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Out of scope