_updateSettlementPostBurn() may not correctly reduce s_grossPremiumLast[chunkKey] #462

c4-bot-10 · 2024-04-22T15:23:01Z

Lines of code

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

Vulnerability details

s_grossPremiumLast[] definitions are as follows:

/// @dev Per-chunk last value that gives the aggregate amount of premium owed to all sellers when multiplied by the total amount of liquidity totalLiquidity
/// totalGrossPremium = totalLiquidity * (grossPremium(perLiquidityX64) - lastGrossPremium(perLiquidityX64)) / 2**64
/// Used to compute the denominator for the fraction of premium available to sellers to collect
/// LeftRight - right slot is token0, left slot is token1
mapping(bytes32 chunkKey => LeftRightUnsigned lastGrossPremium) internal s_grossPremiumLast;

a critical rule: if there is a change in totalLiquidity, we must recalculate this value.

when Pool.mintOptions(), s_grossPremiumLast[chunkKey] will increase.
step: mintOptions()->_mintInSFPMAndUpdateCollateral()->_updateSettlementPostMint()

    function _updateSettlementPostMint(
        TokenId tokenId,
        LeftRightUnsigned[4] memory collectedByLeg,
        uint128 positionSize
    ) internal {
...
            if (tokenId.isLong(leg) == 0) { 
                LiquidityChunk liquidityChunk = PanopticMath.getLiquidityChunk(
                    tokenId,
                    leg,
                    positionSize
                );
...
                uint256[2] memory grossCurrent;
                (grossCurrent[0], grossCurrent[1]) = SFPM.getAccountPremium(
                    address(s_univ3pool),
                    address(this),
                    tokenId.tokenType(leg),
                    liquidityChunk.tickLower(),
                    liquidityChunk.tickUpper(),
                    type(int24).max,
                    0
                );

                unchecked {
                    // L
                    LeftRightUnsigned grossPremiumLast = s_grossPremiumLast[chunkKey];
                    // R
                    uint256 positionLiquidity = liquidityChunk.liquidity();
                    // T (totalLiquidity is (T + R) after minting)
                    uint256 totalLiquidityBefore = totalLiquidity - positionLiquidity;

@>                  s_grossPremiumLast[chunkKey] = LeftRightUnsigned
                        .wrap(0)
                        .toRightSlot(
                            uint128(
                                (grossCurrent[0] *
                                    positionLiquidity +
                                    grossPremiumLast.rightSlot() *
                                    totalLiquidityBefore) / (totalLiquidity)
                            )
                        )
                        .toLeftSlot(
                            uint128(
                                (grossCurrent[1] *
                                    positionLiquidity +
                                    grossPremiumLast.leftSlot() *
                                    totalLiquidityBefore) / (totalLiquidity)
                            )
                        );
                }
            }
        }
    }

when Pool.burnOptions()，s_grossPremiumLast[chunkKey] will decrease

burnOptions()->_burnAndHandleExercise()->_updateSettlementPostBurn()

    function _updateSettlementPostBurn(
        address owner,
        TokenId tokenId,
        LeftRightUnsigned[4] memory collectedByLeg,
        uint128 positionSize,
        bool commitLongSettled
    ) internal returns (LeftRightSigned realizedPremia, LeftRightSigned[4] memory premiaByLeg) {
...
        uint256 numLegs = tokenId.countLegs();
        uint256[2][4] memory premiumAccumulatorsByLeg;

        // compute accumulated fees
        (premiaByLeg, premiumAccumulatorsByLeg) = _getPremia(
            tokenId,
            positionSize,
            owner,
            COMPUTE_ALL_PREMIA,
            type(int24).max
        );

        for (uint256 leg = 0; leg < numLegs; ) {
            LeftRightSigned legPremia = premiaByLeg[leg];

            bytes32 chunkKey = keccak256(
                abi.encodePacked(tokenId.strike(leg), tokenId.width(leg), tokenId.tokenType(leg))
            );

            // collected from Uniswap
            LeftRightUnsigned settledTokens = s_settledTokens[chunkKey].add(collectedByLeg[leg]);

@>          if (LeftRightSigned.unwrap(legPremia) != 0) {
                // (will be) paid by long legs
                if (tokenId.isLong(leg) == 1) {
...
                } else {
                    uint256 positionLiquidity = PanopticMath
                        .getLiquidityChunk(tokenId, leg, positionSize)
                        .liquidity();

                    // new totalLiquidity (total sold) = removedLiquidity + netLiquidity (T - R)
                    uint256 totalLiquidity = _getTotalLiquidity(tokenId, leg);
                    // T (totalLiquidity is (T - R) after burning)
                    uint256 totalLiquidityBefore = totalLiquidity + positionLiquidity;

                    LeftRightUnsigned grossPremiumLast = s_grossPremiumLast[chunkKey];

                    LeftRightUnsigned availablePremium = _getAvailablePremium(
                        totalLiquidity + positionLiquidity,
                        settledTokens,
                        grossPremiumLast,
                        LeftRightUnsigned.wrap(uint256(LeftRightSigned.unwrap(legPremia))),
                        premiumAccumulatorsByLeg[leg]
                    );

                    // subtract settled tokens sent to seller
                    settledTokens = settledTokens.sub(availablePremium);

                    // add available premium to amount that should be settled
                    realizedPremia = realizedPremia.add(
                        LeftRightSigned.wrap(int256(LeftRightUnsigned.unwrap(availablePremium)))
                    );

...

                    unchecked {
                        uint256[2][4] memory _premiumAccumulatorsByLeg = premiumAccumulatorsByLeg;
                        uint256 _leg = leg;

                        // if there's still liquidity, compute the new grossPremiumLast
                        // otherwise, we just reset grossPremiumLast to the current grossPremium
@>                      s_grossPremiumLast[chunkKey] = totalLiquidity != 0
                            ? LeftRightUnsigned
                                .wrap(0)
                                .toRightSlot(
                                    uint128(
                                        uint256(
                                            Math.max(
                                                (int256(
                                                    grossPremiumLast.rightSlot() *
                                                        totalLiquidityBefore
                                                ) -
                                                    int256(
                                                        _premiumAccumulatorsByLeg[_leg][0] *
                                                            positionLiquidity
                                                    )) + int256(legPremia.rightSlot() * 2 ** 64),
                                                0
                                            )
                                        ) / totalLiquidity
                                    )
                                )
                                .toLeftSlot(
                                    uint128(
                                        uint256(
                                            Math.max(
                                                (int256(
                                                    grossPremiumLast.leftSlot() *
                                                        totalLiquidityBefore
                                                ) -
                                                    int256(
                                                        _premiumAccumulatorsByLeg[_leg][1] *
                                                            positionLiquidity
                                                    )) + int256(legPremia.leftSlot()) * 2 ** 64,
                                                0
                                            )
                                        ) / totalLiquidity
                                    )
                                )
                            : LeftRightUnsigned
                                .wrap(0)
                                .toRightSlot(uint128(premiumAccumulatorsByLeg[_leg][0]))
                                .toLeftSlot(uint128(premiumAccumulatorsByLeg[_leg][1]));
                    }
                }
            }

            // update settled tokens in storage with all local deltas
            s_settledTokens[chunkKey] = settledTokens;

            unchecked {
                ++leg;
            }

The issue lies within _updateSettlementPostBurn(), where it adds a condition that if (LeftRightSigned.unwrap(legPremia) != 0) must be met for s_grossPremiumLast[chunkKey] to decrease.

This results in not recalculating s_grossPremiumLast[chunkKey] even when totalLiquidity changes.

For example, in the same block, if a user executes mintOptions(), s_grossPremiumLast[chunkKey] increases by 50. Immediately after, executing burnOptions() doesn't decrease s_grossPremiumLast[chunkKey] by 50 because legPremia == 0.

Proof of Concept

The following code demonstrates this scenario, where s_grossPremiumLast[chunkKey] keeps increasing,so last gross accumulated keeps decreasing.

add to Misc.t.sol

    function test_burn_not_reduce_last() public {
        swapperc = new SwapperC();
        vm.startPrank(Swapper);
        token0.mint(Swapper, type(uint128).max);
        token1.mint(Swapper, type(uint128).max);
        token0.approve(address(swapperc), type(uint128).max);
        token1.approve(address(swapperc), type(uint128).max);

        // mint OTM position
        $posIdList.push(
            TokenId.wrap(0).addPoolId(PanopticMath.getPoolId(address(uniPool))).addLeg(
                0,
                1,
                1,
                0,
                0,
                0,
                15,
                1
            )
        );
        //@info Bob same gross
        vm.startPrank(Bob);
        pp.mintOptions($posIdList, 1_000_000, 0, 0, 0);
        vm.startPrank(Swapper);
        swapperc.swapTo(uniPool, Math.getSqrtRatioAtTick(10) + 1);
        // 1998600539
        accruePoolFeesInRange(address(uniPool), (uniPool.liquidity() * 2) / 3, 1, 1);
        swapperc.swapTo(uniPool, 2 ** 96); 
        //@info end of Bob same gross

        //@info start alice , it should without change gross
        console.log("*****start alice mint/burn should not reduce last accumulated");
        for(uint256 i=0;i<10;i++){
            vm.startPrank(Alice);
            pp.mintOptions($posIdList, 250_000, type(uint64).max, 0, 0);

            vm.startPrank(Alice);
            pp.burnOptions($posIdList[0], new TokenId[](0), 0, 0);
        } 
    }

add to PanopticPool.sol

    function _updateSettlementPostMint(
        TokenId tokenId,
        LeftRightUnsigned[4] memory collectedByLeg,
        uint128 positionSize
    ) internal {
...
               unchecked {
                    // L
                    LeftRightUnsigned grossPremiumLast = s_grossPremiumLast[chunkKey];
                    // R
                    uint256 positionLiquidity = liquidityChunk.liquidity();
                    // T (totalLiquidity is (T + R) after minting)
                    uint256 totalLiquidityBefore = totalLiquidity - positionLiquidity;
                    //console.log("s_grossPremiumLast[chunkKey].rightSlot() from:",s_grossPremiumLast[chunkKey].rightSlot());
                    s_grossPremiumLast[chunkKey] = LeftRightUnsigned
                        .wrap(0)
                        .toRightSlot(
                            uint128(
                                (grossCurrent[0] *
                                    positionLiquidity +
                                    grossPremiumLast.rightSlot() *
                                    totalLiquidityBefore) / (totalLiquidity)
                            )
                        )
                        .toLeftSlot(
                            uint128(
                                (grossCurrent[1] *
                                    positionLiquidity +
                                    grossPremiumLast.leftSlot() *
                                    totalLiquidityBefore) / (totalLiquidity)
                            )
                        );
+                       console.log("last accumulated : ",(grossCurrent[0] - s_grossPremiumLast[chunkKey].rightSlot()) * totalLiquidity);
                }
            }
        }
    }

$ forge test -vvv --match-test test_burn_not_reduce_last

[PASS] test_burn_not_reduce_last() (gas: 5276055)
Logs:
  last accumulated :  0
  *****start alice mint/burn should not reduce last accumulated
  last accumulated :  36893488148466911062
  last accumulated :  29514790528266881407
  last accumulated :  23611832432106857683
  last accumulated :  18889465953679888300
  last accumulated :  15111572767940411986
  last accumulated :  12089258220348131204
  last accumulated :  9671406580275706040
  last accumulated :  7737125266718815505
  last accumulated :  6189700215873303077
  last accumulated :  4951760174697243000

Impact

Due to the incorrect accounting of s_grossPremiumLast[chunkKey], the calculation in _getAvailablePremium() is also incorrect. This results in inaccuracies in the amount of premium received by the seller when closing their position.

Recommended Mitigation

Regardless of the value of legPremia, it should recalculate s_grossPremiumLast[chunkKey] when long == 0.

    function _updateSettlementPostBurn(
        address owner,
        TokenId tokenId,
        LeftRightUnsigned[4] memory collectedByLeg,
        uint128 positionSize,
        bool commitLongSettled
    ) internal returns (LeftRightSigned realizedPremia, LeftRightSigned[4] memory premiaByLeg) {
...
        for (uint256 leg = 0; leg < numLegs; ) {
            LeftRightSigned legPremia = premiaByLeg[leg];

            bytes32 chunkKey = keccak256(
                abi.encodePacked(tokenId.strike(leg), tokenId.width(leg), tokenId.tokenType(leg))
            );

            // collected from Uniswap
            LeftRightUnsigned settledTokens = s_settledTokens[chunkKey].add(collectedByLeg[leg]);

            if (LeftRightSigned.unwrap(legPremia) != 0) {
                // (will be) paid by long legs
                if (tokenId.isLong(leg) == 1) {
...
                } else {
....

                    // subtract settled tokens sent to seller
                    settledTokens = settledTokens.sub(availablePremium);

                    // add available premium to amount that should be settled
                    realizedPremia = realizedPremia.add(
                        LeftRightSigned.wrap(int256(LeftRightUnsigned.unwrap(availablePremium)))
                    );


-                   unchecked {
-                       uint256[2][4] memory _premiumAccumulatorsByLeg = premiumAccumulatorsByLeg;-
-                        uint256 _leg = leg;
-
-                        // if there's still liquidity, compute the new grossPremiumLast
-                        // otherwise, we just reset grossPremiumLast to the current grossPremium
-                        s_grossPremiumLast[chunkKey] = totalLiquidity != 0
-                            ? LeftRightUnsigned
-                                .wrap(0)
-                                .toRightSlot(
-                                    uint128(
-                                        uint256(
-                                            Math.max(
-                                                (int256(
-                                                    grossPremiumLast.rightSlot() *
-                                                        totalLiquidityBefore
-                                                ) -
-                                                    int256(
-                                                        _premiumAccumulatorsByLeg[_leg][0] *
-                                                            positionLiquidity
-                                                    )) + int256(legPremia.rightSlot() * 2 ** 64),
-                                                0
-                                            )
-                                        ) / totalLiquidity
-                                    )
-                                )
-                                .toLeftSlot(
-                                    uint128(
-                                        uint256(
-                                            Math.max(
-                                                (int256(
-                                                    grossPremiumLast.leftSlot() *
-                                                        totalLiquidityBefore
-                                                ) -
-                                                    int256(
-                                                        _premiumAccumulatorsByLeg[_leg][1] *
-                                                            positionLiquidity
-                                                    )) + int256(legPremia.leftSlot()) * 2 ** 64,
-                                                0
-                                            )
-                                        ) / totalLiquidity
-                                    )
-                                )
-                            : LeftRightUnsigned
-                                .wrap(0)
-                                .toRightSlot(uint128(premiumAccumulatorsByLeg[_leg][0]))
-                                .toLeftSlot(uint128(premiumAccumulatorsByLeg[_leg][1]));
-                       }
                   }
               }

+             if (tokenId.isLong(leg) == 0){
+                   uint256 positionLiquidity = PanopticMath
+                   .getLiquidityChunk(tokenId, leg, positionSize)
+                    .liquidity();
+
+                    // new totalLiquidity (total sold) = removedLiquidity + netLiquidity (T - R)
+                    uint256 totalLiquidity = _getTotalLiquidity(tokenId, leg);
+                    // T (totalLiquidity is (T - R) after burning)
+                   uint256 totalLiquidityBefore = totalLiquidity + positionLiquidity;
+
+                    LeftRightUnsigned grossPremiumLast = s_grossPremiumLast[chunkKey];                
+                    unchecked {
+                        uint256[2][4] memory _premiumAccumulatorsByLeg = premiumAccumulatorsByLeg;
+                        uint256 _leg = leg;
+
+                        // if there's still liquidity, compute the new grossPremiumLast
+                        // otherwise, we just reset grossPremiumLast to the current grossPremium                        
+                        s_grossPremiumLast[chunkKey] = totalLiquidity != 0
+                            ? LeftRightUnsigned
+                                .wrap(0)
+                                .toRightSlot(
+                                    uint128(
+                                        uint256(
+                                            Math.max(
+                                                (int256(
+                                                    grossPremiumLast.rightSlot() *
+                                                        totalLiquidityBefore
+                                                ) -
+                                                    int256(
+                                                        _premiumAccumulatorsByLeg[_leg][0] *
+                                                            positionLiquidity
+                                                    )) + int256(legPremia.rightSlot() * 2 ** 64),
+                                                0
+                                            )
+                                        ) / totalLiquidity
+                                    )
+                                )
+                                .toLeftSlot(
+                                    uint128(
+                                       uint256(
+                                            Math.max(
+                                                (int256(
+                                                    grossPremiumLast.leftSlot() *
+                                                        totalLiquidityBefore
+                                                ) -
+                                                    int256(
+                                                        _premiumAccumulatorsByLeg[_leg][1] *
+                                                            positionLiquidity
+                                                    )) + int256(legPremia.leftSlot()) * 2 ** 64,
+                                                0
+                                            )
+                                        ) / totalLiquidity
+                                    )
+                                )
+                            : LeftRightUnsigned
+                                .wrap(0)
+                                .toRightSlot(uint128(premiumAccumulatorsByLeg[_leg][0]))
+                                .toLeftSlot(uint128(premiumAccumulatorsByLeg[_leg][1]));                             
+                    }
+                }

            }

            // update settled tokens in storage with all local deltas
            s_settledTokens[chunkKey] = settledTokens;

            unchecked {
                ++leg;
            }
        }
    }
}

Assessed type

Context

The text was updated successfully, but these errors were encountered:

c4-judge · 2024-05-06T15:52:56Z

Picodes marked the issue as satisfactory

c4-judge · 2024-05-06T15:52:59Z

Picodes marked the issue as selected for report

c4-bot-10 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Apr 22, 2024

c4-bot-2 added a commit that referenced this issue Apr 22, 2024

bin2chen issue #462

fdc5ed9

dyedm1 added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Apr 26, 2024

c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label May 6, 2024

c4-judge added the selected for report This submission will be included/highlighted in the audit report label May 6, 2024

C4-Staff added the M-06 label May 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

_updateSettlementPostBurn() may not correctly reduce s_grossPremiumLast[chunkKey] #462

_updateSettlementPostBurn() may not correctly reduce s_grossPremiumLast[chunkKey] #462

c4-bot-10 commented Apr 22, 2024

c4-judge commented May 6, 2024

c4-judge commented May 6, 2024

_updateSettlementPostBurn() may not correctly reduce s_grossPremiumLast[chunkKey] #462

_updateSettlementPostBurn() may not correctly reduce s_grossPremiumLast[chunkKey] #462

Comments

c4-bot-10 commented Apr 22, 2024

Lines of code

Vulnerability details

Vulnerability details

Proof of Concept

Impact

Recommended Mitigation

Assessed type

c4-judge commented May 6, 2024

c4-judge commented May 6, 2024