BattleChain Confidence Pools

Description

• Normal behavior: every fund-moving function (stake, contributeBonus, withdraw, claimSurvived, claimCorrupted, claimAttackerBounty, sweepUnclaimedCorrupted, sweepUnclaimedBonus, claimExpired) carries nonReentrant, which is intended to prevent any reentrant call from re-entering fund-moving logic mid-execution.

• The issue: nonReentrant's guard is backed by a storage slot scoped to that specific clone — clones do not share storage with each other. The factory's allowedStakeToken allowlist is keyed by token address, not by a (token, pool) pair, so the same token is very likely reused across many pools created via the same factory.

If a token ever admitted to the allowlist has any transfer-time callback (ERC-777-style hooks, or a "safe-looking" wrapper that turns out not to be), a hook fired mid-stake() on pool A can reenter pool B's functions freely — pool A's nonReentrant lock does nothing to protect pool B, because they are different storage spaces entirely. The factory's own comment only calls out fee-on-transfer and rebasing tokens as excluded ("Pools assume a standard ERC20 (no transfer fees, no rebasing)"); it does not mention hook/callback-bearing tokens, and the actual failure mode (cross-clone reentrancy, not intra-clone) is only visible when tracing across pool instances rather than within a single one.

Risk

Likelihood:

• Requires the factory owner to admit a hook-bearing token (e.g. an ERC-777-style token, or a wrapper with an unexpected callback) to the allowedStakeToken allowlist — something the current documentation doesn't explicitly guard against, since it only excludes fee-on-transfer and rebasing tokens by name.

• Not exploitable against today's presumed-standard ERC20 tokens; contingent on a future or mistaken allowlist admission.

Impact:

• A hook fired during pool A's stake()/contributeBonus()/etc. could reenter a sibling pool B (sharing the same stakeToken) mid-execution, since nonReentrant provides no protection across separate clone storage spaces.

• Severity is capped at Low because it requires an atypical token to be admitted first — this is a structural gap in the isolation model rather than a directly exploitable path against the current token set.

Proof of Concept

Not applicable as a direct exploit against the current codebase, since no hook-bearing token is presumed to be on the allowlist today. The structural gap can be confirmed by observing that: (1) nonReentrant's lock variable lives in each clone's own storage (EIP-1167 clones do not share storage), and (2) allowedStakeToken is keyed purely by token address in the factory, with no mechanism preventing the same token address from backing multiple independently-deployed pool clones.

Recommended Mitigation

Tighten the allowlist documentation (and ideally the setStakeTokenAllowed review process) to explicitly exclude ERC-777/hook-bearing tokens, not just fee-on-transfer/rebasing ones. Note that nonReentrant structurally cannot cover this case no matter how it is applied within a single clone — the fix here is process/documentation (review discipline before admitting a token), not a code-level guard, since per-clone reentrancy locks cannot be made cross-clone without a shared, factory-level lock (which would itself introduce unwanted coupling between unrelated pools).