samuraii77 - Lenders might not be able to close their loans and get their collateral back in the case of default
Closed this issue · 0 comments
samuraii77
high
Lenders might not be able to close their loans and get their collateral back in the case of default
Summary
Lenders might not be able to close their loans and get their collateral back in the case of default because of a state change upon a lender claiming a loan NFT.
Vulnerability Detail
Lenders can call the claimLoanNFT()
function in order to mint a loan NFT.
function claimLoanNFT(uint256 _bidId)
external
acceptedLoan(_bidId, "claimLoanNFT")
whenNotPaused
{
// Retrieve bid
Bid storage bid = bids[_bidId];
address sender = _msgSenderForMarket(bid.marketplaceId);
require(sender == bid.lender, "only lender can claim NFT");
// set lender address to the lender manager so we know to check the owner of the NFT for the true lender
bid.lender = address(USING_LENDER_MANAGER);
// mint an NFT with the lender manager
lenderManager.registerLoan(_bidId, sender);
}
This changes the bid.lender
variable to the USING_LENDER_MANAGER
address. The comment above the state change states that they should check the owner of the NFT for the actual lender. That is the reason they have a special getter function getLoanLender()
:
function getLoanLender(uint256 _bidId)
public
view
returns (address lender_)
{
lender_ = bids[_bidId].lender;
if (lender_ == address(USING_LENDER_MANAGER)) {
return lenderManager.ownerOf(_bidId);
}
//this is left in for backwards compatibility only
if (lender_ == address(lenderManager)) {
return lenderManager.ownerOf(_bidId);
}
}
The issue is that this function is not used when a lender tries to close his loan when it is defaulted. A lender calls lenderCloseLoan()
:
function lenderCloseLoan(uint256 _bidId)
external
acceptedLoan(_bidId, "lenderClaimCollateral")
{
Bid storage bid = bids[_bidId];
address _collateralRecipient = bid.lender;
_lenderCloseLoanWithRecipient(_bidId, _collateralRecipient);
}
The function uses the bid.lender
as the _collateralRecipient
. Then, upon calling the internal _lenderCloseLoanWithRecipient()
, the mistake is done again as it is required that the sender
is the bid.lender
which is not the case as the bid.lender
was changed upon claiming the loan NFT.
function _lenderCloseLoanWithRecipient(
uint256 _bidId,
address _collateralRecipient // @audit _collateralRecipient is not used
) internal acceptedLoan(_bidId, "lenderClaimCollateral") {
require(isLoanDefaulted(_bidId), "Loan must be defaulted.");
Bid storage bid = bids[_bidId];
bid.state = BidState.CLOSED;
address sender = _msgSenderForMarket(bid.marketplaceId);
require(sender == bid.lender, "only lender can close loan");
collateralManager.lenderClaimCollateral(_bidId);
emit LoanClosed(_bidId);
}
Impact
Lender with a loan that has defaulted will not be able to get the collateral.
Code Snippet
https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L745-L774
https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L578-L594
Tool used
Manual Review
Recommendation
Use getLoanLender()
instead.
Duplicate of #11