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

9 stars 4 forks source link

Incorrect Premium Calculation for Non-Long Legs in `_calculateAccumulatedPremia` Function #539

Closed c4-bot-5 closed 7 months ago

c4-bot-5 commented 7 months ago

Lines of code

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

Vulnerability details

Impact

The _calculateAccumulatedPremia function calculates the accumulated premia for a user's option positions. However, when handling short positions and excluding pending premium, the function incorrectly converts the signed premia to an unsigned value before passing it to _getAvailablePremium. This incorrect conversion can lead to logical errors in the calculation of available premium, potentially affecting the accuracy of the user's financial position.

The incorrect premium calculation in _calculateAccumulatedPremia affects the call to _getAvailablePremium function, which is responsible for determining the available premium for settlement. Incorrect premium values passed to this function can result in incorrect premium settlements and Inaccurate premium values can distort the valuation of user portfolios.

Proof of Concept

The issue is in the _calculateAccumulatedPremia function where it processes each leg of the option position. Specifically, when a leg is not long and includePendingPremium is false, the function attempts to calculate the available premium:

// Incorrect premium calculation for non-long legs
if (tokenId.isLong(leg) == 0 && !includePendingPremium) {
    bytes32 chunkKey = keccak256(
        abi.encodePacked(
            tokenId.strike(leg),
            tokenId.width(leg),
            tokenId.tokenType(leg)
        )
    );

    // Incorrect argument passed for premium calculation
    LeftRightUnsigned availablePremium = _getAvailablePremium(
        _getTotalLiquidity(tokenId, leg),
        s_settledTokens[chunkKey],
        s_grossPremiumLast[chunkKey],
        LeftRightUnsigned.wrap(uint256(LeftRightSigned.unwrap(premiaByLeg[leg]))), // Incorrect argument here
        premiumAccumulatorsByLeg[leg]
    );
    portfolioPremium = portfolioPremium.add(
        LeftRightSigned.wrap(int256(LeftRightUnsigned.unwrap(availablePremium)))
    );
}

Here, LeftRightUnsigned.wrap(uint256(LeftRightSigned.unwrap(premiaByLeg[leg]))) is used to convert the signed premia (premiaByLeg[leg]) to an unsigned value. This conversion is unnecessary and could lead to incorrect results due to the way signed and unsigned integers are handled in Solidity. The unwrap function is used to extract the signed value from a LeftRightSigned object, and then it's immediately converted to an unsigned integer and wrapped back into a LeftRightUnsigned object. This conversion does not change the sign of the value but rather its representation.

Tools Used

Manual

Recommended Mitigation Steps

To address this issue, ensure that the correct value is passed for premium calculation for non-long legs in the _calculateAccumulatedPremia function. Update the argument passed to _getAvailablePremium to use premiumAccumulatorsByLeg[leg] instead of premiaByLeg[leg] to accurately calculate the available premium for non-long legs when includePendingPremium is false. This correction will help maintain the integrity of premium calculations and financial operations within the PanopticPool contract.

Consider reviewing the Conversion Logic to ensure that the conversion from signed to unsigned premia is done in a way that accurately reflects the premia's value. If _getAvailablePremium expects an unsigned value for premia, ensure that the function can correctly handle signed values by converting them to unsigned in a way that preserves the premia's magnitude and sign.

Assessed type

Math

dyedm1 commented 7 months ago

Short leg premium must be a natural number (only long legs have negative premium), thus, this conversion is safe in all cases.

c4-judge commented 7 months ago

Picodes marked the issue as unsatisfactory: Invalid