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

7 stars 3 forks source link

Unsafe casting of `int256` to `uint256` can prompt the `s_settledTokens` mapping into a broken state #524

Closed c4-bot-5 closed 4 months ago

c4-bot-5 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1650-L1652 https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L473 https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1892

Vulnerability details

Impact

The PanopticPool.settleLongPremium function is used to settle all unpaid premium for long legs on tokenIds of owners. In the function exeuction of the PanopticPool.settleLongPremium, the realizedPremia is deducted from the owner's balance and then added to the cumulative settled token delta as shown below:

            s_collateralToken0.exercise(owner, 0, 0, 0, realizedPremia.rightSlot());
            s_collateralToken1.exercise(owner, 0, 0, 0, realizedPremia.leftSlot());

            ...
            ...

            s_settledTokens[chunkKey] = s_settledTokens[chunkKey].add(
                LeftRightUnsigned.wrap(uint256(LeftRightSigned.unwrap(realizedPremia)))
            );

But the issue here is that the LeftRightSigned.unwrap(realizedPremia) value which is a int256 is converted into a uint256 unsafely and then added onto the s_settledTokens[chunkKey] storage. Since the int256 holds negative values in two's complement format, unsafe casting of a negative LeftRightSigned.unwrap(realizedPremia) value to a uint256 will result in a large positive value.

Hence this will add a large value to the s_settledTokens[chunkKey] (where as it should reduce by that value since the value is negative) thus breaking the state of the s_settledTokens mapping. As a result the amount of tokens owed to sellers that have been settled for that specific chunk is now errorneous.

Since the s_settledTokens[chunkKey] storage value is broken, the critical functions such as _calculateAccumulatedPremia, liquidate, _updateSettlementPostMint and _updateSettlementPostBurn will also prompt into undesirable state. Since this errorneous s_settledTokens value is used in the _calculateAccumulatedPremia function to calculate the portfolioPremium value, and this value is used as the premia value to calculate the margin requirement in the PanopticPool.liquidate function by calling the CollateralTracker.getAccountMarginDetails, this could result in errorneous values for balanceCross and thresholdCross. Due to these errorneous values the positions which are not liquidatable could appear as liquidatable positions and vice versa. If positions which are liqudatable can not be liquidated as a result of the above vulnerability this could prompt the Panoptic protocol into insolvency.

Similar issue of errorneous casting of LeftRightSigned.unwrap(realizedPremia) to a uint256 are found in the PanopticPool._calculateAccumulatedPremia and PanopticPool._updateSettlementPostBurn functions as well.

Proof of Concept

            s_settledTokens[chunkKey] = s_settledTokens[chunkKey].add(
                LeftRightUnsigned.wrap(uint256(LeftRightSigned.unwrap(realizedPremia)))
            );

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1650-L1652

    LeftRightUnsigned.wrap(uint256(LeftRightSigned.unwrap(premiaByLeg[leg]))),

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L473

    LeftRightUnsigned.wrap(uint256(LeftRightSigned.unwrap(legPremia))),

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1892

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to conduct safe casting of LeftRightSigned.unwrap(realizedPremia) value of int256 to a uint256. Hence if the LeftRightSigned.unwrap(realizedPremia) value is negative then it can be converted into a positive value before adding onto the s_settledTokens[chunkKey] state.

Assessed type

Other

c4-judge commented 4 months ago

Picodes marked the issue as primary issue

dyedm1 commented 4 months ago

realizedPremia may be stored in a signed container, but it cannot actually become negative in any situation

c4-judge commented 4 months ago

Picodes marked the issue as unsatisfactory: Insufficient proof