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

10 stars 9 forks source link

Bauer - Front-running repayments to steal rewards from the protocol #163

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

Bauer

high

Front-running repayments to steal rewards from the protocol

Summary

A malicious actor can front-run the repayment of a loan, thereby stealing rewards from the protocol.

Vulnerability Detail

The LenderCommitmentGroup_Smart contract, acting as a lender, provides funds to borrowers. When a borrower's loan is repaid, the protocol calls LenderCommitmentGroup_Smart.repayLoanCallback() to increase the values of totalPrincipalTokensRepaid and totalInterestCollected.

    function repayLoanCallback(
        uint256 _bidId,
        address repayer,
        uint256 principalAmount,
        uint256 interestAmount
    ) external onlyTellerV2 {
        //can use principal amt to increment amt paid back!! nice for math .
        totalPrincipalTokensRepaid += principalAmount;
        totalInterestCollected += interestAmount;
    }

The LenderCommitmentGroup_Smart.addPrincipalToCommitmentGroup() function allows users to deposit principal tokens to receive shares, where rate = poolTotalValue/totalSupply and shares = amount / rate.

function addPrincipalToCommitmentGroup(
        uint256 _amount,
        address _sharesRecipient
    ) external returns (uint256 sharesAmount_) {
        //transfers the primary principal token from msg.sender into this contract escrow

        principalToken.transferFrom(msg.sender, address(this), _amount);

        sharesAmount_ = _valueOfUnderlying(_amount, sharesExchangeRate());

        totalPrincipalTokensCommitted += _amount;
        //principalTokensCommittedByLender[msg.sender] += _amount;

        //mint shares equal to _amount and give them to the shares recipient !!!
        poolSharesToken.mint(_sharesRecipient, sharesAmount_);
    }

The burnSharesToWithdrawEarnings() function allows users to burn shares and withdraw assets, where rate = poolTotalValue/totalSupply and sharesExchangeRateInverse = 1/rate. The extracted asset principalTokenValueToWithdraw = amount / sharesExchangeRateInverse.

 function burnSharesToWithdrawEarnings(
        uint256 _amountPoolSharesTokens,
        address _recipient
    ) external returns (uint256) {

        poolSharesToken.burn(msg.sender, _amountPoolSharesTokens);

        uint256 principalTokenValueToWithdraw = _valueOfUnderlying(
            _amountPoolSharesTokens,
            sharesExchangeRateInverse()
        );

        totalPrincipalTokensWithdrawn += principalTokenValueToWithdraw;

        principalToken.transfer(_recipient, principalTokenValueToWithdraw);

        return principalTokenValueToWithdraw;
    }

The issue here is front-running repayLoanCallback(). When a malicious actor monitors transactions in the transaction pool and detects a loan repayment transaction being prepared, the bad actor executes addPrincipalToCommitmentGroup() with higher gas to acquire shares preemptively. Then, after the repayment transaction is executed, the bad actor executes burnSharesToWithdrawEarnings() to withdraw assets. Since totalInterestCollected increases during the loan repayment, poolTotalValue increases as well. As a result, totalPrincipalTokensWithdrawn calculated in the burnSharesToWithdrawEarnings() function is larger than expected, allowing the user to withdraw more assets.

Impact

Stealing rewards from the protocol.

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#L700-L709

Tool used

Manual Review

Recommendation

Duplicate of #110