sherlock-audit/2024-04-teller-finance-judging

0x3b - liquidateDefaultedLoanWithIncentive sends the collateral to the wrong account

Opened this issue · 2 comments

0x3b

high

liquidateDefaultedLoanWithIncentive sends the collateral to the wrong account

Summary

liquidateDefaultedLoanWithIncentive sends the collateral to the Lender - LenderCommitmentGroup (LCG) instead of the liquidator. Liquidators will not be incentivized to liquidate.

Vulnerability Detail

liquidateDefaultedLoanWithIncentive is intended to liquidate bids, where liquidators pay off the debt and receive the collateral. However, currently the collateral is sent to the lender - LCG, because lenderCloseLoanWithRecipient includes msg.sender as its second parameter but does not utilize it:

    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");

        //@audit we directly call `lenderClaimCollateral`
        collateralManager.lenderClaimCollateral(_bidId);
    }

lenderClaimCollateral in its place withdraws the collateral directly to the lender - LCG, without updating totalPrincipalTokensRepaid. Some effects:

  • Liquidators will gain 0 profits, so they will not liquidate.
  • LPs suffer losses as liquidations are not carried out, and returned collateral from liquidations is not accrued as totalPrincipalTokensRepaid, increasing utilization, eventually bricking the contract.

Impact

Liquidators will not be incentivized to liquidate and incorrect accounting occurs inside LCG.

Code Snippet

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

Tool used

Manual Review

Recommendation

Ensure _lenderCloseLoanWithRecipient sends the funds to _collateralRecipient.

The protocol team fixed this issue in the following PRs/commits:
https://github.com/teller-protocol/teller-protocol-v2-audit-2024/pull/16/files

The Lead Senior Watson signed off on the fix.