FoundrySolidityLayer 2
7.25 ETH
Submission Details
Impact: low
Likelihood: low

long-running attack phase past expiry fully collapses k=2 time-weighting

Author Revealed upon completion

Root + Impact

Root Cause:
_markRiskWindowStart()/_markRiskWindowEnd() cap their sealed timestamp at expiry (if (t > expiry) t = expiry), and risk-window observation is lazy — it only happens when someone calls a pool function, not automatically when the upstream registry transitions. Since BattleChain's UNDER_ATTACK state has no on-chain timeout (confirmed against the upstream AttackRegistry), an agreement can remain under attack indefinitely — well past a given pool's own expiry. If nobody interacts with the pool during that entire active-risk window, the first post-expiry call is what seals riskWindowStart for the first time, and it lands exactly at expiry. Because T for the EXPIRED path is also expiry, every staker's (T - entryTime)² term becomes (expiry - expiry)² = 0 simultaneously, driving globalScore to zero and triggering _bonusShare's amount-weighted fallback for the entire pool.


Impact:

  • The k=2 formula's core incentive — reward early, long-duration risk-takers more than late entrants — silently degrades to a flat pro-rata split by stake amount alone, for every staker in the pool, not just late ones.

  • No principal or bonus is lost from the pool; the total payout is identical, only its distribution among stakers changes versus what the formula was designed to produce.

  • Not attacker-directable: any single stake, withdraw, or a permissionless pokeRiskWindow() call from any party during the active-risk window breaks the setup by sealing riskWindowStart earlier — so this is an organic liveness/timing gap rather than something one party can engineer against another for gain.

Description

  • Normal behavior:
    The bonus pool is meant to be split among stakers using a k=2 time-weighted formula (amount × (T − entryTime)²) so that earlier, longer-duration risk-takers receive a disproportionately larger share than late entrants — deliberately "crushing" late joiners' bonus. T and riskWindowStart are meant to bound this weighting to the interval during which the agreement was genuinely in an active-risk state, each capped at expiry so the accrual window can't extend past the pool's own lifecycle.

  • The issue:
    Risk-window observation is lazy (only recorded when a pool function is called) and BattleChain's UNDER_ATTACK state has no on-chain timeout, so an agreement can remain under attack indefinitely, well past a given pool's expiry. If no one interacts with the pool during that entire window, the first post-expiry call is what seals riskWindowStart for the first time — and the expiry-cap forces that seal to land exactly at expiry itself. Since T for the EXPIRED path is also expiry, every staker's (T − entryTime)² term becomes zero simultaneously, globalScore collapses to zero, and _bonusShare falls back to a flat amount-weighted split — silently erasing the time-weighting for every staker in the pool rather than just the late ones it was designed to penalize.


function _markRiskWindowStart() internal {
uint256 t = block.timestamp;
@> if (t > expiry) t = expiry;
riskWindowStart = uint32(t);
sumStakeTime = totalEligibleStake * t;
sumStakeTimeSq = totalEligibleStake * t * t;
emit RiskWindowStarted(t);
}
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
// ...
@> if (riskWindowStart == 0 && _isActiveRiskState(state)) {
@> _markRiskWindowStart(); // first-ever observation can happen well after `expiry`
@> }
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}
function claimExpired() external nonReentrant {
if (block.timestamp < expiry) revert PoolNotExpired();
// ...
if (outcome == PoolStates.Outcome.UNRESOLVED) {
IAttackRegistry.ContractState state = _observePoolState(); // seals riskWindowStart == expiry here
// ...
} else {
outcome = PoolStates.Outcome.EXPIRED;
@> outcomeFlaggedAt = expiry; // T == expiry, same value riskWindowStart was just capped to
emit OutcomeFlagged(address(0), PoolStates.Outcome.EXPIRED, false, address(0));
}
}
// ...
}
function _bonusShare(address u, uint256 userEligible) internal view returns (uint256) {
if (snapshotTotalBonus == 0) return 0;
if (riskWindowStart == 0) return 0;
uint256 T = outcomeFlaggedAt;
// ... userScore computed from (T - entryTime) terms ...
@> if (globalScore == 0) {
@> // No time elapsed in the risk window for anyone → fallback to amount-weighted.
@> if (snapshotTotalStaked == 0) return 0;
@> return Math.mulDiv(userEligible, snapshotTotalBonus, snapshotTotalStaked);
}
return Math.mulDiv(userScore, snapshotTotalBonus, globalScore);
}

Risk

Likelihood:

  • Occurs whenever a pool's expiry is set shorter than the actual duration the underlying agreement spends in UNDER_ATTACK, which is a real and expected scenario since the BattleChain AttackRegistry places no timeout on that state and protocols are documented to use long-running attack phases.

  • Occurs whenever the risk window closes and reopens/observes without any staker, withdrawer, or third party calling pokeRiskWindow() (or any other pool function) during the active-risk period — a passive, no-action default rather than something requiring deliberate manipulation.

Impact:

  • Every staker's bonus share collapses to a flat amount-weighted split, regardless of how early or how long they were genuinely at risk, silently erasing the core early-staker incentive the k=2 formula exists to provide.

  • Early, long-duration risk-takers receive materially less bonus than the formula promises, while late entrants receive materially more than the formula was designed to allow them — a redistribution among stakers that contradicts the pool's advertised mechanics, even though total payout and principal remain intact.

Proof of Concept

Drop it into test/unit/ — it extends the repo's own BaseConfidencePoolTest so it picks up the existing mocks automatically.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
/// @title ConfidencePool — audit findings PoC
/// @notice Drop this file into `test/unit/` in the ConfidencePool repo. It reuses the project's
/// own `BaseConfidencePoolTest` harness, so it will pick up the same mocks (MockAttackRegistry,
/// MockSafeHarborRegistry, MockAgreement, MockERC20) already wired in `setUp()`.
/// Run with: forge test --match-contract ConfidencePoolFindingsTest -vvvv
contract ConfidencePoolFindingsTest is BaseConfidencePoolTest {
function test_Finding2_LongUnderAttackPastExpiryCollapsesTimeWeighting()
public
{
// Alice stakes on day 0 — a genuinely early, long-duration risk-taker.
_stake(alice, 10 * ONE);
// Bob stakes on day 20 of a 31-day pool — a late, short-duration risk-taker who the k=2
// formula is specifically designed to pay much less bonus than Alice.
vm.warp(block.timestamp + 20 days);
_stake(bob, 10 * ONE);
_contributeBonus(carol, 100 * ONE);
// The agreement only enters UNDER_ATTACK late, and nobody pokes/interacts with the pool
// while it's there. It stays UNDER_ATTACK straight through the pool's own expiry (legal
// upstream — AttackRegistry.UNDER_ATTACK has no timeout).
vm.warp(block.timestamp + 15 days); // now past the 31-day expiry
attackRegistry.setAgreementState(
IAttackRegistry.ContractState.UNDER_ATTACK
);
// First-ever pool interaction post-expiry is the mechanical resolution call. This is what
// lazily (and for the first time) observes UNDER_ATTACK — capping riskWindowStart at
// `expiry`.
pool.claimExpired();
assertEq(uint256(pool.outcome()), uint256(3)); // PoolStates.Outcome.EXPIRED
assertEq(
pool.riskWindowStart(),
pool.expiry(),
"riskWindowStart got capped exactly to expiry"
);
vm.prank(alice);
pool.claimExpired();
vm.prank(bob);
pool.claimExpired();
// Both staked the identical 10 tokens, so under a correctly functioning k=2 formula their
// bonus rate-per-token should differ enormously (Alice was at risk for the whole term,
// Bob for less than a third of it). Instead:
// (bonusShare is not directly exposed; assert via token balances instead)
uint256 aliceBalance = tokenBalanceOf(alice);
uint256 bobBalance = tokenBalanceOf(bob);
// Alice and Bob both staked 10 tokens and receive an identical payout — the time-weighting
// has been fully neutralized despite their very different real risk exposure.
assertEq(
aliceBalance,
bobBalance,
"k=2 time-weighting collapsed to equal payouts despite unequal risk duration"
);
}
function tokenBalanceOf(address who) internal view returns (uint256) {
return token.balanceOf(who);
}
}

Recommended Mitigation

Given this touches the same cap _markRiskWindowEnd relies on and the "accrual bounded by pool lifecycle" invariant the current code leans on elsewhere, I'd flag this one to the sponsor as a discussion point rather than a drop-in patch — the minimal, lowest-risk alternative is to leave the math untouched and instead document the scenario explicitly in DESIGN.md (since total payout/principal safety isn't affected, only the intra-staker bonus split), pushing this from "needs a code fix" to "needs an acknowledged limitation," which is a legitimate response the sponsor may prefer given how load-bearing the expiry-cap is elsewhere in the contract.

function _markRiskWindowStart() internal {
uint256 t = block.timestamp;
- if (t > expiry) t = expiry;
+ // No longer capped at `expiry`: a risk window sealed after `expiry` (agreement still
+ // UNDER_ATTACK past the pool's own deadline) must retain a real, later timestamp so the
+ // EXPIRED-path bonus formula has a nonzero (T - riskWindowStart) spread to weight by,
+ // instead of collapsing every staker's score to the same value.
riskWindowStart = uint32(t);
sumStakeTime = totalEligibleStake * t;
sumStakeTimeSq = totalEligibleStake * t * t;
emit RiskWindowStarted(t);
}

Support

FAQs

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

Give us feedback!