H-06: Owner of a position can prevent liquidation due to the 'onERC721Received' callback
Comments
Revert when liquidating a position transfers the collateralized NFT position of the loan back to the loan owner. This transfer occurs via safeTransferFrom(). Because of this, the loan owner will receive a call when the NFT position is transferred. The loan owner can then choose to revert the call, leading to the liquidation reverting altogether.
This issue allows any loan owner to revert a liquidation at no cost.
Mitigation
PR #8, PR #32
To resolve this issue, Revert implements a pull-over-push pattern for retrieving NFTs. The safeTransferFrom() called in V3Vault._cleanupLoan() is removed. This prevents a call being made to the loan owner. In addition, _cleanupLoan() will now only call _updateAndCheckCollateral() and delete the loan from the loans mapping.
Finally, Revert adds a V3Vault.remove() function which allows the loan owner to retrieve their NFT. This isolates the transfer of the NFT position to just the remove() function.
Lines of code
Vulnerability details
// start here
https://github.com/code-423n4/2024-03-revert-lend-findings/issues/54
C4 issue
H-06: Owner of a position can prevent liquidation due to the 'onERC721Received' callback
Comments
Revert when liquidating a position transfers the collateralized NFT position of the loan back to the loan owner. This transfer occurs via safeTransferFrom(). Because of this, the loan owner will receive a call when the NFT position is transferred. The loan owner can then choose to revert the call, leading to the liquidation reverting altogether.
This issue allows any loan owner to revert a liquidation at no cost.
Mitigation
PR #8, PR #32
To resolve this issue, Revert implements a pull-over-push pattern for retrieving NFTs. The safeTransferFrom() called in V3Vault._cleanupLoan() is removed. This prevents a call being made to the loan owner. In addition, _cleanupLoan() will now only call _updateAndCheckCollateral() and delete the loan from the loans mapping.
Finally, Revert adds a V3Vault.remove() function which allows the loan owner to retrieve their NFT. This isolates the transfer of the NFT position to just the remove() function.
Conclusion
LGTM