ALL liquidation operations can be sandwiched to extract value from the protocol

Summary

Currently, LenderCommitmentGroup_Smart does not use the normal IRM (Interest Rate Model). When liquidation happens, it distribute profit/socialize loss to ALL depositors, no matter how long they've been in the market. That makes it possible to sandwich liquidation to extract value from the protocol.

Vulnerability Detail

function liquidateDefaultedLoanWithIncentive(
    uint256 _bidId,
    int256 _tokenAmountDifference
) public bidIsActiveForGroup(_bidId) {

    ...

    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
    ITellerV2(TELLER_V2).lenderCloseLoanWithRecipient(_bidId, msg.sender);
}

In liquidation function, tokenDifferenceFromLiquidations is updated immediately, which changes sharesExchangeRate at once.

So, an attacker can:

observe a liquidation which profit all LP.
frontrun the liquidation to addPrincipalToCommitmentGroup.
liquidation is executed.
burnSharesToWithdrawEarnings to profit and leave.

OR:

attacker already holds some shares.
observe a liquidation which profit all LP.
frontrun the liquidation to burnSharesToWithdrawEarnings.
liquidation is executed.
addPrincipalToCommitmentGroup to profit and leave.

Impact

Attacker can make other users suffer more loss & earn less yield.

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

Tool used

Manual Review

Recommendation

Here are some possible mitigations:

IRM models can be introduced.
imply a fee in withdraw.

Duplicate of #110

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

BoRonGod - ALL liquidation operations can be sandwiched to extract value from the protocol #251