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

10 stars 9 forks source link

blockchain555 - Collateral assets are locked without being transferred to the liquidator in the `LenderCommitmentGroup_Smart.sol#liquidateDefaultedLoanWithIncentive()` function. #245

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 4 months ago

blockchain555

high

Collateral assets are locked without being transferred to the liquidator in the LenderCommitmentGroup_Smart.sol#liquidateDefaultedLoanWithIncentive() function.

Summary

In the LenderCommitmentGroup_Smart.sol#liquidateDefaultedLoanWithIncentive() function, collateral assets are implemented to be transferred to the liquidator, but collateral is transferred to the LenderCommitmentGroup_Smart contract, not to the liquidator. Therefore, the collateral is locked in the contract.

Vulnerability Detail

The liquidateDefaultedLoanWithIncentive() function is a function called 24 hours after loan default, and can be called by anyone if the collateral value is greater than the principal + difference.

    function liquidateDefaultedLoanWithIncentive(
        uint256 _bidId,
        int256 _tokenAmountDifference
    ) public bidIsActiveForGroup(_bidId) {
        uint256 amountDue = getAmountOwedForBid(_bidId, false);

        uint256 loanDefaultedTimeStamp = ITellerV2(TELLER_V2)
            .getLoanDefaultTimestamp(_bidId);

        int256 minAmountDifference = getMinimumAmountDifferenceToCloseDefaultedLoan(
                amountDue,
                loanDefaultedTimeStamp
            );

        require(
            _tokenAmountDifference >= minAmountDifference,
            "Insufficient tokenAmountDifference"
        );

        if (_tokenAmountDifference > 0) {
            //this is used when the collateral value is higher than the principal (rare)
            //the loan will be completely made whole and our contract gets extra funds too
            uint256 tokensToTakeFromSender = abs(_tokenAmountDifference);

            IERC20(principalToken).transferFrom(
                msg.sender,
                address(this),
                amountDue + tokensToTakeFromSender
            );

            tokenDifferenceFromLiquidations += int256(tokensToTakeFromSender);

            totalPrincipalTokensRepaid += amountDue;
        } else {

            uint256 tokensToGiveToSender = abs(_tokenAmountDifference);

            IERC20(principalToken).transferFrom(
                msg.sender,
                address(this),
                amountDue - tokensToGiveToSender
            );

            tokenDifferenceFromLiquidations -= int256(tokensToGiveToSender);

            totalPrincipalTokensRepaid += amountDue;
        }

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

As you can see, the collateral is expected to be transferred to the caller. However, in the lenderCloseLoanWithRecipient() function, the collateral is transferred to this contract, not the caller.

TellerV2.sol#lenderCloseLoanWithRecipient():

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

TellerV2.sol#_lenderCloseLoanWithRecipient():

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

771:    collateralManager.lenderClaimCollateral(_bidId);

        emit LoanClosed(_bidId);
    }

CollateralManager.sol#lenderClaimCollateral():

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

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

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

As you can see in #L280, collateral is withdrawn from the lender.

However, as shown below, the lender is the LenderCommitmentGroup_Smart contract itself. LenderCommitmentGroup_Smart.sol#_acceptBidWithRepaymentListener():

    function _acceptBidWithRepaymentListener(uint256 _bidId) internal {
385:    ITellerV2(TELLER_V2).lenderAcceptBid(_bidId); //this gives out the funds to the borrower

        ILoanRepaymentCallbacks(TELLER_V2).setRepaymentListenerForBid(
            _bidId,
            address(this)
        );
    }

TellerV2.sol#lenderAcceptBid():

    function lenderAcceptBid(uint256 _bidId)
        external
        override
        pendingBid(_bidId, "lenderAcceptBid")
        whenNotPaused
        returns (
            uint256 amountToProtocol,
            uint256 amountToMarketplace,
            uint256 amountToBorrower
        )
    {
        // Retrieve bid
        Bid storage bid = bids[_bidId];

        address sender = _msgSenderForMarket(bid.marketplaceId);

        SNIP...

        // Declare the bid acceptor as the lender of the bid
518:    bid.lender = sender;
        SNIP...
    }

Therefore, the collateral is transferred to the LenderCommitmentGroup_Smart contract, not the liquidator. As a result, the collateral is locked in this contract.

Impact

Collateral is not transferred to the liquidator and it is locked in the LenderCommitmentGroup_Smart contract. Therefore, the liquidator cannot receive collateral corresponding to the loan.

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#L422-L472 https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L724-L774

Tool used

Manual Review

Recommendation

Modify the LenderCommitmentGroup_Smart.sol#liquidateDefaultedLoanWithIncentive() function as follows. function liquidateDefaultedLoanWithIncentive( uint256 _bidId, int256 _tokenAmountDifference ) public bidIsActiveForGroup(_bidId) { SNIP...

+++ uint256 collateralBefore = IERC20(collateralToken).balanceOf(address(this));
//this will give collateral to the caller ITellerV2(TELLER_V2).lenderCloseLoanWithRecipient(_bidId, msg.sender);

+++ uint256 collateralAfter = IERC20(collateralToken).balanceOf(address(this)); +++ if(collateralAfter > collateralBefore) +++ IERC20(collateralToken).transfer(msg.sender, collateralAfter - collateralBefore); }

Duplicate of #46