sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

simon135 - Some positions wont be able to be liquidated becuase of mistake in the code #221

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

simon135

medium

Some positions wont be able to be liquidated becuase of mistake in the code

Summary

we open a position and our position goes wrong from 10,000 to 5000 maintenance is now 2000 and we only have 50 collateral left since the bad funding rate/liquidity change. Now when we try to liquidate it will revert because maintenance * liquidationFee > totalCollateral will revert since we don't have that much collateral left after fees.

Vulnerability Detail

steps:

  1. Long position with 2,000 maintenance and 5x leverage and 2,000 collateral
  2. eth falls 25% down and so your P&L is now in a huge loss so let's say it takes 95 percent of collateral which would be 100 collateral left
  3. it tries to get liquidated but since the liquidation fee is 100 (2000 * 0.05) and the total collateral left is 100 and when we close the taker Position as long as the fee > 0 the debitAccount will revert since 99-100 will revert

    Impact

    liquidations won't be able to happen with this type of swing in price. It's unlikely but possible so it's a medium risk

    Code Snippet

    UFixed18 totalMaintenance = product.maintenance(account);
    //@audit Here we use this as how much we should pay to the liquidator but if there are fees on closing the position then its 
    // `totalCollateral-fees` in users' balance 
        UFixed18 totalCollateral = collateral(account, product);
    
        if (!totalMaintenance.gt(totalCollateral))
            revert CollateralCantLiquidate(totalMaintenance, totalCollateral);
    
        product.closeAll(account);
    
        // claim fee
        UFixed18 liquidationFee = controller().liquidationFee();
        // If maintenance is less than minCollateral, use minCollateral for fee amount
        UFixed18 collateralForFee = UFixed18Lib.max(totalMaintenance, controller().minCollateral());
    // @audit When we compute this we still use the balance that is not correct //with what the liquidator can get from the user
        UFixed18 fee = UFixed18Lib.min(totalCollateral, collateralForFee.mul(liquidationFee));
    // @audit it will revert since totalCollateral>user.balance
        _products[product].debitAccount(account, fee);

    Tool used

Manual Review

Recommendation

set totalCollateral =user.balance after closing the positions instead oftotalCollateral` that is not updated before that call

Duplicate of #132