sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

Nyx - Position owner can withdraw all his collateral without closing a position #149

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Nyx

high

Position owner can withdraw all his collateral without closing a position

Summary

After opening a position, the position owner can withdraw all his collateral.

Vulnerability Detail

Both LPs (makers) and traders (takers) need to provide collateral to participate in Perennial markets. But after opening a position, they can withdraw their collateral without closing their position.

Impact

The Position owner can withdraw his collateral so he can have a riskless position.

Code Snippet

liquidate.test.ts

it('open - withdraw and close position', 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'))

    // Open position
    await product.connect(user).openMake(POSITION)

    //
    await chainlink.next()
    await chainlink.nextWithPriceModification(price => price.mul(10))
    await product.settle()
    await product.settleAccount(user.address)

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

    //Withdraw
    await expect(collateral.connect(user).withdrawTo(user.address, product.address, constants.MaxUint256))

    //Close Position
    await product.connect(user).closeMake(POSITION)
  })

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

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/product/Product.sol#L274-L355

Tool used

Manual Review

Recommendation

Position owners shouldn't be able to withdraw their collateral before closing their position.