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

13 stars 11 forks source link

EgisSecurity - TellerV2.sol#_lenderCloseLoanWithRecipient() - Doesn't update borrower reputation #170

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

EgisSecurity

medium

TellerV2.sol#_lenderCloseLoanWithRecipient() - Doesn't update borrower reputation

Summary

TellerV2.sol#_lenderCloseLoanWithRecipient() - Doesn't update borrower reputation

Vulnerability Detail

The protocol implements borrower reputation, so that lenders can check if a borrower has ever been late/defaulted on one or more of his loans.

The only place where reputation is updated in inside _repayLoan. The function is called when a loan is being repaid (partially or fully).

The issue is that the reputation isn't updated when a loan is being closed by the lender.

bid.state is set to CLOSED, but because of the way isLoanDefaulted is written, a loan with a state of CLOSED isn't considered defaulted.

function _isLoanDefaulted(uint256 _bidId, uint32 _additionalDelay)
        internal
        view
        returns (bool)
    {
        Bid storage bid = bids[_bidId];

        // Make sure loan cannot be liquidated if it is not active
        if (bid.state != BidState.ACCEPTED) return false;

        uint32 defaultDuration = bidDefaultDuration[_bidId];

        uint32 dueDate = calculateNextDueDate(_bidId);

        return
            uint32(block.timestamp) >
            dueDate + defaultDuration + _additionalDelay;
    }

This is an issue as ReputationManager.sol#_applyReputation relies on isLoanDefaulted to set the borrower's mark to RepMark.Default.

function _applyReputation(address _account, uint256 _bidId)
        internal
        returns (RepMark mark_)
    {
        mark_ = RepMark.Good;

        if (tellerV2.isLoanDefaulted(_bidId)) {
            mark_ = RepMark.Default;

            // Remove delinquent status
            _removeMark(_account, _bidId, RepMark.Delinquent);
        } else if (tellerV2.isPaymentLate(_bidId)) {
            mark_ = RepMark.Delinquent;
        }

        // Mark status if not "Good"
        if (mark_ != RepMark.Good) {
            _addMark(_account, _bidId, mark_);
        }
    }

Even if updateAccountReputation is called after a loan is closed, _applyReputation won't apply the correct mark, as isLoanDefaulted returns false and the mark isn't applied.

Impact

The borrower can default on a loan, the loan gets closed, but his reputation isn't affected and doesn't receive a default mark, which will mislead other lenders, and breaks the idea behind marks.

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L752

Tool used

Manual Review

Recommendation

Mimic the logic inside _repayLoan:

function _lenderCloseLoanWithRecipient(
        uint256 _bidId,
        address _collateralRecipient
    ) internal acceptedLoan(_bidId, "lenderClaimCollateral") {
        require(isLoanDefaulted(_bidId), "Loan must be defaulted.");

        Bid storage bid = bids[_bidId];
        // Loan is closed, update the borrowers reputation
        RepMark mark = reputationManager.updateAccountReputation(
            bid.borrower,
            _bidId
        );
        bid.state = BidState.CLOSED;

        address sender = _msgSenderForMarket(bid.marketplaceId);
        require(sender == bid.lender, "only lender can close loan");

        /*

          address collateralManagerForBid = address(_getCollateralManagerForBid(_bidId)); 

          if( collateralManagerForBid == address(collateralManagerV2) ){
             ICollateralManagerV2(collateralManagerForBid).lenderClaimCollateral(_bidId,_collateralRecipient);
          }else{
             require( _collateralRecipient == address(bid.lender));
             ICollateralManager(collateralManagerForBid).lenderClaimCollateral(_bidId );
          }

          */

        collateralManager.lenderClaimCollateral(_bidId);

        emit LoanClosed(_bidId);
    }

Duplicate of #171