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

Unreachable `_clampUserSums(msg.sender)` call in `withdraw()` is dead code

Author Revealed upon completion

Root + Impact

Description

ConfidencePool.withdraw() calls _clampUserSums(msg.sender) before it subtracts the caller's weight from the global accumulators. That call can never do any work: withdraw() is gated to revert whenever riskWindowStart != 0, so the only paths that reach the call have riskWindowStart == 0, and _clampUserSums returns immediately at its start == 0 guard without writing.

The relevant withdraw() gate and body:

function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
​
if (
riskWindowStart != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
&& state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
) {
revert WithdrawsDisabled();
}
​
uint256 amount = eligibleStake[msg.sender];
if (amount == 0) revert InvalidAmount();
​
@> _clampUserSums(msg.sender);
// Withdrawing forfeits the caller's bonus claim: subtract their full contribution from
// the global accumulators so honest stakers' shares aren't diluted by the exiter's
// forfeited weight.
sumStakeTime -= userSumStakeTime[msg.sender];
sumStakeTimeSq -= userSumStakeTimeSq[msg.sender];
​
///code...
}

The gate reverts unless the compound condition evaluates to false. For execution to pass the gate and reach _clampUserSums(msg.sender), both sub-terms must be false, which requires:

  • riskWindowStart != 0 is false → riskWindowStart == 0, and

  • state ∈ {NOT_DEPLOYED, NEW_DEPLOYMENT, ATTACK_REQUESTED} (the pre-risk states).

So riskWindowStart == 0 is guaranteed at the call site. Now consider _clampUserSums:

function _clampUserSums(address u) internal {
uint256 start = riskWindowStart;
uint256 stake_ = eligibleStake[u];
if (start == 0 || stake_ == 0) return; // <- always taken here (start == 0)
if (userSumStakeTime[u] < stake_ * start) {
userSumStakeTime[u] = stake_ * start;
userSumStakeTimeSq[u] = stake_ * start * start;
}
}

With start == riskWindowStart == 0, the function hits if (start == 0 ...) return; and writes nothing. It is a guaranteed no-op on every path that reaches it inside withdraw().

Risk

Likelihood:

  • The likelihood is high because the dead code is executed every time withdraw() is called.

Impact:

  • The impact is low because no user funds are lost or blocked. The issue is not exploitable and only results in unnecessary gas consumption.

Proof of Concept

Save as test/poc/ClampUserSumsDeadCode.poc.t.sol and run:

forge test --match-contract ClampUserSumsDeadCodePoC -vv
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
/// @notice Demonstrates that `_clampUserSums(msg.sender)` in `withdraw()` is dead code:
/// it is only ever reached while `riskWindowStart == 0`, where `_clampUserSums`
/// returns at its `start == 0` guard without writing (a guaranteed no-op).
​
contract ClampUserSumsDeadCodePoC is BaseConfidencePoolTest {
/// Fact 1: `withdraw()` reaches the `_clampUserSums` line only while riskWindowStart == 0.
function test_withdrawReachableOnlyWhenRiskWindowZero() public {
_stake(alice, 10 * ONE); // stake while registry is NEW_DEPLOYMENT (pre-risk)
assertEq(pool.riskWindowStart(), 0, "precondition: risk window not opened");
​
uint256 balBefore = token.balanceOf(alice);
_withdraw(alice); // executes the `_clampUserSums(msg.sender)` line
​
// The clamp line just ran with riskWindowStart == 0, so `_clampUserSums` returned at
// `if (start == 0) return;` without touching any per-user sum: a guaranteed no-op.
assertEq(pool.riskWindowStart(), 0, "clamp executed while riskWindowStart == 0");
assertEq(token.balanceOf(alice) - balBefore, 10 * ONE, "full principal returned");
}
​
/// Fact 2: once riskWindowStart != 0, `withdraw()` reverts before reaching the clamp line.
function test_withdrawRevertsOnceRiskWindowOpens() public {
_stake(alice, 10 * ONE);
_passThroughUnderAttack(); // UNDER_ATTACK + poke -> seals riskWindowStart != 0
assertGt(pool.riskWindowStart(), 0, "risk window opened");
​
vm.prank(alice);
vm.expectRevert(); // WithdrawsDisabled — control never reaches `_clampUserSums`
pool.withdraw();
}
}

Recommended Mitigation

To fix the issue, remove the dead _clampUserSums call from withdraw().

function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
​
//enable withdrawals in predeployment, attack requested not live yet or riskWindowStart == 0
if (
riskWindowStart != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
&& state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
) {
revert WithdrawsDisabled();
}
​
uint256 amount = eligibleStake[msg.sender];
if (amount == 0) revert InvalidAmount();
​
- _clampUserSums(msg.sender);
// Withdrawing forfeits the caller's bonus claim: subtract their full contribution from
// the global accumulators so honest stakers' shares aren't diluted by the exiter's
// forfeited weight.
sumStakeTime -= userSumStakeTime[msg.sender];
sumStakeTimeSq -= userSumStakeTimeSq[msg.sender];
​
///code...
}

Support

FAQs

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

Give us feedback!