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

Salt Burning on Failed Initialization

Author Revealed upon completion

When createPool() deploys a clone and initialize() reverts, the salt used for deterministic deployment is permanently burned. The index _poolsByAgreement[agreement].length is never incremented, so the next deployment attempt uses the same salt, which now collides with the bricked clone's address.

This effectively prevents any pool from being created for that agreement at index 0 (or whatever index was used) ever again.

// Root cause in the codebase with @> marks to highlight the relevant section
// ConfidencePoolFactory.sol
function createPool(...) external whenNotPaused returns (address pool) {
// ...
bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length)); // @> Uses current length
pool = Clones.cloneDeterministic(poolImplementation, salt); // @> Deploys clone
IConfidencePool(pool).initialize(...); // @> May revert
// @> If initialize reverts, this is never executed
_poolsByAgreement[agreement].push(pool);
}

Risk

Likelihood:

  • Only occurs when initialize() reverts after cloneDeterministic() succeeds

  • Requires specific conditions: valid salt but invalid initialization parameters

  • Moderate likelihood during development, low in production

Impact:

  • Permanent loss of a deterministic address slot for that agreement

  • Reduced usability for protocol on that agreement

  • Could be used as a targeted griefing vector

Proof of Concept

Setup scenario
​
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import "forge-std/Test.sol";
import "../src/ConfidencePoolFactory.sol";
import "../src/ConfidencePool.sol";
import "@battlechain/interface/IAgreement.sol";
​
contract ConfidencePoolFactoryTest is Test {
ConfidencePoolFactory public factory;
ConfidencePool public poolImplementation;
MockAgreement public mockAgreement;
MockSafeHarborRegistry public mockRegistry;
address public factoryOwner = address(0x1);
address public agreementOwner = address(0x2);
address public moderator = address(0x3);
address public recoveryAddress = address(0x4);
address public stakeToken = address(0x5);
function setUp() public {
// Deploy mock agreement with configurable behavior
mockAgreement = new MockAgreement();
mockAgreement.setOwner(agreementOwner);
mockAgreement.setContractInScope(true); // Initially true
// Deploy mock registry
mockRegistry = new MockSafeHarborRegistry();
mockRegistry.setAgreementValid(address(mockAgreement), true);
// Deploy pool implementation
poolImplementation = new ConfidencePool();
// Deploy factory
vm.startPrank(factoryOwner);
factory = new ConfidencePoolFactory();
factory.initialize(
address(mockRegistry),
address(poolImplementation),
moderator
);
factory.setStakeTokenAllowed(stakeToken, true);
vm.stopPrank();
}
}
​
contract MockAgreement is IAgreement {
address public owner;
mapping(address => bool) public inScope;
bool public scopeValidationEnabled = true;
bool public scopeResult = true;
function setOwner(address _owner) external { owner = _owner; }
function setContractInScope(bool result) external { scopeResult = result; }
function setScopeValidationEnabled(bool enabled) external { scopeValidationEnabled = enabled; }
function owner() external view returns (address) { return owner; }
function isContractInScope(address account) external view returns (bool) {
require(scopeValidationEnabled, "Scope validation disabled");
return scopeResult;
}
}
​
contract MockSafeHarborRegistry {
mapping(address => bool) public validAgreements;
function setAgreementValid(address agreement, bool valid) external {
validAgreements[agreement] = valid;
}
function isAgreementValid(address agreement) external view returns (bool) {
return validAgreements[agreement];
}
function getAttackRegistry() external view returns (address) {
return address(this);
}
}
​
​
​
POC test
​
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import "forge-std/Test.sol";
import "./ConfidencePoolFactoryTest.sol";
​
contract SaltBurningTest is ConfidencePoolFactoryTest {
function test_SaltBurningOnFailedInitialization() public {
// Step 1: Agreement owner creates a pool with valid scope
address[] memory accounts = new address[](1);
accounts[0] = address(0x100);
vm.startPrank(agreementOwner);
// Pool 1 should succeed
address pool1 = factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
accounts
);
assertTrue(pool1 != address(0));
console.log("Pool 1 deployed at:", pool1);
// Check: poolsByAgreement length is 1
address[] memory pools = factory.getPoolsByAgreement(address(mockAgreement));
assertEq(pools.length, 1);
assertEq(pools[0], pool1);
console.log("Pool 1 deployed successfully at index 0");
vm.stopPrank();
// Step 2: Make agreement scope validation fail
// This simulates a malicious agreement or registry change
vm.prank(address(mockAgreement)); // Assuming owner of mock agreement
mockAgreement.setContractInScope(false);
// Step 3: Agreement owner tries to create another pool
vm.startPrank(agreementOwner);
// This should fail because scope validation fails in _replaceScope()
vm.expectRevert(abi.encodeWithSelector(
bytes4(keccak256("AccountNotInAgreementScope(address)")),
accounts[0]
));
factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
accounts
);
vm.stopPrank();
// Step 4: The salt for index 1 is now burned!
// Even though the transaction reverted, cloneDeterministic was called
// The clone was deployed but initialize reverted
// The salt (agreement, 1) is now used
// Step 5: Restore scope validation
mockAgreement.setContractInScope(true);
// Step 6: Try to create another pool with the same index
vm.startPrank(agreementOwner);
// This should revert with address collision
vm.expectRevert(); // Address collision from cloneDeterministic
factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
accounts
);
vm.stopPrank();
console.log("Salt for index 1 is permanently burned!");
console.log("Cannot deploy any more pools for this agreement");
}
function test_SaltBurningWithDuplicateAccounts() public {
// Step 1: Agreement owner tries to create pool with duplicate accounts
address[] memory accounts = new address[](2);
accounts[0] = address(0x100);
accounts[1] = address(0x100); // Duplicate!
vm.startPrank(agreementOwner);
// This will fail due to duplicate accounts in _replaceScope()
vm.expectRevert(abi.encodeWithSelector(
bytes4(keccak256("DuplicateAccount(address)")),
address(0x100)
));
factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
accounts
);
vm.stopPrank();
// Step 2: Try to create a pool with valid accounts
address[] memory validAccounts = new address[](1);
validAccounts[0] = address(0x200);
vm.startPrank(agreementOwner);
// This should revert with address collision because the salt for index 0 is burned
vm.expectRevert(); // Address collision
factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
validAccounts
);
vm.stopPrank();
console.log("Index 0 permanently burned due to duplicate account error!");
}
function test_SaltBurningWithInvalidExpiry() public {
// Step 1: Agreement owner tries to create pool with invalid expiry
address[] memory accounts = new address[](1);
accounts[0] = address(0x100);
vm.startPrank(agreementOwner);
// This will fail due to expiry too soon
vm.expectRevert(abi.encodeWithSignature("ExpiryTooSoon()"));
factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 1 days, // Less than MIN_EXPIRY_LEAD (30 days)
1 ether,
recoveryAddress,
accounts
);
vm.stopPrank();
// Step 2: Try to create a pool with valid expiry
vm.startPrank(agreementOwner);
// This should revert with address collision
vm.expectRevert(); // Address collision
factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
accounts
);
vm.stopPrank();
console.log("Index 0 permanently burned due to invalid expiry!");
}
function test_SaltBurningWithInvalidRecoveryAddress() public {
// Step 1: Agreement owner tries to create pool with zero recovery address
address[] memory accounts = new address[](1);
accounts[0] = address(0x100);
vm.startPrank(agreementOwner);
// This will fail due to zero recovery address
vm.expectRevert(abi.encodeWithSignature("InvalidRecoveryAddress()"));
factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
address(0), // Invalid recovery address
accounts
);
vm.stopPrank();
// Step 2: Try to create a pool with valid recovery address
vm.startPrank(agreementOwner);
// This should revert with address collision
vm.expectRevert(); // Address collision
factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
accounts
);
vm.stopPrank();
console.log("Index 0 permanently burned due to invalid recovery address!");
}
function test_MultipleFailedAttemptsBurnMultipleSalts() public {
address[] memory accounts = new address[](1);
accounts[0] = address(0x100);
vm.startPrank(agreementOwner);
// Attempt 1: Fails due to scope validation (we'll toggle it)
mockAgreement.setContractInScope(false);
vm.expectRevert(abi.encodeWithSelector(
bytes4(keccak256("AccountNotInAgreementScope(address)")),
accounts[0]
));
factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
accounts
);
// Index 0 is burned
// Attempt 2: Fails due to duplicate accounts
address[] memory duplicateAccounts = new address[](2);
duplicateAccounts[0] = address(0x100);
duplicateAccounts[1] = address(0x100);
mockAgreement.setContractInScope(true);
vm.expectRevert(abi.encodeWithSelector(
bytes4(keccak256("DuplicateAccount(address)")),
address(0x100)
));
factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
duplicateAccounts
);
// Index 1 is burned
// Attempt 3: Valid, should succeed
// But it will use index 2 (since 0 and 1 are burned)
address pool = factory.createPool(
address(mockAgreement),
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
accounts
);
assertTrue(pool != address(0));
console.log("Pool created at index 2 after burning indices 0 and 1");
vm.stopPrank();
// Verify the pools array has length 1 (only the successful one)
address[] memory pools = factory.getPoolsByAgreement(address(mockAgreement));
assertEq(pools.length, 1);
assertEq(pools[0], pool);
console.log("Indices 0 and 1 are permanently burned but index 2 works");
console.log("This means some agreement indexes are permanently unusable");
}
}

Recommended Mitigation

- remove this code
+ add this // ConfidencePoolFactory.sol
+ mapping(address agreement => uint256 count) public saltCounter;
​
function createPool(
address agreement,
address stakeToken,
uint256 expiry,
uint256 minStake,
address recoveryAddress,
address[] calldata accounts
) external whenNotPaused returns (address pool) {
if (agreement == address(0) || stakeToken == address(0)) revert ZeroAddress();
if (recoveryAddress == address(0)) revert ZeroAddress();
if (!allowedStakeToken[stakeToken]) revert StakeTokenNotAllowed();
if (expiry < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (!safeHarborRegistry.isAgreementValid(agreement)) revert InvalidAgreement();
if (IAgreement(agreement).owner() != msg.sender) revert UnauthorizedCreator();
​
- bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length));
+ uint256 saltNonce = saltCounter[agreement];
+ bytes32 salt = keccak256(abi.encode(agreement, saltNonce));
+ saltCounter[agreement] = saltNonce + 1;
pool = Clones.cloneDeterministic(poolImplementation, salt);
​
IConfidencePool(pool).initialize(
agreement,
stakeToken,
address(safeHarborRegistry),
defaultOutcomeModerator,
expiry,
minStake,
recoveryAddress,
msg.sender,
accounts
);
​
_poolsByAgreement[agreement].push(pool);
emit PoolCreated(
agreement,
pool,
stakeToken,
expiry,
minStake,
recoveryAddress,
defaultOutcomeModerator,
address(safeHarborRegistry)
);
}

Support

FAQs

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

Give us feedback!