search

code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

Borrowers can DoS liquidations by repaying as little as 1 share. #237

Open c4-bot-1 opened 5 months ago

c4-bot-1 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseSecurity/WiseSecurityHelper.sol#L895-L899

Vulnerability details

Impact

Liquidations can be DoSed which increments the risk of bad debt being generated on position.

Proof of Concept

When a liquidator is liquidating a position in WiseLending, the liquidator needs to specify the amount of shares to be repaid. The liquidation logic checks if the positions are indeed liquidable, if so, it validates if the number of shares to be liquidated exceeds the total amount of shares that can be liquidated by using the WiseSecurityHelper.checkMaxShares() function. If the amount of shares the liquidator intends to liquidate exceeds the maximum amount of liquidable shares, the execution is reverted

The problem with this approach is that borrowers can frontrun the liquidation and repay as little as 1 share of the existing debt on the same pool that the liquidator decided to liquidate the debt from. This will cause when the liquidation is executed, the total borrow shares of the user on the pool being liquidated to be lower. If the liquidator was trying to liquidate the maximum possible amount of shares, now, the maxShares that can be liquidated will be slightly less than the amount of shares that the liquidator specified, which will cause the tx to revert.

function checkMaxShares(
    uint256 _nftId,
    address _tokenToPayback,
    uint256 _borrowETHTotal,
    uint256 _unweightedCollateralETH,
    uint256 _shareAmountToPay
)
    public
    view
{
    //@audit-ok => total borrowShares a position owns for a _tokenToPayback pool
    uint256 totalSharesUser = WISE_LENDING.getPositionBorrowShares(
        _nftId,
        _tokenToPayback
    );

    //@audit-info => If baddebt, maxShares that can be liquidated are the totalSharesUser
    //@audit-info => If not baddebt, maxShares can be 50% of the total borrowShares
    uint256 maxShares = checkBadDebtThreshold(_borrowETHTotal, _unweightedCollateralETH)
        ? totalSharesUser
        : totalSharesUser * MAX_LIQUIDATION_50 / PRECISION_FACTOR_E18;

    if (_shareAmountToPay <= maxShares) {
        return;
    }

    //@audit-issue => reverts if the amount of shares to be repaid exceeds maxShares!
    revert TooManyShares();
}

For example, if there is a position in a liquidable state that has 100 borrowShares on the PoolA, and, a liquidator decides to liquidate the maximum possible amount of shares from this position it will send a tx to liquidate 50 shares from that position on the PoolA. The position's owner can use the WiseLending.paybackExactShares() function to repay 1 share and frontrun the liquidator's tx. Now, when the liquidator's tx is executed, the position is still liquidable, but, it only has 99 borrowShares on the PoolA, as a result of this, the WiseSecurityHelper.checkMaxShares() function will determine that the maximum possible amount of liquidable shares is 49.5, and, because the liquidator specified that he intended to liquidate 50 shares, the tx will be reverted.

Tools Used

Manual Audit

Recommended Mitigation Steps

In the WiseSecurityHelper.checkMaxShares() function, if the _shareAmountToPay exceeds maxShares, don't revert, re-adjust the number of shares that can be liquidated. Return the final value of _shareAmountToPay and forward it back to the WiseLending.liquidatePartiallyFromTokens() function. Then, use the final value of shareAmountToPay to compute the exact amount of tokens to be repaid in the WiseLending.liquidatePartiallyFromTokens() function.

WiseSecurity.sol


function checksLiquidation(
...
uint256 _shareAmountToPay
)
external
view
+   returns (uint256)
{
...

WiseSecurityHelper.sol


function checkMaxShares(
...
uint256 _shareAmountToPay
)
public
view
+   returns (uint256)    
{
...
uint256 maxShares = checkBadDebtThreshold(_borrowETHTotal, _unweightedCollateralETH)
        ? totalSharesUser
        : totalSharesUser * MAX_LIQUIDATION_50 / PRECISION_FACTOR_E18;

if (_shareAmountToPay <= maxShares) {

> WiseLending.sol

function liquidatePartiallyFromTokens( ... uint256 _shareAmountToPay ) ... { ...

Assessed type

Context

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as primary issue

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as sufficient quality report

vonMangoldt commented 5 months ago

Instead of wasting gas by frontrunning a newbie liquidator they could also make money and liquidate themselves. On Arbitrum its not possible at all. And if liquidators dont use tools like flashbots or eden its their own fault. No reason to do anything here imo. Dismissed

trust1995 commented 5 months ago

Whether the option for liquidators to bypass hacks through private mempools is grounds for reducing severity of their abuse is not a trivial question and would best be standardized in the Supreme Court.

I'm taking a stance that this cheap hack is annoying enough for liquidators to be worth Medium severity. Note that if private mempool is in-scope, attacker can use it as well and insert the annoying TX at start of block.

c4-judge commented 5 months ago

trust1995 marked the issue as selected for report

thebrittfactor commented 3 months ago

For transparency and per conversation with the sponsors, see here for the Wise Lending team's mitigation.

vm06007 commented 3 months ago

Mentioned issue does not seems to be in interest of the borrower, since borrower is more incentivized to perform self-liquidation to earn than preventing others, which becomes a race on who will liquidate first making liquidation a more desired outcome for both parties, no intention to gate the minimum payback threshold by admin.