c4-bot-7 commented 7 months ago

Lines of code

Vulnerability details

C4 issue

M-15: Users' newly created positions can be prematurely closed and removed from the vault directly after they are created

Comments

The V3Vault._repay() function allows anyone to repay a loan with zero assets. When a loan is "repaid" and the loan debt shares is zero, _repay() will call _cleanupLoan(). This function will transfer the NFT position for the closed loan back to the NFT owner. Normally this operation is acceptable except in the following case:

A user has transferred their NFT position to the Vault in anticipation of a future loan borrow.
A malicious user before V3Vault.borrow() is called frontruns the borrow() call and "repays" the loan via V3Vault.repay().
Since the malicious user doesn't have to pay anything for the repay, the NFT position is transferred back to the NFT owner forcing the NFT owner to re-send the NFT to the protocol if they want to borrow loans.

The core issue with _repay() is as follows:

There are no safety checks that the shares amount being deducted is greater than 0. This allows zero-cost repayments.
The NFT position is automatically pushed back to the NFT owner if the loan debt shares is zero.

Mitigation

PR #8, PR #32

Several major changes were made to prevent this exploit from occurring:

_repay() no longer calls _cleanupLoan(). Because of this, the NFT position is not pushed to the NFT owner. Now the NFT owner must call V3Vault.remove() to retrieve their NFT. This follows the pull-over-push pattern.
_repay() now has a guard check to see if the shares being repaid equal 0. If it does, the function reverts. This prevents repayers from making zero-cost repayments.

These two changes prevent a NFT being sent back to a repayer when the loan has no debt.

Conclusion

LGTM

c4-judge commented 7 months ago

jhsagd76 marked the issue as satisfactory