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

`claimSurvived()` and `claimExpired()` revert with `InvalidAmount()` on double-claim instead of a semantically correct error, and soft-success path allows unbounded repeated calls

Author Revealed upon completion

Short Summary

When a staker attempts to double-claim via claimSurvived() or claimExpired(), both functions revert with InvalidAmount() — the same error used for zero-amount stake inputs. The semantically correct response is a distinct AlreadyClaimed error. This inconsistency misleads off-chain integrators, DeFi aggregators, and user interfaces that route on error selectors. Additionally, the soft-success path in claimExpired() (for zero-stake callers) never sets hasClaimed, allowing unlimited repeated calls post-resolution.

Root Cause

// src/ConfidencePool.sol L384 — claimSurvived
if (hasClaimed[msg.sender]) revert InvalidAmount(); // wrong selector
​
// src/ConfidencePool.sol L578 — claimExpired
if (hasClaimed[msg.sender]) revert InvalidAmount(); // wrong selector
​
// src/ConfidencePool.sol L434 — claimAttackerBounty (correct by contrast)
if (bountyClaimed == bountyEntitlement) revert BountyAlreadyClaimed();
​
// src/ConfidencePool.sol L581-584 — soft-success path in claimExpired
if (userEligible == 0) {
return; // hasClaimed never set — caller can repeat forever
}

InvalidAmount() is documented and used elsewhere to mean "you passed 0 or a below-minimum value to a deposit function." Reusing it for "you already claimed your tokens" breaks the ABI contract with callers who decode errors by selector.

Proper Description

Solidity custom errors are the primary signalling mechanism between smart contracts and their off-chain consumers (SDKs, frontends, bots). Each error code carries an implicit semantic contract: InvalidAmount means the input value is wrong. Using it to signal state (already claimed) produces a false positive in any error-routing logic that tries to distinguish "user made an input mistake" from "user already redeemed."

A DeFi aggregator that retries on InvalidAmount (interpreting it as a transient input issue) would submit the same claim transaction indefinitely, burning user gas. A frontend that displays the error string for InvalidAmount as "invalid amount" tells the user nothing about what actually happened.

The claimAttackerBounty function handles its equivalent case correctly with BountyAlreadyClaimed(), demonstrating that the protocol team had the right idea — it was simply not applied consistently.

The soft-success issue is a secondary bug in the same area: a non-staker who calls claimExpired() to trigger mechanical pool resolution (a legitimate use-case) has their hasClaimed flag never written. Post-resolution, that address can call claimExpired() in a tight loop (all calls will soft-succeed with no state change). While there is no financial loss, it burns unnecessary gas and is confusing to monitor.

Internal Pre-condition

  • For double-claim revert: hasClaimed[msg.sender] == true (staker has already claimed once).

  • For soft-success repetition: outcome != UNRESOLVED (pool resolved), eligibleStake[msg.sender] == 0 (caller is not a staker).

External Pre-condition

None — these are pure behavioral defects triggered by normal user interaction patterns.

Details Attack Path

Double-claim wrong selector:

  1. Staker calls claimSurvived() — succeeds, hasClaimed[alice] = true.

  2. Staker calls claimSurvived() again (accidentally or in a retry loop).

  3. Reverts with InvalidAmount() — integrator displays "invalid amount entered" to user instead of "already claimed."

  4. User is confused; integrator's error handler may retry the transaction, burning further gas.

Soft-success unbounded repetition:

  1. Non-staker (address with eligibleStake == 0) calls claimExpired() post-expiry.

  2. Pool resolves to EXPIRED; claimsStarted = true; function returns early at line 584.

  3. hasClaimed[caller] is never written to true.

  4. Same caller repeats call — enters the already-resolved branch (line 518 skipped), hits line 578 (hasClaimed == false, no revert), hits line 581 (userEligible == 0, returns again).

  5. This cycle can repeat indefinitely with no on-chain consequence other than gas cost.

Impact

No loss of funds in either sub-case. Impact is:

  • Integrator breakage: error selector mismatch causes incorrect UX rendering and potentially incorrect retry logic.

  • Gas waste: soft-success repetition burns gas on every call with no benefit.

  • Inconsistent ABI: BountyAlreadyClaimed exists for bounty claims but not for staker claims, creating an uneven developer experience.

Proof of Concept

forge test --match-path "test/audit/L2*" -vvv
# [PASS] testPoC_L2_DoubleClaim_ClaimSurvived_WrongSelector()
# [PASS] testPoC_L2_DoubleClaim_ClaimExpired_WrongSelector()
# [PASS] testPoC_L2_SoftSuccessUnboundedRepetition()
// test/audit/L2_WrongRevertSelector.t.sol
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
contract L2_WrongRevertSelectorPoC is BaseConfidencePoolTest {
​
function testPoC_L2_DoubleClaim_ClaimSurvived_WrongSelector() external {
_stake(alice, 100 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
pool.pokeRiskWindow();
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
vm.prank(alice);
pool.claimSurvived();
assertTrue(pool.hasClaimed(alice));
​
// Second claim reverts InvalidAmount() — NOT AlreadyClaimed()
vm.expectRevert(IConfidencePool.InvalidAmount.selector);
vm.prank(alice);
pool.claimSurvived();
}
​
function testPoC_L2_DoubleClaim_ClaimExpired_WrongSelector() external {
_stake(alice, 100 * ONE);
vm.warp(pool.expiry() + 1);
​
vm.prank(alice);
pool.claimExpired();
assertTrue(pool.hasClaimed(alice));
​
vm.expectRevert(IConfidencePool.InvalidAmount.selector);
vm.prank(alice);
pool.claimExpired();
}
​
function testPoC_L2_SoftSuccessUnboundedRepetition() external {
_stake(alice, 100 * ONE);
vm.warp(pool.expiry() + 1);
​
address noStake = makeAddr("noStake");
vm.prank(noStake);
pool.claimExpired(); // resolves pool
​
assertFalse(pool.hasClaimed(noStake)); // hasClaimed never written
​
vm.prank(noStake);
pool.claimExpired(); // no revert — repeats indefinitely
​
vm.prank(noStake);
pool.claimExpired(); // still no revert
}
}

Mitigation

// src/interfaces/IConfidencePool.sol — add a new error:
+ error AlreadyClaimed();
​
// src/ConfidencePool.sol — claimSurvived():
- if (hasClaimed[msg.sender]) revert InvalidAmount();
+ if (hasClaimed[msg.sender]) revert AlreadyClaimed();
​
// src/ConfidencePool.sol — claimExpired():
- if (hasClaimed[msg.sender]) revert InvalidAmount();
+ if (hasClaimed[msg.sender]) revert AlreadyClaimed();
​
// src/ConfidencePool.sol — claimExpired() soft-success path:
if (userEligible == 0) {
+ hasClaimed[msg.sender] = true; // prevent repeated gas-waste calls
return;
}

Support

FAQs

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

Give us feedback!