Submission Details
Impact:
Likelihood:
Due to the bonus share calculations which do not consider the duration of staking or distance of staking from the `riskWindowEnd`, the last second stakers can almost totally evade the risk and even get more bonus comparing to the early stakers.
Author Revealed upon completion
Root + Impact
Description
-
The current formula for calculating the bonus is disregarding the time that the staking actually happens in the risk window.
-
It is the way that a last second staker, can stake funds just before the staking period is closed. This way, they almost completely evade the risk of losing their funds to the attacker. Moreover, interestingly, they can even get more bonus comparing with users who had staked much earlier (even before the contract was set as
UNDER_ATTACK).
function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
if (amount == 0) revert InvalidAmount();
if (amount < minStake) revert BelowMinStake();
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
if (block.timestamp >= expiry) revert StakingClosed();
_assertDepositsAllowed(_observePoolState());
if (!expiryLocked) {
expiryLocked = true;
}
// Balance-diff defense-in-depth against the factory allowlist admitting a fee-on-transfer
// or rebasing token (governance error, or a proxy-token upgrade post-allowlist).
// aderyn-fp-next-line(reentrancy-state-change)
uint256 balanceBefore = stakeToken.balanceOf(address(this));
stakeToken.safeTransferFrom(msg.sender, address(this), amount);
// aderyn-fp-next-line(reentrancy-state-change)
uint256 balanceAfter = stakeToken.balanceOf(address(this));
uint256 received = balanceAfter > balanceBefore ? balanceAfter - balanceBefore : 0;
if (received == 0) revert NoTokensReceived();
if (received < minStake) revert BelowMinStake();
_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;
eligibleStake[msg.sender] += received;
@> userSumStakeTime[msg.sender] += contribTime;
@> userSumStakeTimeSq[msg.sender] += contribTimeSq;
totalEligibleStake += received;
sumStakeTime += contribTime;
sumStakeTimeSq += contribTimeSq;
emit Staked(msg.sender, received);
}
function _bonusShare(address u, uint256 userEligible) internal view returns (uint256) {
if (snapshotTotalBonus == 0) return 0;
// No observable risk → no bonus (see contract natspec).
if (riskWindowStart == 0) return 0;
uint256 T = outcomeFlaggedAt;
// Underflow guards on both subtractions: globally the sum of squares is nonneg, but
// truncation/rounding pathologies could push individual terms over.
@> uint256 userPlus = T * T * userEligible + userSumStakeTimeSq[u];
@> uint256 userMinus = 2 * T * userSumStakeTime[u];
@> uint256 userScore = userPlus > userMinus ? userPlus - userMinus : 0;
uint256 plus = T * T * snapshotTotalStaked + snapshotSumStakeTimeSq;
uint256 minus = 2 * T * snapshotSumStakeTime;
uint256 globalScore = plus > minus ? plus - minus : 0;
if (globalScore == 0) {
// No time elapsed in the risk window for anyone → fallback to amount-weighted.
if (snapshotTotalStaked == 0) return 0;
return Math.mulDiv(userEligible, snapshotTotalBonus, snapshotTotalStaked);
}
// mulDiv handles the final multiply-then-divide via 512-bit intermediates, so a very
// large `snapshotTotalBonus` cannot push the numerator over uint256 before division.
return Math.mulDiv(userScore, snapshotTotalBonus, globalScore);
}
Risk
Likelihood:
-
Since the contract will be verified and the source code will be available to everyone, any skillful staker can see the flawed calculations and benefit from this opportunity.
-
This is as easy as writing a smart contract and set it up to stake some funds at a specific moment in time.
Impact:
It breaks the state of fairness of the protocol. Honest users who stake early and accept the possible risk of losing their funds to provide incentives for the security researchers and white-hat hackers, get less bonus than opportunistic dishonest users who evade the risk and gain more bonus.
Proof of Concept
Please put the following code in a file like ConfidencePoolContest.t.sol and then run the test using forge test --mt test_claimSurvived_lastSecondStakersCanEvadeRiskAndEvenGetMoreBonus -vvv.
// SPDX-License-Identifier: MIT
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {stdStorage, StdStorage} from "forge-std/StdStorage.sol";
import {ConfidencePool} from "src/ConfidencePool.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
import {
IERC20Errors
} from "lib/openzeppelin-contracts/contracts/interfaces/draft-IERC6093.sol";
import {console} from "forge-std/console.sol";
contract ConfidencePoolContestTest is BaseConfidencePoolTest {
using stdStorage for StdStorage;
event AttackerBountyClaimed(
address indexed attacker,
uint256 amount,
uint256 totalClaimed,
uint256 totalEntitlement
);
function test_claimSurvived_lastSecondStakersCanEvadeRiskAndEvenGetMoreBonus()
external
{
token.mint(carol, ONE);
vm.startPrank(carol);
token.approve(address(pool), ONE);
pool.contributeBonus(ONE);
vm.stopPrank();
token.mint(alice, ONE);
vm.startPrank(alice);
token.approve(address(pool), ONE);
pool.stake(ONE);
vm.stopPrank();
_passThroughUnderAttack();
vm.warp(pool.expiry() - 1);
token.mint(bob, ONE);
vm.startPrank(bob);
token.approve(address(pool), ONE);
pool.stake(ONE);
vm.stopPrank();
vm.warp(1);
attackRegistry.setAgreementState(
IAttackRegistry.ContractState.PRODUCTION
);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
uint256 aliceBalanceBefore = token.balanceOf(alice);
vm.startPrank(alice);
pool.claimSurvived();
vm.stopPrank();
uint256 aliceBalanceAfter = token.balanceOf(alice);
uint256 bobBalanceBefore = token.balanceOf(bob);
vm.startPrank(bob);
pool.claimSurvived();
vm.stopPrank();
uint256 bobBalanceAfter = token.balanceOf(bob);
uint256 aliceBonus = aliceBalanceAfter - aliceBalanceBefore;
uint256 bobBonus = bobBalanceAfter - bobBalanceBefore;
console.log("Alice bonus:", aliceBonus);
console.log("Bob bonus:", bobBonus);
assert(bobBonus > aliceBonus);
}
}
Recommended Mitigation
I recommend using the distance from the expiry time as the newEntry in the stake function for time-based calculations.
function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
if (amount == 0) revert InvalidAmount();
if (amount < minStake) revert BelowMinStake();
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
if (block.timestamp >= expiry) revert StakingClosed();
_assertDepositsAllowed(_observePoolState());
if (!expiryLocked) {
expiryLocked = true;
}
// Balance-diff defense-in-depth against the factory allowlist admitting a fee-on-transfer
// or rebasing token (governance error, or a proxy-token upgrade post-allowlist).
// aderyn-fp-next-line(reentrancy-state-change)
uint256 balanceBefore = stakeToken.balanceOf(address(this));
stakeToken.safeTransferFrom(msg.sender, address(this), amount);
// aderyn-fp-next-line(reentrancy-state-change)
uint256 balanceAfter = stakeToken.balanceOf(address(this));
uint256 received = balanceAfter > balanceBefore ? balanceAfter - balanceBefore : 0;
if (received == 0) revert NoTokensReceived();
if (received < minStake) revert BelowMinStake();
_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;
+ newEntry = expiry - newEntry;
uint256 contribTime = received * newEntry;
uint256 contribTimeSq = received * newEntry * newEntry;
eligibleStake[msg.sender] += received;
userSumStakeTime[msg.sender] += contribTime;
userSumStakeTimeSq[msg.sender] += contribTimeSq;
totalEligibleStake += received;
sumStakeTime += contribTime;
sumStakeTimeSq += contribTimeSq;
emit Staked(msg.sender, received);
}
Prize pool breakdown
Total prize
7.25 ETH
nSLOC
5890
ETH / LOC
6
1.25
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.
Rewards Distribution
The contest is complete and the rewards are being distributed.