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

Missing `renounceOwnership` override lets the owner permanently brick pool and factory administration

Author Revealed upon completion

Description

  • ConfidencePool::Ownable2Step and ConfidencePoolFactory::Ownable2StepUpgradeable manage the recovery address, expiry, scope, pausing, UUPS upgrades, and the stake-token allowlist through a single onlyOwner account, guarded by a two-step transfer so ownership is never lost by accident.

  • Neither contract overrides the inherited renounceOwnership(). It is public, callable by the owner, and sets the owner to address(0) in one step - bypassing the two-step guard. Afterward every onlyOwner function is permanently uncallable. Renounce while paused makes the pause permanent (staking / createPool dead forever) and a wrong recoveryAddress can never be corrected.

// Inherited from OZ, NOT overridden in either contract.
function renounceOwnership() public virtual onlyOwner {
@> _transferOwnership(address(0)); // one-way; all onlyOwner paths die permanently
}

Risk

Likelihood (Low):

  • Occurs when the owner calls renounceOwnership() - a public, zero-arg, no-confirmation function on the same ABI as routine admin calls, fired by an operator mistake or a copy-pasted admin script.

Impact (Low):

  • Administration is irreversibly bricked: pause/unpause, setRecoveryAddress, setExpiry, setPoolScope on the pool; _authorizeUpgrade, setStakeTokenAllowed, setSafeHarborRegistry, createPool on the factory.

  • No staker principal is lost - withdraw/claim*/sweep*/claimExpired are neither owner- nor pause-gated - which bounds this to Low. But unlike every other admin lever (all reversible/backstopped), renounce removes all recovery paths at once with no counter-action from any address. Not covered by the docs: DESIGN.md/README.md never mention renounce, and §11's out-of-model carve-out is for deliberate trusted-entity sabotage, not an accidental irreversible foot-gun.

Proof of Concept

Self-contained: drop into test/ and run. PASSES against HEAD 58e8ba4 (no RPC). After pause() then renounceOwnership(), administration is permanently dead while a prior staker still recovers principal.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
​
contract L01RenouncePoC is BaseConfidencePoolTest {
function test_renounceOwnership_permanentlyBricksAdmin() public {
_stake(alice, ONE);
​
pool.pause();
pool.renounceOwnership();
assertEq(pool.owner(), address(0));
​
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, address(this)));
pool.unpause();
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, address(this)));
pool.setRecoveryAddress(makeAddr("newRecovery"));
​
token.mint(bob, ONE);
vm.startPrank(bob);
token.approve(address(pool), ONE);
vm.expectRevert(IConfidencePool.PoolPaused.selector);
pool.stake(ONE);
vm.stopPrank();
​
_withdraw(alice);
assertEq(token.balanceOf(alice), ONE); // funds recoverable
}
}

Recommended Mitigation

Override renounceOwnership() to revert on both contracts (two-step transfer still works):

// src/ConfidencePool.sol and src/ConfidencePoolFactory.sol
+ function renounceOwnership() public view override onlyOwner {
+ revert RenounceDisabled();
+ }
// src/interfaces/IConfidencePool.sol and IConfidencePoolFactory.sol
+ error RenounceDisabled();

Support

FAQs

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

Give us feedback!