sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

Nyx - Self Liquidation #142

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Nyx

high

Self Liquidation

Summary

The borrower can liquidate himself.

Vulnerability Detail

function liquidate(address account, IProduct product)
    external
    nonReentrant
    notPaused
    isProduct(product)
    settleForAccount(account, product)
    {
        if (product.isLiquidating(account)) revert CollateralAccountLiquidatingError(account);

        UFixed18 totalMaintenance = product.maintenance(account);
        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());
        UFixed18 fee = UFixed18Lib.min(totalCollateral, collateralForFee.mul(liquidationFee));

        _products[product].debitAccount(account, fee);
        token.push(msg.sender, fee);

        emit Liquidation(account, product, msg.sender, fee);
    }

There is no check in liquidate() function that prevents self-liquidation.

Users can self-liquidate their positions when they are in danger or front-run other liquidators and prevent liquidators from taking their collaterals.

Impact

Liquidators cant liquidate any position.

Code Snippet

liquidate.test.ts

it('self liquidation test', async () => {
    //@audit test
    const POSITION = utils.parseEther('0.0001')
    const { user, userB, collateral, dsu, chainlink } = instanceVars

    const product = await createProduct(instanceVars)
    await depositTo(instanceVars, user, product, utils.parseEther('1000'))
    await product.connect(user).openMake(POSITION)
    //await expect(collateral.connect(user).withdrawTo(user.address, product.address, constants.MaxUint256))
    //await product.connect(user).closeMake(POSITION)
    expect(await collateral.liquidatable(user.address, product.address)).to.be.false

    // Settle the product with a new oracle version
    await chainlink.nextWithPriceModification(price => price.mul(10))
    await product.settle()

    await product.settleAccount(user.address)

    expect(await collateral.liquidatableNext(user.address, product.address)).to.be.true
    expect(await collateral.liquidatable(user.address, product.address)).to.be.true

    await expect(collateral.connect(user).liquidate(user.address, product.address))
  })

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/collateral/Collateral.sol#L102-L135

Tool used

Manual Review

Recommendation

if (msg.sender == account) revert();