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

`InvalidAgreement` Used for Zero Attack Registry Address

Author Revealed upon completion

Root + Impact

Description

In _getAgreementState(), when safeHarborRegistry.getAttackRegistry() returns address(0), the function reverts with InvalidAgreement. But the agreement itself is valid — the failure is that the Safe Harbor Registry's attack registry pointer is misconfigured (returns zero). The same InvalidAgreement error is used when safeHarborRegistry.isAgreementValid(agreement) returns false, which IS a genuinely invalid agreement. Off-chain systems parsing revert reasons cannot distinguish "zero attack registry" from "invalid agreement" — two different root causes sharing one error selector.

// ConfidencePool.sol — _getAgreementState() lines 740-744
function _getAgreementState() internal view returns (IAttackRegistry.ContractState) {
@> address attackRegistry = safeHarborRegistry.getAttackRegistry();
@> if (attackRegistry == address(0)) revert InvalidAgreement(); // <-- not an agreement issue
return IAttackRegistry(attackRegistry).getAgreementState(agreement);
}

Risk

Likelihood: Low — requires the DAO-controlled SafeHarborRegistry to have a zero attack registry pointer, which the DAO controls and won't intentionally set to zero.

Impact: Low — no funds lost; the transaction reverts cleanly. Off-chain debugging and error handling are degraded because two distinct failure modes share one selector.

Proof of Concept

File: L3-InvalidAgreement-AttackRegistry.poc.t.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {ConfidencePool} from "src/ConfidencePool.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
/// @notice PoC for L-3: `InvalidAgreement` error used when attackRegistry returns address(0)
contract L3_InvalidAgreement_AttackRegistry_POC is BaseConfidencePoolTest {
function testPOC_L3_invalidAgreement_whenAttackRegistryZero() external {
// Set the attack registry to zero — _getAgreementState will return 0
safeHarborRegistry.setAttackRegistry(address(0));
​
// _observePoolState is called by stake(), which calls _getAgreementState()
// which checks attackRegistry == address(0) → reverts InvalidAgreement
// But the AGREEMENT is valid — the error name is misleading
token.mint(alice, ONE);
vm.startPrank(alice);
token.approve(address(pool), ONE);
vm.expectRevert(IConfidencePool.InvalidAgreement.selector);
pool.stake(ONE);
vm.stopPrank();
​
// Restore for other tests
safeHarborRegistry.setAttackRegistry(address(attackRegistry));
}
​
function testPOC_L3_misleadingError_confusesDebugging() external {
// Set attackRegistry to valid address but mark agreement as invalid
safeHarborRegistry.setAgreementValid(agreement, false);
​
ConfidencePool implementation = new ConfidencePool();
ConfidencePool freshPool = ConfidencePool(Clones.clone(address(implementation)));
​
// Same InvalidAgreement error — but this time the AGREEMENT is actually invalid
vm.expectRevert(IConfidencePool.InvalidAgreement.selector);
freshPool.initialize(
agreement, address(token), address(safeHarborRegistry),
moderator, block.timestamp + 31 days, ONE, recovery, address(this), _defaultScope()
);
​
// Restore
safeHarborRegistry.setAgreementValid(agreement, true);
}
}

Run: forge test --match-path 'L3-InvalidAgreement-AttackRegistry.poc.t.sol' -vv

Recommended Mitigation

Introduce a distinct AttackRegistryNotSet error and use it in _getAgreementState for the zero-address check, keeping InvalidAgreement for the isAgreementValid check in initialize:

+ error AttackRegistryNotSet();
​
- if (attackRegistry == address(0)) revert InvalidAgreement();
+ if (attackRegistry == address(0)) revert AttackRegistryNotSet();

Support

FAQs

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

Give us feedback!