pkqs90 - `LenderCommitmentGroup_Smart.sol#liquidateDefaultedLoanWithIncentive` does NOT send collateral to caller, but to the `LenderCommitmentGroup_Smart` pool

[sherlock-admin3]

pkqs90

high

LenderCommitmentGroup_Smart.sol#liquidateDefaultedLoanWithIncentive does NOT send collateral to caller, but to the LenderCommitmentGroup_Smart pool

Summary

The core issue is: the function TellerV2.sol#lenderCloseLoanWithRecipient(uint256 _bidId, address _collateralRecipient) does NOT send the collateral to _collateralRecipient, but to the lender instead.

This causes that when users call LenderCommitmentGroup_Smart.sol#liquidateDefaultedLoanWithIncentive to liquidate a loan, the collateral goes to the LenderCommitmentGroup_Smart contract, and not to the user. The collateral is also locked since there is no way to retrieve it.

Vulnerability Detail

First, let's see LenderCommitmentGroup_Smart.sol#liquidateDefaultedLoanWithIncentive. At the very last of the function, lenderCloseLoanWithRecipient is called and the msg.sender is passed as user address.

    function liquidateDefaultedLoanWithIncentive(
        uint256 _bidId,
        int256 _tokenAmountDifference
    ) public bidIsActiveForGroup(_bidId) {
        ...

        //this will give collateral to the caller
>       ITellerV2(TELLER_V2).lenderCloseLoanWithRecipient(_bidId, msg.sender);
    }

Then, let's see TellerV2.sol#lenderCloseLoanWithRecipient. The call stack is lenderCloseLoanWithRecipient() -> _lenderCloseLoanWithRecipient() -> CollateralManager.sol#lenderClaimCollateral -> CollateralManager.sol#_withdraw. We can see that the address _collateralRecipient is not used at all, and the collateral goes to tellerV2.getLoanLender(_bidId) instead.

    function lenderCloseLoanWithRecipient(
        uint256 _bidId,
        address _collateralRecipient
    ) external {
        _lenderCloseLoanWithRecipient(_bidId, _collateralRecipient);
    }

    function _lenderCloseLoanWithRecipient(
        uint256 _bidId,
        address _collateralRecipient
    ) 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);
    }

CollateralManager.sol

    function lenderClaimCollateral(uint256 _bidId) external onlyTellerV2 {
        if (isBidCollateralBacked(_bidId)) {
            BidState bidState = tellerV2.getBidState(_bidId);

            require(
                bidState == BidState.CLOSED,
                "Loan has not been liquidated"
            );

            _withdraw(_bidId, tellerV2.getLoanLender(_bidId));
            emit CollateralClaimed(_bidId);
        }
    }

Impact

The liquidation feature for LenderCommitmentGroup_Smart does not work, and user collateral would be locked in LenderCommitmentGroup_Smart forever.

Code Snippet

• https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L471
• https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L738-L743

Tool used

Manual review.

Recommendation

Send the collateral to passed _collateralRecipient for TellerV2.sol#lenderCloseLoanWithRecipient(uint256 _bidId, address _collateralRecipient).

Duplicate of #46