howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/SemiFungiblePositionManager.sol#L542

Vulnerability details

Sellers will lose premia collected on their positions and buyers can avoid paying premia on their positions by transferring the position to another account. This is because the registerTokenTransfer() function in SFPM does not update the premia accumulators.

function safeTransferFrom(
        address from,
        address to,
        uint256 id,
        uint256 amount,
        bytes calldata data
    ) public override {
        // we don't need to reentrancy lock on transfers, but we can't allow transfers for a pool during mint/burn with a reentrant call
        // so just check if there is an active reentrancy lock for the relevant pool on the token we're transferring
        if (s_poolContext[TokenId.wrap(id).poolId()].locked) revert Errors.ReentrantCall();

        // update the position data
        registerTokenTransfer(from, to, TokenId.wrap(id), amount);

        // transfer the token (note that all state updates are completed before reentrancy is possible through onReceived callbacks)
        super.safeTransferFrom(from, to, id, amount, data);
    }

function registerTokenTransfer(address from, address to, TokenId id, uint256 amount) internal {
        id.validate();

        // Extract univ3pool from the poolId map to Uniswap Pool
        IUniswapV3Pool univ3pool = s_poolContext[id.poolId()].pool;

        uint256 numLegs = id.countLegs();
        for (uint256 leg = 0; leg < numLegs; ) {
            LiquidityChunk liquidityChunk = PanopticMath.getLiquidityChunk(
                id,
                leg,
                uint128(amount)
            );

            // construct the positionKey for the from and to addresses
            bytes32 positionKey_from = keccak256(
                abi.encodePacked(
                    address(univ3pool),
                    from,
                    id.tokenType(leg),
                    liquidityChunk.tickLower(),
                    liquidityChunk.tickUpper()
                )
            );
            bytes32 positionKey_to = keccak256(
                abi.encodePacked(
                    address(univ3pool),
                    to,
                    id.tokenType(leg),
                    liquidityChunk.tickLower(),
                    liquidityChunk.tickUpper()
                )
            );

            // Revert if recipient already has that position
            if (
                (LeftRightUnsigned.unwrap(s_accountLiquidity[positionKey_to]) != 0) ||
                (LeftRightSigned.unwrap(s_accountFeesBase[positionKey_to]) != 0)
            ) revert Errors.TransferFailed();

            // Revert if sender has long positions in that chunk or the entire liquidity is not being transferred
            LeftRightUnsigned fromLiq = s_accountLiquidity[positionKey_from];
            if (LeftRightUnsigned.unwrap(fromLiq) != liquidityChunk.liquidity())
                revert Errors.TransferFailed();

            LeftRightSigned fromBase = s_accountFeesBase[positionKey_from];

            // update+store liquidity and fee values between accounts
            s_accountLiquidity[positionKey_to] = fromLiq;
            s_accountLiquidity[positionKey_from] = LeftRightUnsigned.wrap(0);

            s_accountFeesBase[positionKey_to] = fromBase;
            s_accountFeesBase[positionKey_from] = LeftRightSigned.wrap(0);

            unchecked {
                ++leg;
            }
        }
    }

The getAccountPremium() will consequently return inaccurate values when called, which will lead to incorrect premium calculation as the new holder of the position will start with a value of 0 for both of these accumulators.

Impact

Buyers can avoid paying premia by transferring the position to a new account, while sellers will lose premia by transferring the position to a different account.

Proof of Concept

Tools Used

Recommended Mitigation Steps

Update the accumulators for premia in the registerTokenTransfer() function.

Assessed type

Other

c4-judge commented 1 month ago

Picodes marked the issue as duplicate of #40