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

0xadrii - Escrowed repayments belonging to the lender commitment contract can’t be retrieved from the escrow vault

Closed this issue · 0 comments

0xadrii

high

Escrowed repayments belonging to the lender commitment contract can’t be retrieved from the escrow vault

Summary

Escrowed repaid assets corresponding to the lender commitment contract can never be retrieved because the lender commitment does not implement a way to interact with the escrow to obtain the funds.

Vulnerability Detail

TellerV2 implements a safety mechanism to prevent ERC20 token transfers that fail from Dos’ing repayments. This is done inside the _sendOrEscrowFunds function, where if the transfer of repaid assets to the lender fails, assets will be deposited to the escrow vault so that the lender can later retrieve them:

// TellerV2.sol
function _sendOrEscrowFunds(uint256 _bidId, Payment memory _payment)
        internal
    {
        Bid storage bid = bids[_bidId];
        address lender = getLoanLender(_bidId);
 
        uint256 _paymentAmount = _payment.principal + _payment.interest;

        try 

            bid.loanDetails.lendingToken.transferFrom{ gas: 100000 }( 
                _msgSenderForMarket(bid.marketplaceId),
                lender,
                _paymentAmount
            )
        {} catch {
            
            address sender = _msgSenderForMarket(bid.marketplaceId);
 
            uint256 balanceBefore = bid.loanDetails.lendingToken.balanceOf(
                address(this)
            ); 

            //if unable, pay to escrow
            bid.loanDetails.lendingToken.safeTransferFrom(
                sender,
                address(this),
                _paymentAmount
            );

            uint256 balanceAfter = bid.loanDetails.lendingToken.balanceOf(
                address(this)
            );

            //used for fee-on-send tokens
            uint256 paymentAmountReceived = balanceAfter - balanceBefore;

            bid.loanDetails.lendingToken.approve(
                address(escrowVault),
                paymentAmountReceived
            );
 
            IEscrowVault(escrowVault).deposit(
                lender,
                address(bid.loanDetails.lendingToken),
                paymentAmountReceived
            );
        }

        ...
        }
    }

The LenderCommitmentGroup_Smart operates as a lender in TellerV2. Its pooled assets can be lent out to borrowers by creating new loans, and borrowers can then repay their assets back to the LenderCommitmentGroup_Smart contract.

If one of those token transfer fails when repaying back to LenderCommitmentGroup_Smart (maybe because that specific lendingToken performs gas-intensive transfers that cost more than 100000 units of gas), then the tokens will be deposited into the escrow vault on behalf of LenderCommitmentGroup_Smart. The only way to recover the assets from the escrow vault is to call its withdraw function being the msg.sender who actually has had the funds deposited:

// EscrowVault.sol

function withdraw(address token, uint256 amount) external {
        address account = _msgSender();

        balances[account][token] -= amount;
        ERC20(token).safeTransfer(account, amount);
    }

The problem is that the LenderCommitmentGroup_Smart does not have a way to call the withdraw function in the EscrowVault contract. Because of this, all repaid assets to the LenderCommitmentGroup_Smart contract that have failed and have been sent to the escrow vault will be unrecoverable, and will remain in the escrow vault forever.

Impact

High. If a token transfer fails when repaying a loan to the LenderCommitmentGroup_Smart contract (which is a possible scenario explicitly handled in _sendOrEscrowFunds), the funds will remain locked forever in the escrow vault. This will make the LenderCommitmentGroup_Smart contract insolvent, and liquidity providers in the LenderCommitmentGroup_Smart will never be able to retrieve their assets.

Code Snippet

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

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

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

Tool used

Manual Review

Recommendation

Add a function in the LenderCommitmentGroup_Smart contract that allows funds to be pulled from the Escrow Vault.

Duplicate of #126