BattleChain Confidence Pools

Severity: Low ; defense-in-depth gap, no active exploit path today
Affected: ConfidencePool.flagOutcome at src/ConfidencePool.sol:322
Likelihood × Impact: Likelihood: Low × Impact: Low

TL;DR

The flagOutcome function is the only state-mutating external function in ConfidencePool without nonReentrant, leaving an unprotected window between its outcome write and snapshot writes that could become exploitable if a future code change introduces an external call.

Description

ConfidencePool inherits OpenZeppelin's ReentrancyGuard and applies nonReentrant to every state-mutating external function except one. The inconsistency is structural: every function that writes storage is protected except the function that writes the most storage variables per invocation.

The function writes eight storage variables: outcome at line 354, then snapshotTotalStaked, snapshotTotalBonus, snapshotSumStakeTime, snapshotSumStakeTimeSq, corruptedReserve, bountyEntitlement, and corruptedClaimDeadline at lines 357-375. No external call sits between these writes today ; _observePoolState() at line 328 completes before any storage mutation ; but the missing guard creates a gap that the other eight functions close.

All other mutative functions carry the guard:

Scope Eligibility

ConfidencePool.sol is listed in the contest scope (src/ConfidencePool.sol, 589 nSLOC total). The flagOutcome function is in the deployed bytecode and callable by the moderator. This finding does not rely on any out-of-scope dependency (the registry is trusted by design, per docs/DESIGN.md §11) ; the inconsistency is purely within the pool contract's own modifier posture.

Severity Calibration

CodeHawks severity rules: Low findings are "issues that are not exploitable under normal circumstances but represent poor coding practices or deviations from best practices that could theoretically lead to issues."

This finding: not exploitable today (no external call between the two write phases), but a clear deviation from the contract's own pattern (8/9 functions use nonReentrant). The future risk is that a code change introducing a callback between lines 354 and 357 would open a reentrancy window that the existing pattern was designed to prevent. Severity: Low.

Known-Issue Distinction

No prior audit reports or PRs flag the missing nonReentrant on flagOutcome. The docs/DESIGN.md does not discuss the modifier posture of individual functions. The contract carries // aderyn-ignore-next-line(centralization-risk) on flagOutcome but this suppression is for the onlyModerator gate, not the missing reentrancy guard.

This finding is novel: it identifies a structural inconsistency in modifier application, not a centralization concern.

Likelihood / External Preconditions

1. A future code change introduces an external call between lines 354 and 357. This could be a registry hook, a token callback, or an oracle query added to flagOutcome. Likelihood: unknown, depends on future development.

2. During the reentrancy window, a staker calls claimSurvived() or another claim function. The onlyModerator gate on flagOutcome limits the reentrant caller to the moderator, but cross-contract callbacks (e.g., ERC-777 token hooks) could execute arbitrary code. Likelihood: low ; requires both the external call AND a staker positioned to claim during the window.

3. The snapshot writes are still pending when the claim executes. The attacker would read snapshotTotalBonus == 0 and receive zero bonus share. Likelihood: deterministic if conditions 1-2 are met.

Historical occurrence: No similar exploit has been observed on this contract (code is pre-launch). The pattern of missing nonReentrant has been flagged in other audits as a Low finding (e.g., Spearbit and Trail of Bits reports on various protocols).

Attack Path

1. Moderator calls flagOutcome(SURVIVED, false, address(0)). _observePoolState() completes. outcome = SURVIVED is written (line 354).

2. A future external call (token hook, registry callback) fires between line 354 and line 357. During this callback, a staker calls claimSurvived() ; the outcome == SURVIVED gate passes (line 383), but snapshotTotalBonus is still 0.

3. _bonusShare returns 0 because snapshotTotalBonus == 0 at line 697. The staker claims their full principal with zero bonus.

4. flagOutcome resumes: snapshots are written (lines 357-375). The staker's principal is deducted from totalEligibleStake, but the staker received no bonus share.

5. The unclaimed bonus share remains in the pool and is eventually swept to recoveryAddress via sweepUnclaimedBonus, permanently lost to the staker.

For a pool with 10,000 staked tokens and 1,000 bonus tokens, a staker with 10% of the pool (1,000 staked) would forfeit their proportional bonus share of ~100 tokens to recoveryAddress. The loss scales linearly with the staker's pool share and is bounded by their bonus entitlement, not the full pool.

Today: zero impact ; no external call exists between the writes. The finding is a defense-in-depth gap.

Proof of Concept

Structural confirmation:

Mitigation

This aligns flagOutcome with every other state-mutating function and closes the future-exploitability window between the outcome write and snapshot writes.