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

Missing renounceOwnership() override leads to single owner tx permanently bricks all ConfidencePool.sol admin functions

Author Revealed upon completion

Root + Impact

Description

Ownable2Step inherits the Ownable which contains a function named renounceOwnership which can be used to remove the ownership of contracts in a protocol.

This can lead to ConfidencePool.sol contract becoming diswoned, which will then break critical functions of the protocol.

ConfidencePool.sol inherits the Ownable2Step and renouncing ownership via renounceOwnership() is possible since function is not overriden. The owner can call renounceOwnership() and permanently set _owner = address(0), which freezes the setRecoveryAddress, setExpiry, setPoolScope, pause and unpause at their last state with no on-chain recovery path. Even with a properly-deployed Safe multisig as the brodcaster (so the multisig is Ownable owner), a fulty Safe transaction proposal that calls renounceOwnership() permanently disables governance over the corridor.

ConfidencePool.sol inherits the Ownable2Step (Source):

@> contract ConfidencePool is Initializable, Ownable2Step, ReentrancyGuard, Pausable, IConfidencePool {

Ownable2Step inherits Ownable (Source):

abstract contract Ownable2Step is Ownable {

if we take look at the Ownable from openzeppelin the function (Source) :

function renounceOwnership() public virtual onlyOwner {
_transferOwnership(address(0));
}

which makes call directly to the _transferOwnership() function (Source) :

function _transferOwnership(address newOwner) internal virtual {
address oldOwner = _owner;
_owner = newOwner;
emit OwnershipTransferred(oldOwner, newOwner);
}

in here there is no check for address(0) so directly transfer ownership to 0 leads to bricking all admin operations.

Risk

Likelihood:

  • The protocol uses administrative functions frequently to update core states like pool scopes, expiries, or recovery addresses.

  • A faulty Safe multisig transaction proposal or an operational error executes a call to the inherited renounceOwnership() function.

Impact:

  • The _owner is permanently set to address(0).

  • Critical administrative functions (setRecoveryAddress, setExpiry, setPoolScope, pause, and unpause) become permanently frozen, leaving no on-chain recovery path.

Proof of Concept

Step 1: Open the ConfidencePool.t.sol file under the folder test/unit/ConfidencePool.t.sol.

Step 2: copy and paste the below import under the all import and before ConfidencePoolTest.

import {console} from "forge-std/console.sol";

Step 3: copy and paste the below test function into the ConfidencePoolTest at the very end.

function testRenounceOwnership() external {
address poolOwnerBefore = pool.owner();
console.log("Owner before: ", poolOwnerBefore);
// set new recoveryAddress in the confidencepool contract.
address newRecoveryAddr = makeAddr("newRecoveryAddr");
pool.setRecoveryAddress(newRecoveryAddr);
​
console.log("contract recoveryAddress Changed: ", pool.recoveryAddress());
​
// renounce ownership using renounceOwnership() from owner contract.
pool.renounceOwnership();
address safeWalletRecoveryAddress = makeAddr("safeWalletRecoveryAddress");
// revert because ownership is address to 0x0.
vm.expectRevert();
pool.setRecoveryAddress(safeWalletRecoveryAddress);
address poolOwnerAfter = pool.owner();
console.log("Owner after: ", poolOwnerAfter);
assertEq(poolOwnerAfter ,address(0));
}

Step 4: Run the below command to see result it will show result of renounce ownership.

forge test --match-test testRenounceOwnership -vvv

Result:

Ran 1 test for test/unit/ConfidencePool.t.sol:ConfidencePoolTest
[PASS] testRenounceOwnership() (gas: 39338)
Logs:
Owner before: 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496
contract recoveryAddress Changed: 0x0dD72a54a27eC8731c2AE201a211536b7FA2b9dD
Owner after: 0x0000000000000000000000000000000000000000
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.18ms (327.92µs CPU time)
​
Ran 1 test suite in 9.66ms (2.18ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Cosider overriding renounceOwnership() to revert when called by placing the following code in the ConfidencePool.sol:

+ /// @dev Disables the ability to renounce ownership to prevent protocol bricking.
+ function renounceOwnership() public override onlyOwner {
+ revert("renouncing ownership not allowed");
+ }

Support

FAQs

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

Give us feedback!