Vanguard
First Flight #56
Beginner FriendlyDeFiFoundry
0 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: high

Phase Duration Off-by-One Inconsistency Between _beforeSwap and getCurrentPhase

Author Revealed upon completion

Description

  • The contract has two places where phase is calculated: _beforeSwap (for actual enforcement) and getCurrentPhase (view function for external queries).

  • These two functions use different comparison operators (<= vs <), causing a 1-block discrepancy in phase reporting.

// _beforeSwap uses <= (inclusive of boundary)
function _beforeSwap(...) internal override returns (...) {
uint256 blocksSinceLaunch = block.number - launchStartBlock;
uint256 newPhase;
if (blocksSinceLaunch <= phase1Duration) { // @> Uses <=
newPhase = 1;
} else if (blocksSinceLaunch <= phase1Duration + phase2Duration) { // @> Uses <=
newPhase = 2;
} else {
newPhase = 3;
}
}
​
// getCurrentPhase uses < (exclusive of boundary)
function getCurrentPhase() public view returns (uint256) {
if (launchStartBlock == 0) return 0;
uint256 blocksSinceLaunch = block.number - launchStartBlock;
if (blocksSinceLaunch < phase1Duration) { // @> Uses <
return 1;
} else if (blocksSinceLaunch < phase1Duration + phase2Duration) { // @> Uses <
return 2;
} else {
return 3;
}
}

Risk

Likelihood:

  • Occurs at every phase boundary block (e.g., block launchStartBlock + phase1Duration)

  • Users querying during these exact blocks will see incorrect phase

Impact:

  • getUserRemainingLimit() and getUserCooldownEnd() return values based on wrong phase

  • Frontend/UI displays incorrect phase information

  • Users may make decisions based on incorrect phase data

  • At boundary blocks: getCurrentPhase() returns Phase 2, but swap executes with Phase 1 rules

Proof of Concept

At phase boundary block: getCurrentPhase() returns 2, but swap executes with Phase 1 rules.

function test_PhaseOffByOne() public {
// Fast forward to exactly the phase boundary
vm.roll(block.number + phase1Duration);
// getCurrentPhase says Phase 2
assertEq(antiBotHook.getCurrentPhase(), 2, "View says Phase 2");
// But _beforeSwap will use Phase 1 rules (because <= is used)
// This is demonstrated by the currentPhase state variable after a swap
vm.deal(user1, 10 ether);
vm.startPrank(user1);
SwapParams memory params = SwapParams({
zeroForOne: true,
amountSpecified: -int256(0.001 ether),
sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE + 1
});
PoolSwapTest.TestSettings memory testSettings =
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false});
swapRouter.swap{value: 0.001 ether}(key, params, testSettings, ZERO_BYTES);
vm.stopPrank();
// After swap, currentPhase state is still 1 (the swap used Phase 1 rules)
assertEq(antiBotHook.currentPhase(), 1, "Swap executed with Phase 1 rules");
}

Recommended Mitigation

Change < to <= in getCurrentPhase() to match _beforeSwap logic.

function getCurrentPhase() public view returns (uint256) {
if (launchStartBlock == 0) return 0;
uint256 blocksSinceLaunch = block.number - launchStartBlock;
- if (blocksSinceLaunch < phase1Duration) {
+ if (blocksSinceLaunch <= phase1Duration) {
return 1;
- } else if (blocksSinceLaunch < phase1Duration + phase2Duration) {
+ } else if (blocksSinceLaunch <= phase1Duration + phase2Duration) {
return 2;
} else {
return 3;
}
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!