pash0k - LendingPool.sol::donateToTranche() may be frontrun

[sherlock-admin]

pash0k

medium

LendingPool.sol::donateToTranche() may be frontrun

Summary

LendingPool.sol::donateToTranche() is vulnerable to frontrun, which may cause the attacker to get the majority of funds donated to the tranche.

Vulnerability Detail

LendingPool.sol#L350-L363
LendingPool.sol::donateToTranche() is a function that enables anyone to voluntarily send funds to the tranche and increase realisedLiquidityOf[tranche].

Though users are not expected to just donate funds, this function is made for manual settling of liquidation auctions, in cases when the auction was not bought before cutoffTime. Liquidator.sol#L456-L461

In that case the remaining collateral of the account being liquidated is transferred to some admin address, sold manually by the admin and then the funds are expected to be added back to the tranche via LendingPool.sol::donateToTranche().

donateToTranche takes two arguments: trancheIndex and assets. Attacker can frontrun the call to this function by depositing large amount of funds to the tranche (using Tranche.sol::deposit() or Tranche.sol::mint()) and then backrun the call with redeeming their shares from the tranche (using Tranche.sol::redeem()).

As donateToTranche increases the realisedLiquidityOf[tranche], and the amount of assets sent to the user when redeeming their shares is calculated using this value, attacker gets instant profit from sandwiching the call to LendingPool.sol::donateToTranche(). Specific amounts or percentages gained by the attacker depend on the current state of the tranche: the less funds it currently holds, the less capital it is required for the attack.

Risk for the malicious user is very low: after depositing they can lose funds only in case of price volatility of underlying asset or another liquidation with bad debt occuring. Chance of the latter in short period of time is low and price volatility is zero for stablecoins or can be hedged otherwise.

Impact

As it was specified earlier, main purpose of donateToTranche is to manually settle the liquidation auctions. In this case, from the LendingPool.sol::settleLiquidationUnhappyFlow() we can see, that _processDefault() is called, which subtracts the bad debt from the junior tranche. So, the funds sent via donateToTranche is a part of the funds taken from the junior tranche as a part of settling.

If attacker can take most of these funds by sandwiching the call, the main victim are the users who provided liquidity to the junior tranche, as these are basically their assets that got stolen.

So, the impact is users' loss of funds.

Code Snippet

LendingPool.sol::donateToTranche()

function donateToTranche(uint256 trancheIndex, uint256 assets) external whenDepositNotPaused processInterests {
    if (assets == 0) revert LendingPoolErrors.ZeroAmount();

    address tranche = tranches[trancheIndex];

    // Need to transfer before donating or ERC777s could reenter.
    // Address(this) is trusted -> no risk on re-entrancy attack after transfer.
    asset.safeTransferFrom(msg.sender, address(this), assets);

    unchecked {
        realisedLiquidityOf[tranche] += assets; //[̲̅$̲̅(̲̅ ͡° ͜ʖ ͡°̲̅)̲̅$̲̅] 
        totalRealisedLiquidity = SafeCastLib.safeCastTo128(assets + totalRealisedLiquidity);
    }
}

Tool used

Manual Review

Recommendation

I suggest to implement some sort of vesting (linear, for example) instead of a straightforward donating, as this will smoothen the realisedLiquidityOf[tranche] increase.

Duplicate of #121

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid

[pa-sh0k]

In the discord channel the sponsor said: "A combination of Front and Back running not possible on Base, Optimism and Arbitrum due to private sequencers". I totally agree with this point, but there is no need to perform a MEV sandwich attack to cause user’s loss of funds in this case. Performing such attack in one block just maximizes attacker’s profit and user’s loss, however it is almost not possible on the mentioned chains at the moment.

As mentioned in the “Impact”, donateToTranche is expected to be called by the admin after they manually sell the unsold collateral.

This actually means that sooner or later, after every auction that ended in unhappy flow with unsold collateral, donateToTranche will be called, so frontrunning a call to donateToTranche is actually the same as backrunning a call to Liquidator.sol:: _settleAuction(), just with a bigger time lag. And backrunning is absolutely possible on any chain, as here it doesn’t have to be in the same block.

Of course, as the donateToTranche call won’t happen instantly after _settleAuction, so EV of attacker’s profit will be lower compared to a MEV-alike attack, as there are risks associated with depositing to the tranche for more than one block (though some of them can be hedged).

But the biggest concern here is not the attacker’s profit, but users' losses, and their expected value is the same in MEV-like scenario and the scenario that I have provided, which is totally possible on the mentioned chains.