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

ConfidencePoolFactory.createPool Never Verifies AttackRegistry Binding, Allowing Pools to Track an Unrelated Agreement's State

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: createPool should only allow pools for accounts genuinely covered by the given Safe Harbor agreement — the accounts' actual AttackRegistry binding should match agreement.

  • Specific issue: createPool validates only agreement provenance (isAgreementValid) and caller ownership (owner() == msg.sender) — it never checks that each account is bound to agreement in the AttackRegistry. Because the caller is the agreement's own owner, and scope is self-reported by the agreement contract, an attacker can create a throwaway agreement, list any accounts as "in scope," and spin up a factory-listed ConfidencePool that resolves against their own agreement's state instead of the accounts' real binding agreement.

function createPool(
address agreement,
address stakeToken,
uint256 expiry,
uint256 minStake,
address recoveryAddress,
address[] calldata accounts
) external whenNotPaused returns (address pool) {
...
if (!safeHarborRegistry.isAgreementValid(agreement)) revert InvalidAgreement();
// @> owner() is trivially satisfied by an attacker who created `agreement` themselves —
// @> no check ever queries AttackRegistry.getAgreementForContract(account) to confirm
// @> `agreement` is actually the bound agreement for each scoped account.
if (IAgreement(agreement).owner() != msg.sender) revert UnauthorizedCreator();
​
pool = Clones.cloneDeterministic(poolImplementation, salt);
IConfidencePool(pool).initialize(
agreement, stakeToken, address(safeHarborRegistry), defaultOutcomeModerator,
expiry, minStake, recoveryAddress, msg.sender,
// @> `accounts` is only checked via ConfidencePool._replaceScope's
// @> agreement.isContractInScope(account) — self-reported, not a binding check.
accounts
);
...
}

Risk

Likelihood:

  • The factory never imports or calls IAttackRegistry, so no code path in createPool could catch this regardless of caller behavior.

  • Creating an agreement via AgreementFactory and calling owner() on your own contract is permissionless and zero-cost — not a rare precondition.

  • BattleChain's DAO review (approveAttack) only gates an agreement's own transition into UNDER_ATTACK; it never gates createPool, which requires no particular registry state. A throwaway agreement can sit in NOT_DEPLOYED forever, never reviewed by the DAO, while still backing a live pool.

Impact:

  • Stakers and bonus contributors treat "factory-created" as a legitimacy signal; a pool resolving against the wrong agreement's state silently breaks that assumption.

  • Pool resolution (SURVIVED/CORRUPTED/EXPIRED) decouples from the named accounts' real security status, since _getAgreementState reads the attacker's throwaway agreement — misrouting pre-funded bonus and staked principal.

Proof of Concept

The test below shows an attacker creating their own agreement, self-listing a real, unrelated protocol contract as "in scope," and successfully spinning up a factory-listed pool around it — despite the registry confirming that contract is not actually bound to the attacker's agreement.

function test_UnboundAgreementPoolCreation() public {
vm.startPrank(attacker);
address fakeAgreement = agreementFactory.create(/* attacker-controlled details */);
IAgreement(fakeAgreement).setScope(_toArray(realProtocolVault));
​
assertTrue(
attackRegistry.getAgreementForContract(realProtocolVault) != fakeAgreement,
"sanity: realProtocolVault is not bound to fakeAgreement"
);
​
address pool = factory.createPool(
fakeAgreement, address(stakeToken), block.timestamp + 31 days,
1e18, recoveryAddress, _toArray(realProtocolVault)
);
vm.stopPrank();
​
assertEq(IConfidencePool(pool).agreement(), fakeAgreement);
}

Recommended Mitigation

Add a binding check against the AttackRegistry alongside the existing scope check, so a pool can only be created when each account's true bound agreement matches the one being used. This costs one external call per scoped account and rejects pool creation before any clone is deployed.

+ import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
​
if (!safeHarborRegistry.isAgreementValid(agreement)) revert InvalidAgreement();
if (IAgreement(agreement).owner() != msg.sender) revert UnauthorizedCreator();
​
+ address attackRegistry = safeHarborRegistry.getAttackRegistry();
+ for (uint256 i = 0; i < accounts.length; i++) {
+ if (IAttackRegistry(attackRegistry).getAgreementForContract(accounts[i]) != agreement) {
+ revert AccountNotBoundToAgreement(accounts[i]);
+ }
+ }

Note on DAO Review (pre-empting a common rebuttal)

BattleChain's DAO approval process (approveAttack) only gates an agreement's own transition into UNDER_ATTACK — it screens for mainnet-copycat bytecode and confirms deployment origin via getContractDeployer. It does not gate ConfidencePoolFactory.createPool, which never requires any particular registry state. An attacker's throwaway agreement can remain in NOT_DEPLOYED indefinitely — never submitted for DAO review at all — while still backing a live, factory-listed ConfidencePool. The DAO checklist is therefore not a mitigating control for this finding.

Support

FAQs

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

Give us feedback!