code-423n4 / 2024-04-panoptic-findings

7 stars 3 forks source link

Accounts with a large amount of position can become unliquidable, resulting in bad debt #521

Closed c4-bot-9 closed 4 months ago

c4-bot-9 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L547 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1017

Vulnerability details

Impact

An account can become unliquidatable if it holds a large number of positions, as there is no maximum cap. Users can intentionally make their accounts unliquidatable in this manner, resulting in bad debt for the protocol.

Proof of Concept

Run export FOUNDRY_PROFILE=ci_test && forge test --match-test test_success_liquidate --gas-report:

    | test/foundry/core/PanopticPool.t.sol:PanopticPoolHarness contract |                 |         |         |         |         |
    |-------------------------------------------------------------------|-----------------|---------|---------|---------|---------|
    | Deployment Cost                                                   | Deployment Size |         |         |         |         |
    | 5221337                                                           | 24167           |         |         |         |         |
    | Function Name                                                     | min             | avg     | median  | max     | # calls |
    | burnAllOptionsFrom                                                | 482399          | 524409  | 524409  | 566420  | 2       |
    | calculateAccumulatedFeesBatch                                     | 52381           | 52381   | 52381   | 52381   | 6       |
    | collateralToken0                                                  | 553             | 553     | 553     | 553     | 4       |
    | collateralToken1                                                  | 432             | 432     | 432     | 432     | 4       |
    | getUniV3TWAP_                                                     | 333699          | 343199  | 333699  | 371699  | 8       |
@>  | liquidate                                                         | 1059873         | 1087657 | 1087657 | 1115442 | 2       |
@>  | mintOptions                                                       | 440870          | 555757  | 560100  | 755233  | 8       |
    | onERC1155Received                                                 | 1471            | 1471    | 1471    | 1471    | 8       |
    | settledTokens                                                     | 614             | 614     | 614     | 614     | 14      |
    | startPool                                                         | 219089          | 219089  | 219089  | 219089  | 2       |

mintOptions is cheaper than liquidate, so a user can continue to add positions to their account until liquidate runs out of gas. There isn't a hard cap on the number of positions that an account can have, so at one point, mintOptions will not run OOS, but liquidate will.

To execute liquidate, the liquidator must pass the entire positionIdList owned by the user, where a bunch of calculations occur to make sure that the account is being margin called. These calculations are more expensive than minting/burning a position, so a user can start accumulating low-risk positions until a point where calling liquidate will always revert due to OOS.

At this point, the user can start minting high-risk positions with zero risks of being liquidated as the function would revert. Note that options are perpetual (they don't have an expiry date, like classic options), so this scenario is basically guaranteed to happen at one point in time.

Tools Used

Manual review

Recommended Mitigation Steps

Consider implementing an hard cap on how many position an account can have open at a time.

Assessed type

DoS

c4-judge commented 4 months ago

Picodes marked the issue as primary issue

dyedm1 commented 4 months ago

"The liquidator may not be able to execute a liquidation if MAX_POSITIONS is too high for the deployed chain due to an insufficient gas limit. This parameter is not final and will be adjusted by deployed chain such that the most expensive liquidation is well within a safe margin of the gas limit." see contest readme

c4-judge commented 4 months ago

Picodes marked the issue as unsatisfactory: Out of scope

Picodes commented 4 months ago

See https://github.com/code-423n4/2024-04-panoptic/tree/833312ebd600665b577fbd9c03ffa0daf250ed24