QuickSwap and StellaSwap contest - cccz's results

Lines of code

https://github.com/code-423n4/2022-09-quickswap/blob/15ea643c85ed936a92d2676a7aabf739b210af39/src/core/contracts/AlgebraPool.sol#L226-L231

Vulnerability details

Impact

In the AlgebraPool contract, when the user provides liquidity via the mint function, the lastLiquidityAddTimestamp is updated to the current time.

      (_position.liquidity, _position.lastLiquidityAddTimestamp) = (
        liquidityNext,
        liquidityNext > 0 ? (liquidityDelta > 0 ? _blockTimestamp() : lastLiquidityAddTimestamp) : 0
      );

Later when the user removes the liquidity via burn function, the transaction will revert if the current time is less than lastLiquidityAddTimestam + _liquidityCooldown.

      if (liquidityDelta < 0) {
        uint32 _liquidityCooldown = liquidityCooldown;
        if (_liquidityCooldown > 0) {
          require((_blockTimestamp() - lastLiquidityAddTimestamp) >= _liquidityCooldown);
        }
      }

liquidityCooldown is max 1 day. However, in the mint function, users can provide liquidity on behalf of other users, which also means that malicious users can keep other users on liquidity cooldown forever by providing a little bit of liquidity on behalf of other users, thus preventing other users from removing liquidity.

  function mint(
    address sender,
    address recipient,  // @audit: users can provide liquidity on behalf of other users
    int24 bottomTick,
    int24 topTick,
    uint128 liquidityDesired,
    bytes calldata data
  )
...
      (, int256 amount0Int, int256 amount1Int) = _updatePositionTicksAndFees(recipient, bottomTick, topTick, int256(liquidityActual).toInt128());

Proof of Concept

https://github.com/code-423n4/2022-09-quickswap/blob/15ea643c85ed936a92d2676a7aabf739b210af39/src/core/contracts/AlgebraPool.sol#L226-L231 https://github.com/code-423n4/2022-09-quickswap/blob/15ea643c85ed936a92d2676a7aabf739b210af39/src/core/contracts/AlgebraPool.sol#L513-L523

Tools Used

None

Recommended Mitigation Steps

Consider allowing users to provide liquidity only for themselves, or setting liquidityCooldown to 0

Lines of code

https://github.com/code-423n4/2022-09-quickswap/blob/15ea643c85ed936a92d2676a7aabf739b210af39/src/core/contracts/AlgebraPool.sol#L920-L946

Vulnerability details

Impact

Some tokens (e.g. LEND) revert when transfering a zero value amount.

https://github.com/d-xo/weird-erc20#revert-on-zero-value-transfers

AlgebraPool.flash() might revert when fees0 = 0 or fees1 = 0 and asset is a revert on zero value transfers token.

And during the fees0/fees1 calculation, it's possible fees0/fees1 = 0 when communityFeeToken0/communityFeeToken1 == 0 or paid0/paid1 is small.

    uint256 paid0 = balanceToken0();
    require(balance0Before.add(fee0) <= paid0, 'F0');
    paid0 -= balance0Before;

    if (paid0 > 0) {
      uint8 _communityFeeToken0 = globalState.communityFeeToken0;
      uint256 fees0;
      if (_communityFeeToken0 > 0) {
        fees0 = (paid0 * _communityFeeToken0) / Constants.COMMUNITY_FEE_DENOMINATOR;
        TransferHelper.safeTransfer(token0, vault, fees0);
      }
      totalFeeGrowth0Token += FullMath.mulDiv(paid0 - fees0, Constants.Q128, _liquidity);
    }

    uint256 paid1 = balanceToken1();
    require(balance1Before.add(fee1) <= paid1, 'F1');
    paid1 -= balance1Before;

    if (paid1 > 0) {
      uint8 _communityFeeToken1 = globalState.communityFeeToken1;
      uint256 fees1;
      if (_communityFeeToken1 > 0) {
        fees1 = (paid1 * _communityFeeToken1) / Constants.COMMUNITY_FEE_DENOMINATOR;
        TransferHelper.safeTransfer(token1, vault, fees1);
      }
      totalFeeGrowth1Token += FullMath.mulDiv(paid1 - fees1, Constants.Q128, _liquidity);
    }

Proof of Concept

https://github.com/code-423n4/2022-09-quickswap/blob/15ea643c85ed936a92d2676a7aabf739b210af39/src/core/contracts/AlgebraPool.sol#L920-L946

Tools Used

None

Recommended Mitigation Steps

We should check if fees0 != 0 and fees1 != 0 before transfer.