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

Unreachable code in `ConfidencePool.stake()`

Author Revealed upon completion

Description

  • ConfidencePool.stake() records each deposit with an entry timestamp. Pre-risk deposits are later promoted to riskWindowStart by _clampUserSums(), while deposits made after the risk window has opened should use the current block.timestamp directly.

  • The stake() function contains a defensive floor that attempts to move newEntry forward to riskWindowStart when newEntry < start. That branch is dead code. stake() observes the registry before newEntry is computed, so the first active-risk stake can only set riskWindowStart to the same block timestamp. Later stakes happen at timestamps greater than or equal to the already stored riskWindowStart.

function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
...
_assertDepositsAllowed(_observePoolState());
...
_clampUserSums(msg.sender);
​
// Pre-risk deposits use wall clock and get promoted to riskWindowStart later via
// `_clampUserSums`; post-risk deposits go in at wall clock (already >= riskWindowStart).
uint256 newEntry = block.timestamp;
uint256 start = riskWindowStart;
@> if (start != 0 && newEntry < start) newEntry = start;
​
uint256 contribTime = received * newEntry;
uint256 contribTimeSq = received * newEntry * newEntry;
...
}
​
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
...
@> if (riskWindowStart == 0 && _isActiveRiskState(state)) {
@> _markRiskWindowStart();
@> }
...
}
​
function _markRiskWindowStart() internal {
uint256 t = block.timestamp;
if (t > expiry) t = expiry;
@> riskWindowStart = uint32(t);
...
}

Risk

Likelihood:

  • The unreachable check is evaluated whenever a stake is recorded while riskWindowStart is nonzero.

  • On the first active-risk stake, _observePoolState() sets riskWindowStart from the current block.timestamp before newEntry is loaded, making newEntry == start.

  • On every later stake, normal timestamp monotonicity makes the current block.timestamp greater than or equal to the stored riskWindowStart.

Impact:

  • No funds are at risk. Just a branch with dead code logic which is never reachable.

  • The dead branch makes the entry-time logic look more complex than it is, which can slow future review and maintenance.

Proof of Concept

  • Copy paste the given test in the ConfidencePool.riskWindow.t.sol to run it

function testPOCStakeEntryFloorBranchIsUnreachable() external {
// `if (start != 0 && newEntry < start) newEntry = start;`
//
// `stake()` observes the registry before computing `newEntry`. If the call opens the
// risk window, `riskWindowStart` is set to the current block timestamp, so `newEntry ==
// start`. If the window was already open, normal chain chronology means
// `block.timestamp >= riskWindowStart`. In neither nonzero-start path can
// `newEntry < start` hold.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
​
uint256 firstStakeTs = vm.getBlockTimestamp();
_stake(alice, 100 * ONE);
​
assertEq(pool.riskWindowStart(), firstStakeTs, "first active stake opens at current timestamp");
assertEq(pool.userSumStakeTime(alice), 100 * ONE * firstStakeTs, "entry is not floored forward");
assertEq(pool.sumStakeTime(), 100 * ONE * firstStakeTs, "global entry uses current timestamp");
​
vm.warp(firstStakeTs + 1 days);
uint256 secondStakeTs = vm.getBlockTimestamp();
_stake(bob, 100 * ONE);
​
assertGt(secondStakeTs, pool.riskWindowStart(), "later stake is after risk start");
assertEq(pool.userSumStakeTime(bob), 100 * ONE * secondStakeTs, "later entry is current timestamp");
assertEq(
pool.sumStakeTime(),
100 * ONE * firstStakeTs + 100 * ONE * secondStakeTs,
"global sum proves the floor branch did not alter either entry"
);
}

Recommended Mitigation

Remove the unreachable floor and keep the entry timestamp as the current block timestamp.

uint256 newEntry = block.timestamp;
-uint256 start = riskWindowStart;
-if (start != 0 && newEntry < start) newEntry = start;
uint256 contribTime = received * newEntry;
uint256 contribTimeSq = received * newEntry * newEntry;

Support

FAQs

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

Give us feedback!