ArmedGoose - `Emergency repayment` and `takeOverDebt` rely on the same conditions as Liquidations, making them prone to frontrunning by liquidators

[sherlock-admin]

ArmedGoose

medium

Emergency repayment and takeOverDebt rely on the same conditions as Liquidations, making them prone to frontrunning by liquidators

Summary

Either takeOverDebt and emergency repayment require the collateralBalance to drop below zero until they can be executed. But at this point, it is also requirement for a liquidation, so bots specialized in sniping liquidations will target those positions before any user will be able to interact with them. Nowadays the liquidations are being handled often by specialized bots, competing for the profit made off them, which also requires them to instantly detect and take advantage of liquidation opportunities across DeFi platforms (to get the liquidation bonuses).

Vulnerability Detail

Emergency repayment and takeOverDebt on a position can only be done when collateralBalance >= 0 (described further in Code section). Once a position is in that state, it is also liquidatable. In normal market conditions it is very likely that noone will be able to conduct these operations because liquidations will come first, frontrunning those users.

Impact

Users who wish to emergency repay their position will not be able to do so. Taking over debt, for example to save user's position by another user, will not be successful and the user with debt to be taken will lose funds, because can be liquidated. Similarly with emergency repay.

Code Snippet

1) takeOverDebt

Here if collateralBalance >= 0 it reverts as per the revertError definition (note: the comment says otherwise for some reason) . So in order to takeOverDebt user willing to do it has to wait until this value drops below zero:

            // Ensure that the collateral balance is greater than or equal to 0
            (collateralBalance >= 0).revertError(ErrLib.ErrorCode.FORBIDDEN);

2) emergency Repay

Emergency repay will be performed by the LP'er (so not borrowing.borrower) therefore to avoid revert, collateralBalance has to be below zero so the condition is not met and there is no revert.

            (msg.sender != borrowing.borrower && collateralBalance >= 0).revertError(
                ErrLib.ErrorCode.INVALID_CALLER
            );

Later, there is a conditional check if the operation is emergency or not. So until this point condition for emergency repayment and for liquidation are also the same.

Tool used

Manual Review

Recommendation

The threshold for emergency repayment and taking over debt should be higher than threshold for liquidation, e.g. when there's 5% remaining of collateral. So anyone can do emergency repayment or take over debt in this time window. Otherwise, most of the liquidations will always come first as soon as the threshold for liquidation is reached.

[sherlock-admin2]

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

tsvetanovv commented:

This cannot be an issue of protocol. It is important to them that the liquidation works properly