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

Reverting observation rolls back the risk-window latch

Author Revealed upon completion

Affected: ConfidencePool.sol:222-227 · ConfidencePool.sol:727-733 · ConfidencePool.sol:784-799

Description

stake() and contributeBonus() observe registry state before enforcing their deposit gate. _observePoolState() writes riskWindowStart and scopeLocked as one-way transitions, then _assertDepositsAllowed() reverts on PROMOTION_REQUESTED, PRODUCTION, or CORRUPTED. EVM atomicity rolls the writes back with the revert, so the pool never persists the first observation.

// L222-227 — stake observes, then may revert on the same tx
_assertDepositsAllowed(_observePoolState());
​
// L727-733 — revert path
if (state == PROMOTION_REQUESTED || state == PRODUCTION || state == CORRUPTED) {
revert StakingClosed();
}

A permissionless caller (Bob) can therefore attempt stake(1 wei) during PROMOTION_REQUESTED and knowingly force the revert; the latches that _observePoolState() would have sealed are lost. Combined with a later legitimate DAO setAttackRegistry migration that reports NOT_DEPLOYED for the agreement, the withdraw() gate re-opens for previously staked capital — even though active risk was already publicly observable on-chain.

Same root cause as M-3, distinct entrypoint: here the failed sealing is triggered by an unprivileged Bob rather than by the observation ordering of a low-traffic pool.

Risk

Likelihood: Requires two events — permissionless reverting deposit (or bonus) call during active risk, plus a later routine DAO registry migration (DESIGN.md §11 treats migration as expected).

Impact: Existing staker exits principal after active-risk was on-chain. Bounded per-staker; aggregate up to totalEligibleStake. No attacker capture — this is a leak vs. the intended commitment invariant, not a direct value transfer to the attacker.

Actor: Bob (revert trigger) is fully permissionless. The migration is the DAO's documented onlyOwner operation.

Recommended Mitigation

When a deposit is blocked, persist the transition instead of reverting the whole transaction. A seal-only-return pattern preserves the one-way observation without accepting the deposit:

function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
if (amount == 0) revert InvalidAmount();
if (amount < minStake) revert BelowMinStake();
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
if (block.timestamp >= expiry) revert StakingClosed();
- _assertDepositsAllowed(_observePoolState());
+ IAttackRegistry.ContractState state = _observePoolState();
+ if (_depositsBlocked(state)) {
+ // observation is durable; drop the caller's deposit silently
+ return;
+ }
// ... rest unchanged
}

Apply the same rule to contributeBonus(). Alternatively, harden the withdraw gate as recommended in M-3 (|| riskWindowEnd != 0) plus store scopeLocked durably via a dedicated sealObservation() no-op path.

Proof of Concept

Save as test/ObserverRevertRollback.t.sol and run forge test --match-contract ObserverRevertRollbackTest -vv.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
​
contract ObserverRevertRollbackTest is BaseConfidencePoolTest {
function testBlockedStakeRollsBackRiskLatchAndMigrationReopensWithdraw() external {
_stake(alice, 100 * ONE);
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PROMOTION_REQUESTED);
​
// Bob's stake observes active risk then reverts; latches never persist.
token.mint(bob, ONE);
vm.startPrank(bob);
token.approve(address(pool), ONE);
vm.expectRevert(IConfidencePool.StakingClosed.selector);
pool.stake(ONE);
vm.stopPrank();
​
assertEq(pool.riskWindowStart(), 0);
assertFalse(pool.scopeLocked());
​
// DAO migrates; new AttackRegistry reads NOT_DEPLOYED for this agreement.
MockAttackRegistry rewound = new MockAttackRegistry();
rewound.setAgreementState(IAttackRegistry.ContractState.NOT_DEPLOYED);
safeHarborRegistry.setAttackRegistry(address(rewound));
​
_withdraw(alice);
assertEq(token.balanceOf(alice), 100 * ONE);
assertEq(pool.eligibleStake(alice), 0);
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
9 days ago
inallhonesty Lead Judge 2 days ago
Submission Judgement Published
Validated
Assigned finding tags:

withdraw()'s own revert rolls back the riskWindowStart seal it just set, leaving a locked-in staker with zero bonus despite DESIGN.md #9's exit/premium guarantee

Impact - Low The staker keeps their principal, so what is lost is the premium they were accruing while locked, and it ends up at the sponsor's recoveryAddress. DESIGN.md #9 justifies the whole no-observed-risk rule by pairing two events, saying a staker forfeits the exit exactly when the premium starts. One call does the first and undoes the second. Likelihood - Low Any staker can cure this for free with pokeRiskWindow, which succeeds in the same state the withdrawal failed in. A permanent loss therefore needs the whole cohort to miss that step, and the trap is that the obvious move is the one that fails. A short active-risk interval can also close before anyone gets a block in which to poke. DESIGN.md #6 explains an unsealed window as nobody having interacted with the pool, which is exactly what did not happen here.

Support

FAQs

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

Give us feedback!