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

Unrestricted owner capability to update recoveryAddress enables potential fund siphoning

Author Revealed upon completion

Root + Impact

Description

  • Normal Behavior: The recoveryAddress is intended to be a secure, predetermined address where pool funds are routed if the contract enters a CORRUPTED state. This acts as a safety net for users' deposited assets.

  • Issue: The setRecoveryAddress function is entirely unrestricted. The contract owner can modify the recoveryAddress at any point in the pool's lifecycle, including after users have deposited substantial funds. There is no timelock, no multi-sig requirement, and no mechanism to lock this address once the pool is active.

  • Root Cause: The setRecoveryAddress implementation lacks state-based constraints (such as checking if the pool is already funded or active) or time-delays that would prevent the owner from redirecting funds to an arbitrary address just before triggering a recovery event.

@> function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
@> if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
@>
@> address oldRecoveryAddress = recoveryAddress;
@> recoveryAddress = newRecoveryAddress;
@>
@> emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}

Risk

  • Likelihood: This represents a significant centralization risk. While the owner may be trusted, a compromised owner private key or a malicious actor with control over the administrative address could exploit this permissionless function to divert assets.

  • Impact: If the pool balance is directed to a recovery address, an attacker can siphon 100% of the pool's funds. Users have no protection or "early warning" mechanisms to detect this change before the state-transition occurs.


Proof of Concept

The following test case demonstrates that the recoveryAddress can be modified by the owner at any time, even after the pool setup is finalized:

function test_OwnerCanUpdateRecoveryAddressAtWill() public {
// 1. Initial recovery address is set
address initialRecovery = pool.recoveryAddress();
// 2. Owner changes recovery address to an attacker-controlled address
address maliciousAddress = address(0xDEADBEEF);
vm.prank(owner);
pool.setRecoveryAddress(maliciousAddress);
// 3. Confirm the change was successful
assertEq(pool.recoveryAddress(), maliciousAddress);
assertTrue(pool.recoveryAddress() != initialRecovery);
}


Recommended Mitigation:

To protect stakers, the recoveryAddress should be immutable once the pool has been initialized or after the first deposit. Alternatively, introduce a mandatory delay (Timelock) for sensitive administrative changes:

- function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
+ function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
+ // Prevent changes after the pool is funded to protect user assets
+ require(totalStaked == 0, "Pool already active");
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();

Support

FAQs

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

Give us feedback!