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

Pool sponsor can repoint recoveryAddress after stakers deposit, redirecting CORRUPTED principal sweeps

Author Revealed upon completion

Root + Impact

Description

• Stakers deposit into a Confidence Pool after inspecting on-chain parameters, including recoveryAddress, which is the destination for bad-faith CORRUPTED full-pool sweeps and post-window CORRUPTED remainder sweeps. The protocol already treats deposit-time reliance as security-critical for expiry: the first successful stake sets expiryLocked = true, so the sponsor cannot move the deadline after capital is committed.

• setRecoveryAddress has no equivalent protection. It remains callable by the pool owner for the entire pool lifetime—after stakes, after risk locks withdrawals, and even after a CORRUPTED flag. A sponsor who is malicious or compromised can repoint recovery to an address stakers never accepted, then capture 100% of principal + bonus on bad-faith CORRUPTED (or residual funds on the good-faith path). This breaks the on-chain parameter set that stakers underwrote and is strictly weaker than the immutability model already applied to expiry.

// src/ConfidencePool.sol — expiry is one-way locked on first stake
function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
// ...
if (!expiryLocked) {
expiryLocked = true; // @> expiry becomes immutable after first stake
}
// ...
}
​
// src/ConfidencePool.sol — recovery has no equivalent latch
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress; // @> mutable forever, including post-stake and post-flag
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
​
// Bad-faith CORRUPTED sweeps full balance to the *current* recoveryAddress
function claimCorrupted() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
if (goodFaith && bountyClaimed < bountyEntitlement) revert MustClaimBountyFirst();
uint256 toSweep = stakeToken.balanceOf(address(this));
// ...
stakeToken.safeTransfer(recoveryAddress, toSweep); // @> uses live recovery, not a deposit-time snapshot
}

Risk

Likelihood:

  • A pool sponsor becomes malicious or key-compromised after deposits have begun, and later a CORRUPTED outcome is finalized via bad-faith flagOutcome or via auto-CORRUPTED after the moderator grace window.

    • A single setRecoveryAddress transaction after the first stake is sufficient. Once the registry has been observed in an active-risk state, withdraw is permanently disabled, so stakers cannot exit before the redirected CORRUPTED sweep.

Impact:

  • On bad-faith CORRUPTED, 100% of staker principal and bonus is transferred to a recovery address that can differ from the address stakers verified at deposit time.

    • On good-faith CORRUPTED, the named attacker bounty is snapshot-capped, but residual balance, post-deadline sweepUnclaimedCorrupted funds, and post-resolution donations still follow the live recoveryAddress, which the sponsor can repoint after the flag.

Proof of Concept

  1. Alice and Carol deposit while recoveryAddress is still the original, advertised recovery address.

  2. The pool owner (sponsor) calls setRecoveryAddress(evilRecovery) after those deposits no staker consent required.

  3. The registry is moved through active risk into CORRUPTED, and the moderator flags bad-faith CORRUPTED.

  4. claimCorrupted sweeps the entire pool balance (stake + bonus = 150e18) to evilRecovery.

  5. The original recovery address receives zero, demonstrating that deposit-time verification of recoveryAddress is not binding.

Why the steps matter

• Step 1 establishes staker reliance on the initial recovery parameter (the same diligence stakers are told to perform for on-chain pool config).
• Step 2 is the root cause in action: setRecoveryAddress is unrestricted after stake, unlike setExpiry.

• Steps 3-4 show the fund path that actually moves principal: bad-faith CORRUPTED → claimCorrupted → safeTransfer(recoveryAddress, ...).

• Step 5 makes the impact measurable: full economic loss relative to the address stakers thought would receive CORRUPTED recovery.


// File: test/audit/AuditResiduals.t.sol
// Run: forge test --match-test test_PoC_sponsorChangesRecoveryAfterStake_redirectsCorruptedSweep -vv
// Result: PASS — 150e18 to evilRecovery; original recovery balance unchanged
​
function test_PoC_sponsorChangesRecoveryAfterStake_redirectsCorruptedSweep() external {
// 1) Stakers deposit against the init-time recovery address
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
​
// 2) Sponsor repoints recovery after capital is committed
address evilRecovery = makeAddr("evilRecovery");
pool.setRecoveryAddress(evilRecovery);
assertEq(pool.recoveryAddress(), evilRecovery);
​
// 3) Resolve bad-faith CORRUPTED (full pool entitled to recovery)
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
​
// 4) Permissionless sweep follows the *new* recovery address
uint256 before = token.balanceOf(evilRecovery);
pool.claimCorrupted();
assertEq(token.balanceOf(evilRecovery) - before, 150 * ONE, "full pool to new recovery");
assertEq(token.balanceOf(recovery), 0, "original recovery gets nothing");
}

Recommended Mitigation

Goal: Make recoveryAddress as sticky as expiry once any staker has committed capital, so deposit-time verification remains valid through CORRUPTED resolution.

Approach: Introduce a one-way recoveryLocked flag set on the first successful stake, and gate setRecoveryAddress on that flag (mirror expiryLocked). This preserves sponsor freedom to set recovery at create time / pre-stake, while preventing post-deposit rugs of the CORRUPTED destination.

bool public expiryLocked;
+bool public recoveryLocked;
+error RecoveryLocked();
​
function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
// ... existing checks ...
if (!expiryLocked) {
expiryLocked = true;
}
+ // Latch recovery on first stake so CORRUPTED destination cannot move under bonded capital
+ if (!recoveryLocked) {
+ recoveryLocked = true;
+ }
// ... transfers and accounting ...
}
​
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
+ if (recoveryLocked) revert RecoveryLocked();
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
​
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
​
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
​
​
​
}

Support

FAQs

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

Give us feedback!