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

Double Storage Read — `eligibleStake[msg.sender]` via `_clampUserSums` (3 Sites)

Author Revealed upon completion

Root + Impact

Description

In withdraw(), claimSurvived(), and claimExpired(), eligibleStake[msg.sender] is read into a local variable, then _clampUserSums(msg.sender) is called on the next line. Inside _clampUserSums() (line 679), eligibleStake[u] is read from storage a second time for the same address and same slot — a redundant warm SLOAD (~100 gas each). This happens at 3 call sites.

// claimSurvived() lines 386, 389
@> uint256 userEligible = eligibleStake[msg.sender]; // SLOAD #1
// ...
@> _clampUserSums(msg.sender); // SLOAD #2 inside (line 679)
​
// _clampUserSums() line 679
@> uint256 stake_ = eligibleStake[u]; // same slot, same value

Risk

Likelihood: High — every call to withdraw(), claimSurvived(), and claimExpired() incurs the redundant read.

Impact: Low — approximately 100 gas wasted per call (warm SLOAD). No correctness impact.

Proof of Concept

File: G1-DoubleSload-ClampUserSums.poc.t.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
/// @notice PoC for G-1: Double SLOAD of eligibleStake[msg.sender] via _clampUserSums (3 sites)
contract G1_DoubleSload_ClampUserSums_POC is BaseConfidencePoolTest {
function testPOC_G1_doubleSload_inClaimSurvived() external {
_stake(alice, 100 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
// claimSurvived reads eligibleStake[alice] into userEligible (SLOAD #1),
// then calls _clampUserSums(alice) which reads eligibleStake[alice] again (SLOAD #2).
// The value hasn't changed — the second SLOAD is redundant (~100 gas).
uint256 gasBefore = gasleft();
vm.prank(alice);
pool.claimSurvived();
uint256 gasUsed = gasBefore - gasleft();
assertGt(gasUsed, 0, "gas measurement sanity");
}
​
function testPOC_G1_doubleSload_inClaimExpired() external {
_stake(alice, 100 * ONE);
vm.warp(pool.expiry());
​
uint256 gasBefore = gasleft();
vm.prank(alice);
pool.claimExpired();
uint256 gasUsed = gasBefore - gasleft();
assertGt(gasUsed, 0, "gas measurement sanity");
}
​
function testPOC_G1_doubleSload_inWithdraw() external {
_stake(alice, 100 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.NOT_DEPLOYED);
​
uint256 gasBefore = gasleft();
vm.prank(alice);
pool.withdraw();
uint256 gasUsed = gasBefore - gasleft();
assertGt(gasUsed, 0, "gas measurement sanity");
}
}

Run: forge test --match-path 'G1-DoubleSload-ClampUserSums.poc.t.sol' -vv

Recommended Mitigation

Pass the already-read eligibleStake value to _clampUserSums as a second parameter:

- function _clampUserSums(address u) internal {
+ function _clampUserSums(address u, uint256 stake_) internal {
- uint256 stake_ = eligibleStake[u];
​
- _clampUserSums(msg.sender);
+ _clampUserSums(msg.sender, userEligible);

Support

FAQs

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

Give us feedback!