Revert Lend - 0xBugSlayer's results

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L397-L402 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L427-L474 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L1075-L1085

Vulnerability details

Impact

User can create a position that can't be liquidated by assigning the position NFT ownership to a maliciously created contract

Proof of Concept

Attack flow:

1. User creates a contract and implements onERC721Received() function, which will always return value that reverts the ERC721::safeTransfer()/safeTransferFrom() functions.
2. The malicious user calls the V3Vault::create(uint256 tokenId, address recipient) function and as input for the recipient address, puts the contract he created, which makes this contract the owner of the position NFT in the V3Valut contract logic.
3. Then calls the borrow() function through the malicious contract
4. After some time his position goes unhealthy and the V3Vault::liquidate() function is called
5. Midway through the liquidation, the _cleanupLoan() function is called. It sends the position NFT to the owner of the position, which in the V3Vault contract logic is the malicious contract with implemented onERC721Received() that will always return value that makes the ERC721::safeTransferFrom() function revert, which means that the V3Vlaut::liquidate() function reverts as well.

All of this means that the position can never be liquidated. This attack can be easily performed by any user, which makes its severity not high but critical!

Tools Used

Manual review.

Recommended Mitigation Steps

This can be fixed by changing the ERC721::safeTransferFrom() with ERC721::transferFrom() function.

Assessed type

DoS