BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: low

Risk-window markers can become asymmetric or change after resolution

Author Revealed upon completion

Root + Impact

Description

_observePoolState() updates the risk-window start and end with independent guards. If the registry reaches PRODUCTION or CORRUPTED without an active-risk state ever being observed, the pool writes riskWindowEnd but leaves riskWindowStart at zero. The pair therefore no longer has the ordinary meaning of two bounds belonging to the same interval.

The current implementation deliberately treats a zero start as “no observable risk”: stakers receive no bonus and the contributed bonus can be swept to recovery. The immediate effect is consequently limited to an internally asymmetric representation and a latent integration hazard for future code that reads the nonzero end without also checking the start.

One source report additionally claimed that repeated claimExpired() calls could write markers after resolution. That portion is stale for the reviewed source. claimExpired() now calls _observePoolState() only while the outcome is unresolved, and pokeRiskWindow() returns immediately after resolution. The PoC includes a regression test proving that both markers and outcomeFlaggedAt remain frozen. This finding is therefore partially confirmed: marker asymmetry is reproducible, but post-resolution mutation is not.

The start marker is written only for UNDER_ATTACK or PROMOTION_REQUESTED, whereas the end marker is written only for PRODUCTION or CORRUPTED. Because the checks are independent, directly observing a terminal state creates the persistent combination riskWindowStart == 0 && riskWindowEnd != 0.

src/ConfidencePool.sol
​
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
// ... scope lock omitted ...
if (riskWindowStart == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
if (riskWindowEnd == 0 && _isTerminalState(state)) { // @> END is independent of START.
_markRiskWindowEnd();
}
}
​
function _markRiskWindowEnd() internal {
uint256 t = block.timestamp;
if (t > expiry) t = expiry;
riskWindowEnd = uint32(t); // @> A terminal observation can write END while START is zero.
emit RiskWindowEnded(t);
}
​
function _bonusShare(address u, uint256 userEligible) internal view returns (uint256) {
if (snapshotTotalBonus == 0) return 0;
if (riskWindowStart == 0) return 0; // @> Current consumers deliberately special-case the partial pair.
// ...
}

This representation is internally handled today, but its structural meaning is easy to misread: a nonzero end normally suggests that a risk interval existed. A future reward formula, view helper, indexer, or upgrade of surrounding infrastructure that checks only riskWindowEnd or outcomeFlaggedAt could treat the timestamp as the end of a window beginning at zero, or otherwise diverge from the pool's “no observable risk” rule.

The suspected post-resolution path is already defended as follows:

function claimExpired() external nonReentrant {
// ...
if (outcome == PoolStates.Outcome.UNRESOLVED) { // @> Observation is resolution-only.
IAttackRegistry.ContractState state = _observePoolState();
// ...
}
// ...
}
​
function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return; // @> Resolved markers are frozen.
_observePoolState();
// ...
}

Risk

Likelihood:

  • A direct terminal observation without an observed active-risk transition is supported by the state machine and reproducibly creates the asymmetric pair.

  • Current payout and sweep paths consistently check riskWindowStart, so exploitation requires a future or external consumer to infer semantics from the end marker alone.

  • Post-resolution mutation is not reachable through the two cited entrypoints in the reviewed version.

Impact:

  • There is no current principal loss. A zero start intentionally pays no bonus, and the PoC confirms that the bonus remains sweepable.

  • The partial representation weakens the storage invariant and can cause incorrect accounting or analytics if a later consumer assumes a nonzero end implies a valid window.

  • Retaining the existing post-resolution guards is important because reopening either marker after snapshots are frozen would make live markers diverge from resolution accounting.

Proof of Concept

Create test/audit/CP032RiskWindowMarkerAsymmetry.t.sol with the following contents:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
​
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
contract CP032RiskWindowMarkerAsymmetryTest is BaseConfidencePoolTest {
function test_DirectTerminalObservationLeavesAnAsymmetricRiskWindow() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
uint256 terminalObservedAt = block.timestamp;
pool.pokeRiskWindow();
​
assertEq(pool.riskWindowStart(), 0, "no active-risk state was observed");
assertEq(pool.riskWindowEnd(), terminalObservedAt, "terminal observation still writes the end marker");
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
assertEq(pool.outcomeFlaggedAt(), terminalObservedAt);
uint256 aliceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.claimSurvived();
​
assertEq(token.balanceOf(alice) - aliceBefore, 100 * ONE, "zero start encodes no bonus eligibility");
uint256 recoveryBefore = token.balanceOf(recovery);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(recovery) - recoveryBefore, 50 * ONE, "all bonus remains sweepable");
}
​
function test_PostResolutionCallsCannotChangeMarkersInCurrentCode() external {
_stake(alice, 100 * ONE);
​
vm.warp(pool.expiry());
vm.prank(dave);
pool.claimExpired();
​
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.EXPIRED));
assertEq(pool.riskWindowStart(), 0);
assertEq(pool.riskWindowEnd(), 0);
uint256 flaggedAt = pool.outcomeFlaggedAt();
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.warp(block.timestamp + 1 days);
​
pool.pokeRiskWindow();
vm.prank(carol);
pool.claimExpired();
​
assertEq(pool.riskWindowStart(), 0, "post-resolution poke cannot open the window");
assertEq(pool.riskWindowEnd(), 0, "post-resolution calls cannot seal the window");
assertEq(pool.outcomeFlaggedAt(), flaggedAt, "resolution timestamp remains frozen");
}
}

Run the PoC from the repository root:

  1. Execute forge test --offline --match-path test/audit/CP032RiskWindowMarkerAsymmetry.t.sol -vv.

  2. Confirm that both tests pass. The first test produces riskWindowStart == 0 and a nonzero riskWindowEnd, then shows that the staker receives principal only and the full bonus sweeps to recovery. The second resolves with both markers at zero, changes the registry after resolution, calls both relevant entrypoints, and confirms that the markers and resolution timestamp do not change.

Recommended Mitigation

Represent the semantic state explicitly instead of encoding “no observed risk” as one zero marker and one nonzero marker. An enum avoids making existing payout code interpret a fabricated start timestamp; simply setting riskWindowStart = riskWindowEnd would be unsafe because current code treats every nonzero start as evidence that bonus eligibility existed.

src/ConfidencePool.sol
​
+enum RiskWindowStatus {
+ UNOBSERVED,
+ ACTIVE,
+ TERMINAL_WITHOUT_OBSERVED_RISK,
+ TERMINAL_AFTER_OBSERVED_RISK
+}
+
+RiskWindowStatus public riskWindowStatus;
​
function _markRiskWindowStart() internal {
uint256 t = block.timestamp;
if (t > expiry) t = expiry;
riskWindowStart = uint32(t);
+ riskWindowStatus = RiskWindowStatus.ACTIVE;
// ...
}
​
function _markRiskWindowEnd() internal {
uint256 t = block.timestamp;
if (t > expiry) t = expiry;
riskWindowEnd = uint32(t);
+ riskWindowStatus = riskWindowStart == 0
+ ? RiskWindowStatus.TERMINAL_WITHOUT_OBSERVED_RISK
+ : RiskWindowStatus.TERMINAL_AFTER_OBSERVED_RISK;
emit RiskWindowEnded(t);
}
​
function _bonusShare(address u, uint256 userEligible) internal view returns (uint256) {
if (snapshotTotalBonus == 0) return 0;
- if (riskWindowStart == 0) return 0;
+ if (riskWindowStatus == RiskWindowStatus.TERMINAL_WITHOUT_OBSERVED_RISK) return 0;
// ...
}

If adding storage is undesirable, document the partial pair as a formal invariant and expose a view such as hasObservedRiskWindow() that all internal and external consumers use. Preserve the current outcome == UNRESOLVED guards in claimExpired() and pokeRiskWindow(), and keep regression tests that verify both markers and outcomeFlaggedAt remain immutable after resolution.

Support

FAQs

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

Give us feedback!