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

State modification during loop execution in `_replaceScope` violates Checks-Effects-Interactions

Author Revealed upon completion

Root + Impact

The internal _replaceScope function performs external contract calls while in the middle of clearing and reconstructing the scope state arrays. This violation of the Checks-Effects-Interactions (CEI) pattern introduces potential reentrancy risks and transient state inconsistencies.

Description

When updating the pool's insured scope, the contract clears the previous mapping, deletes _scopeAccounts, and iterates over the new list.

Inside the loop, the contract calls the external agreement contract via IAgreement(agreement).isContractInScope(account). Because this external call occurs before the new elements are completely written to storage, the contract’s state is in a half-constructed transient phase during the external interaction. If the target agreement contract is malicious or behaves dynamically, it can reenter the pool or cause dependent state checks to view an inconsistent scope, breaking the wholesale replacement assumption.

function _replaceScope(address[] calldata accounts) internal {
_clearOldScope();
​
for (uint256 i = 0; i < accounts.length; i++) {
address account = accounts[i];
@> if (!IAgreement(agreement).isContractInScope(account)) {
revert InvalidScopeAccount(account);
}
_scopeAccounts.push(account);
isAccountInScope[account] = true;
}
}

Risk

Likelihood:

  • Occurs whenever the owner updates the pool scope via setPoolScope.

  • Triggered if the associated agreement executes arbitrary code or callbacks during its isContractInScope execution.

Impact:

  • Reentrancy vulnerabilities where transient state can be abused to bypass scope checks.

  • Inconsistent event emissions or state divergence if the call reverts or reenters during scope mutation.

Proof of Concept

function test_PoC_ScopeUpdateInconsistency() public {
MaliciousAgreement malAgreement = new MaliciousAgreement();
safeHarborRegistry.setAgreementValid(address(malAgreement), true);
ConfidencePool malPool = _deployPoolWithAgreement(address(malAgreement));
address[] memory newScope = new address[](1); newScope[0] = address(0xBAD);
malPool.setPoolScope(newScope);
assertTrue(malPool.isAccountInScope(address(0xBAD)));
}

Recommended Mitigation

function _replaceScope(address[] calldata accounts) internal {
+ // Perform all external checks first (Checks)
+ for (uint256 i = 0; i < accounts.length; i++) {
+ if (!IAgreement(agreement).isContractInScope(accounts[i])) {
+ revert InvalidScopeAccount(accounts[i]);
+ }
+ }
+
+ // Modify state afterward (Effects)
_clearOldScope();
for (uint256 i = 0; i < accounts.length; i++) {
address account = accounts[i];
- if (!IAgreement(agreement).isContractInScope(account)) {
- revert InvalidScopeAccount(account);
- }
_scopeAccounts.push(account);
isAccountInScope[account] = true;
}
}

Support

FAQs

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

Give us feedback!