Pool is not initialized after deployment

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/TimeswapV2Pool.sol#L175-L181 https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/structs/Pool.sol#L205 https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-pool/src/structs/Pool.sol#L294-L366

Vulnerability details

Impact

In TimeswapPool each maturity & strike combination for a given Option has to be initialized by assigning it a rate. According to the whitepaper, the first person to add liquidity to the pool is supposed to specify the rate, see "3.2 Timeswap Pool". But, with the current implementation, there's no guarantee that a pool is ever initialized. Neither is it guaranteed that the first LP is assigning the initial rate. The pool can be re-initialized as long as there is no liquidity provided yet. So it's possible that someone initializes the pool with rate x and calls mint() after that. A bot can frontrun the mint() call and set the interest rate to y. Since the interest rate is part of the calculation to determine the LP's minted position this will cause unexpected behavior for the user and potentially a loss of funds.

Proof of Concept

Every pool can be initialized as long as there is no liquidity deposited yet:

    // TimeswapV2Pool
    function initialize(uint256 strike, uint256 maturity, uint160 rate) external override noDelegateCall {
        if (maturity < blockTimestamp(0)) Error.alreadyMatured(maturity, blockTimestamp(0));
        if (rate == 0) Error.cannotBeZero();
        addPoolEnumerationIfNecessary(strike, maturity);

        pools[strike][maturity].initialize(rate);
    }

    // Pool library:
    function initialize(Pool storage pool, uint160 rate) external {
        if (pool.liquidity != 0) Error.alreadyHaveLiquidity(pool.liquidity);
        pool.sqrtInterestRate = rate;
    }

If you call mint() without initializing the pool first, the ConstantProduct library will use 0 for the sqrtInterestRate value. That in turn will either cause the function to revert because you try to divide by zero or because the calculated result will be 0:

// Pool library:
    function mint(
        Pool storage pool,
        TimeswapV2PoolMintParam memory param,
        uint96 blockTimestamp
    ) external returns (uint160 liquidityAmount, uint256 long0Amount, uint256 long1Amount, uint256 shortAmount, bytes memory data) {
        // Update the state of the pool first for the short fee growth.
        if (pool.liquidity != 0) updateDurationWeightBeforeMaturity(pool, blockTimestamp);
        // Update the fee growth and fees of caller.
        LiquidityPosition storage liquidityPosition = pool.liquidityPositions[param.to];

        uint256 longAmount;
        if (param.transaction == TimeswapV2PoolMint.GivenLiquidity) {
            (longAmount, shortAmount) = ConstantProduct.calculateGivenLiquidityDelta(
                pool.sqrtInterestRate,
                liquidityAmount = param.delta.toUint160(),
                DurationCalculation.unsafeDurationFromNowToMaturity(param.maturity, blockTimestamp),
                true
            );

            if (longAmount == 0) Error.zeroOutput();
            if (shortAmount == 0) Error.zeroOutput();
        } else if (param.transaction == TimeswapV2PoolMint.GivenLong) {
            (liquidityAmount, shortAmount) = ConstantProduct.calculateGivenLiquidityLong(
                pool.sqrtInterestRate,
                longAmount = param.delta,
                DurationCalculation.unsafeDurationFromNowToMaturity(param.maturity, blockTimestamp),
                true
            );

            if (liquidityAmount == 0) Error.zeroOutput();
            if (shortAmount == 0) Error.zeroOutput();
        } else if (param.transaction == TimeswapV2PoolMint.GivenShort) {
            (liquidityAmount, longAmount) = ConstantProduct.calculateGivenLiquidityShort(
                pool.sqrtInterestRate,
                shortAmount = param.delta,
                DurationCalculation.unsafeDurationFromNowToMaturity(param.maturity, blockTimestamp),
                true
            );

            if (liquidityAmount == 0) Error.zeroOutput();
            if (longAmount == 0) Error.zeroOutput();
        } else if (param.transaction == TimeswapV2PoolMint.GivenLarger) {
            (liquidityAmount, longAmount, shortAmount) = ConstantProduct.calculateGivenLiquidityLargerOrSmaller(
                pool.sqrtInterestRate,
                param.delta,
                DurationCalculation.unsafeDurationFromNowToMaturity(param.maturity, blockTimestamp),
                true
            );

            if (liquidityAmount == 0) Error.zeroOutput();
            if (longAmount == 0) Error.zeroOutput();
            if (shortAmount == 0) Error.zeroOutput();
        }
      // ...

If you search for initialize() you will see that there's no instance of a pool being initialized within any of the contracts. That means it has to be executed externally. Unless the caller uses a multicall to initialize & mint within the same tx, a bot can frontrun their tx to re-initialize the pool before their mint() tx is executed.

Tools Used

none

Recommended Mitigation Steps

There are two issues:

• pool can be re-initialized if no liquidity was deposited -> enables the user to be frontrun
• there's no guarantee that a pool is initialized before a user interacts with it -> causes the user's tx to revert.

To fix both you can force the first minter to provide the sqrtInterestRate parameter within the tx's calldata. Subsequent callers can then just ignore the parameter.

[c4-judge]

Picodes marked the issue as duplicate of #270

[c4-judge]

Picodes changed the severity to 3 (High Risk)

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as grade-b