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

Stale _firstGoodFaithCorruptedAt anchor causes corruptedClaimDeadline to reuse an already-expired window after a legitimate flag→re-flag cycle, permanently denying a validly re-named whitehat attacker their bounty

Author Revealed upon completion

Description

Normal behavior: flagOutcome allows the moderator to re-flag the outcome any time before the first claim (claimsStarted == false), so a mistaken flag can be corrected. When a good-faith CORRUPTED flag is set, corruptedClaimDeadline is meant to give the named whitehat attacker a full CORRUPTED_CLAIM_WINDOW (180 days) to claim their bounty via claimAttackerBounty().

The issue: the anchor timestamp used to compute that deadline, _firstGoodFaithCorruptedAt, is only ever set once and never reset — by design, to prevent the moderator from repeatedly extending the window. But this same protection has an unintended consequence: if the moderator flags good-faith CORRUPTED (setting the anchor), then re-flags away to a different outcome before any claim occurs, then later re-flags good-faith CORRUPTED again — naming the same or a different attacker — corruptedClaimDeadline is recomputed from the original, stale anchor. If enough time has passed since the first flag, the "new" claim window is already in the past the moment it's set, and the attacker has zero time to claim. The bounty then becomes unclaimable and eventually falls through to sweepUnclaimedCorrupted(), which redirects the entire pool to recoveryAddress instead of the whitehat.

function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
// ...
bool willBeGoodFaithCorrupted = newOutcome == PoolStates.Outcome.CORRUPTED && goodFaith_;
// ...
if (willBeGoodFaithCorrupted) {
if (_firstGoodFaithCorruptedAt == 0) {
_firstGoodFaithCorruptedAt = uint32(block.timestamp);
}
@> // Reuses the ORIGINAL anchor on every subsequent good-faith CORRUPTED flag --
@> // even after a toggle away and back. If the original window has already elapsed,
@> // the recomputed deadline lands in the past the instant it's set.
corruptedClaimDeadline = uint32(_firstGoodFaithCorruptedAt + CORRUPTED_CLAIM_WINDOW);
} else {
corruptedClaimDeadline = 0;
}
function claimAttackerBounty() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
if (bountyClaimed == bountyEntitlement) revert BountyAlreadyClaimed();
if (!goodFaith) revert InvalidGoodFaithParams();
if (msg.sender != attacker) revert NotAttacker();
@> // Reverts immediately if the stale anchor already pushed the deadline into the past --
@> // the attacker never gets a chance to claim, regardless of how recently they were named.
if (block.timestamp > corruptedClaimDeadline) revert ClaimWindowExpired();

Risk

Likelihood:

  • The moderator uses the documented pre-claim correction window as intended — flags good-faith CORRUPTED, then later corrects course (e.g. new information suggests SURVIVED is more accurate) — and only re-flags back to good-faith CORRUPTED after the original 180-day window would have elapsed. This requires no malicious intent; a slow-moving DAO vote or a delayed second review is enough to trigger it.

  • No claim occurs between the two good-faith CORRUPTED flags, which is normal if the attacker hasn't yet acted (nothing requires the attacker to claim immediately after being named).

Impact:

  • The named whitehat attacker receives a zero-length (or already-expired) claim window and cannot claim their bounty at all, despite being validly named a second time by the moderator.

  • The full stake + bonus entitlement that should have gone to the attacker instead sits until sweepUnclaimedCorrupted() redirects it to recoveryAddress, permanently denying the whitehat their reward with no on-chain recourse.

Proof of Concept

Save as test/poc/DeadlineAnchorReuse.t.sol — fully self-contained, no other test files required.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import "forge-std/Test.sol";
import "src/ConfidencePool.sol";
import "src/libraries/PoolStates.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {IBattleChainSafeHarborRegistry} from "@battlechain/interface/IBattleChainSafeHarborRegistry.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IAgreement} from "@battlechain/interface/IAgreement.sol";
import {Account, Chain, BountyTerms, AgreementDetails, Contact} from "@battlechain/types/AgreementTypes.sol";
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
// ---------------------------------------------------------------------------
// Self-contained mocks (inlined so this PoC has zero external file dependencies)
// ---------------------------------------------------------------------------
contract MockAttackRegistry is IAttackRegistry {
ContractState public state = ContractState.NOT_DEPLOYED;
function setState(ContractState s) external { state = s; }
function getAgreementState(address) external view returns (ContractState) { return state; }
function approveAttack(address) external pure { revert("stub"); }
function authorizeAgreementOwner(address, address) external pure { revert("stub"); }
function cancelPromotion(address) external pure { revert("stub"); }
function changeRegistryModerator(address) external pure { revert("stub"); }
function getAgreementForContract(address) external pure returns (address) { revert("stub"); }
function getAgreementInfo(address) external pure returns (AgreementInfo memory) { revert("stub"); }
function getAttackModerator(address) external pure returns (address) { revert("stub"); }
function getAuthorizedOwner(address) external pure returns (address) { revert("stub"); }
function getRegistryModerator() external pure returns (address) { revert("stub"); }
function getSafeHarborRegistry() external pure returns (address) { revert("stub"); }
function goToProduction(address) external pure { revert("stub"); }
function instantCorrupt(address) external pure { revert("stub"); }
function instantPromote(address) external pure { revert("stub"); }
function isTopLevelContractUnderAttack(address) external pure returns (bool) { revert("stub"); }
function markCorrupted(address) external pure { revert("stub"); }
function promote(address) external pure { revert("stub"); }
function registerContractForExistingAgreement(address) external pure { revert("stub"); }
function registerDeployment(address, address) external pure { revert("stub"); }
function rejectAttackRequest(address, bool) external pure { revert("stub"); }
function requestUnderAttack(address) external pure { revert("stub"); }
function requestUnderAttackByNonAuthorized(address) external pure { revert("stub"); }
function requestUnderAttackForUnverifiedContracts(address) external pure { revert("stub"); }
function setSafeHarborRegistry(address) external pure { revert("stub"); }
function syncNewContracts(address) external pure { revert("stub"); }
function transferAttackModerator(address, address) external pure { revert("stub"); }
function unregisterContractForExistingAgreement(address) external pure { revert("stub"); }
}
contract MockAgreement is IAgreement {
address public owner_;
mapping(address => bool) public inScope;
constructor(address o) { owner_ = o; }
function owner() external view returns (address) { return owner_; }
function setInScope(address a, bool v) external { inScope[a] = v; }
function isContractInScope(address a) external view returns (bool) { return inScope[a]; }
function addAccounts(string memory, Account[] memory) external pure { revert("stub"); }
function addOrSetChains(Chain[] memory) external pure { revert("stub"); }
function extendCommitmentWindow(uint256) external pure { revert("stub"); }
function getAgreementURI() external pure returns (string memory) { revert("stub"); }
function getBattleChainCaip2ChainId() external pure returns (string memory) { revert("stub"); }
function getBattleChainScopeAddresses() external pure returns (address[] memory) { revert("stub"); }
function getBattleChainScopeCount() external pure returns (uint256) { revert("stub"); }
function getBountyTerms() external pure returns (BountyTerms memory) { revert("stub"); }
function getCantChangeUntil() external pure returns (uint256) { revert("stub"); }
function getChainIds() external pure returns (string[] memory) { revert("stub"); }
function getDetails() external pure returns (AgreementDetails memory) { revert("stub"); }
function getProtocolName() external pure returns (string memory) { revert("stub"); }
function getRegistry() external pure returns (address) { revert("stub"); }
function removeAccounts(string memory, string[] memory) external pure { revert("stub"); }
function removeChains(string[] memory) external pure { revert("stub"); }
function setAgreementURI(string memory) external pure { revert("stub"); }
function setBountyTerms(BountyTerms memory) external pure { revert("stub"); }
function setContactDetails(Contact[] memory) external pure { revert("stub"); }
function setProtocolName(string memory) external pure { revert("stub"); }
}
contract MockRegistry is IBattleChainSafeHarborRegistry {
address public attackRegistry_;
mapping(address => bool) public valid;
constructor(address ar) { attackRegistry_ = ar; }
function setValid(address a, bool v) external { valid[a] = v; }
function isAgreementValid(address a) external view returns (bool) { return valid[a]; }
function getAttackRegistry() external view returns (address) { return attackRegistry_; }
function adoptSafeHarbor(address) external pure { revert("stub"); }
function getAgreement(address) external pure returns (address) { revert("stub"); }
function getAgreementFactory() external pure returns (address) { revert("stub"); }
function isChainValid(string calldata) external pure returns (bool) { revert("stub"); }
function setAgreementFactory(address) external pure { revert("stub"); }
}
contract MockStakeToken is ERC20 {
constructor() ERC20("Mock", "MCK") { _mint(msg.sender, 1_000_000e18); }
}
// ---------------------------------------------------------------------------
// PoC
// ---------------------------------------------------------------------------
contract DeadlineAnchorReuseTest is Test {
ConfidencePool pool;
MockRegistry registry;
MockAttackRegistry attackRegistry;
MockAgreement agreement;
MockStakeToken token;
address sponsor = address(0xA11CE);
address moderator = address(0xB0B);
address staker = address(0xCAFE);
address recovery = address(0xD00D);
address attacker = address(0xEEEE);
function setUp() public {
attackRegistry = new MockAttackRegistry();
registry = new MockRegistry(address(attackRegistry));
agreement = new MockAgreement(sponsor);
token = new MockStakeToken();
registry.setValid(address(agreement), true);
agreement.setInScope(address(0x1), true);
ConfidencePool implementation = new ConfidencePool();
pool = ConfidencePool(Clones.clone(address(implementation)));
address[] memory accounts = new address[](1);
accounts[0] = address(0x1);
vm.prank(sponsor);
pool.initialize(
address(agreement),
address(token),
address(registry),
moderator,
block.timestamp + 31 days,
1e18,
recovery,
sponsor,
accounts
);
token.transfer(staker, 100e18);
vm.startPrank(staker);
token.approve(address(pool), 100e18);
pool.stake(100e18);
vm.stopPrank();
}
function test_reflagCyclingKillsClaimWindow() public {
// First good-faith CORRUPTED flag anchors _firstGoodFaithCorruptedAt.
attackRegistry.setState(IAttackRegistry.ContractState.CORRUPTED);
vm.startPrank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
uint256 firstDeadline = pool.corruptedClaimDeadline();
// Moderator corrects course before any claim locks in claimsStarted.
attackRegistry.setState(IAttackRegistry.ContractState.PRODUCTION);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
vm.stopPrank();
// Time passes well beyond the ORIGINAL 180-day window.
vm.warp(block.timestamp + 181 days);
// Moderator re-flags good-faith CORRUPTED again, naming the attacker a second time.
attackRegistry.setState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
// Deadline did not reset -- it's already in the past the instant it was recomputed.
assertEq(pool.corruptedClaimDeadline(), firstDeadline, "deadline should not reset on re-flag");
assertLt(pool.corruptedClaimDeadline(), block.timestamp, "window should already be expired");
// The validly re-named attacker cannot claim at all.
vm.prank(attacker);
vm.expectRevert(IConfidencePool.ClaimWindowExpired.selector);
pool.claimAttackerBounty();
}
}

How to run:

forge test --match-contract DeadlineAnchorReuseTest -vvvv

Result: passes. Trace confirms corruptedClaimDeadline is identical before and after the toggle/re-flag cycle, is already in the past at the moment of the second flag, and claimAttackerBounty() reverts with ClaimWindowExpired() for a legitimately (re-)named attacker.

Recommended Mitigation

bool willBeGoodFaithCorrupted = newOutcome == PoolStates.Outcome.CORRUPTED && goodFaith_;
// ...
if (willBeGoodFaithCorrupted) {
- if (_firstGoodFaithCorruptedAt == 0) {
- _firstGoodFaithCorruptedAt = uint32(block.timestamp);
- }
- corruptedClaimDeadline = uint32(_firstGoodFaithCorruptedAt + CORRUPTED_CLAIM_WINDOW);
+ // Only reuse the original anchor if this is a genuine re-affirmation with no
+ // intervening toggle away from good-faith CORRUPTED. If the outcome was ever
+ // changed away and back, treat this as a fresh good-faith flag and grant a
+ // full new window -- the "no extension" protection should guard against a
+ // moderator repeatedly re-flagging *while already in* good-faith CORRUPTED,
+ // not against a legitimate correction-then-reaffirmation sequence.
+ if (_firstGoodFaithCorruptedAt == 0 || !wasLastGoodFaithCorrupted) {
+ _firstGoodFaithCorruptedAt = uint32(block.timestamp);
+ }
+ corruptedClaimDeadline = uint32(_firstGoodFaithCorruptedAt + CORRUPTED_CLAIM_WINDOW);
} else {
corruptedClaimDeadline = 0;
+ wasLastGoodFaithCorrupted = false;
}
+ if (willBeGoodFaithCorrupted) wasLastGoodFaithCorrupted = true;

This preserves the original anti-extension intent (a moderator can't keep re-flagging while already in good-faith CORRUPTED to push the deadline out) while fixing the case this PoC demonstrates: a genuine toggle-away-and-back sequence, which should be treated as a new good-faith determination with its own full claim window, not silently inherit a stale, already-expired one.

Support

FAQs

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

Give us feedback!