Incomplete Auction State Management in startLiquidation, and _endLiquidation Function in LendingPool.sol

Summary

These function fails to correctly call setAuctionInProgress function for the most junior tranche when multiple auctions are active. This could lead to unintended deposits or withdrawals in that tranche during concurrent liquidations, potentially affecting liquidity management and risk exposure.

Vulnerability Detail

The code checks if auctionsInProgress is equal to 0 before calling setAuctionInProgress. If auctionsInProgress is already greater than 0 (indicating another ongoing auction), the call to setAuctionInProgress is skipped. This means the most junior tranche might not be flagged as having an ongoing auction, even though an additional liquidation has started.

    function startLiquidation(address initiator, uint256 minimumMargin_)
        external
        override
        whenLiquidationNotPaused
        processInterests
        returns (uint256 startDebt)
    {
       ....
        if (auctionsInProgress == 0 && tranches.length > 0) {
            unchecked {
                ITranche(tranches[tranches.length - 1]).setAuctionInProgress(true);
            }
        }

      ....
    }

Impact

Inconsistent Tranche State: The state of the junior tranche regarding ongoing auctions becomes inconsistent with the actual number of running auctions. This could lead to issues with other functions relying on this flag.
Unexpected Function Behavior: Dependent functions might handle the tranche incorrectly if they expect it to be marked as having an ongoing auction.
Security Concerns: In extreme cases, depending on the specific implementation of dependent functions, this inconsistency could potentially be exploited for unintended behavior or security vulnerabilities.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Remove the check for auctionsInProgress == 0 before calling setAuctionInProgress. This ensures the junior tranche is always flagged as having an ongoing auction whenever startLiquidation is called. Alternatively, consider using a different mechanism to track ongoing auctions per tranche instead of a global counter. This would provide more granular control and avoid potential inconsistencies.


    // Set auctionInProgress flag unconditionally for most junior tranche
    unchecked {
        if (auctionInProgress && tranches.length > 0) {
            ITranche(tranches[tranches.length - 1]).setAuctionInProgress(true);
        }
    }

    // ... (remaining code)
}

sherlock-admin2 commented 9 months ago

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

takarez commented:

invalid: it checks whether the flag has already been set.

nevillehuang commented 9 months ago

Invalid, if no auctions is in progress, there is no reason to stop deposits and withdrawals in the most junior tranches

sherlock-admin2 commented 9 months ago

Escalate

The intended behaviour of this code is to restrict certain financial operations in the last tranche by calling “setAuctionInProgress(true)”, which in turn restricts the tranches’ financial operation by setting the auctionInProgress bool value to be true. However, the intended behaviour can be skipped easily.

For example, the 'auctionsInProgress == 0' condition in startLiquidation only checks if there are currently no active auctions. However, the auctionsInProgress variable is incremented unconditionally, even if this condition fails.

So, If one or more auctions are already in progress(i.e., uint16 auctionsInProgress=2 or whatever value >0 ), the condition fails, and the code doesn't set the auctionInProgress flag for the last tranche to be true. This means the intended restrictions won't be applied to the most junior tranche, potentially leading to unintended transactions.

Also, The auctionsInProgress variable is incremented regardless of the condition, resulting in an inaccurate count of ongoing auctions. This could lead to incorrect decisions or behaviours based on this count.

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page, in the Sherlock webapp.

sherlock-audit / 2023-12-arcadia-judging

anya - Incomplete Auction State Management in startLiquidation, and _endLiquidation Function in LendingPool.sol #206