Skip to content

fix(dvm2.0): guard VotingV2 effective-stake subtraction against underflow#4955

Open
tcoatswo wants to merge 3 commits into
UMAprotocol:masterfrom
tcoatswo:codex/harden-votingv2-effective-stake
Open

fix(dvm2.0): guard VotingV2 effective-stake subtraction against underflow#4955
tcoatswo wants to merge 3 commits into
UMAprotocol:masterfrom
tcoatswo:codex/harden-votingv2-effective-stake

Conversation

@tcoatswo

Copy link
Copy Markdown

Summary

The two effectiveStake computations in VotingV2 — in revealVote and in _updateAccountSlashingTrackers — subtract a per-round pendingStakes memo from a voter's current stake:

uint128 effectiveStake = voterStakes[voter].stake - voterStakes[voter].pendingStakes[currentRoundId];
...
uint256 effectiveStake = voterStake.stake - voterStake.pendingStakes[trackers.lastVotingRound];

pendingStakes[roundId] is only ever incremented (Staker._incrementPendingStake) and is never cleared. The subtraction stays non-negative today only because of a non-local invariant spread across staking, slashing, and resolved-request ordering (update-before-mutate in every stake mutation + the monotonic nextIndexToProcess traversal).

Relying on a downstream/global assumption to keep a critical subtraction from underflowing is the same class of mistake as CVE-2018-17144 (the duplicate-input check removed from CheckTransaction because a downstream layer was assumed to make it redundant). If a future change to slashing order, request rolling/deletion, batched processing, or the staking lifecycle ever lets pendingStakes[R] exceed a slashed-down stake while a round-R request is still pending processing, the checked subtraction reverts inside _updateAccountSlashingTrackers. That path is reached by every state-changing entry point for the voter (commitVote, revealVote, updateTrackers, stake, requestUnstake, withdrawRewards), permanently locking their staked principal.

Change

Enforce the safety property locally with a saturating _effectiveStake helper used at both sites:

function _effectiveStake(uint128 stake, uint128 pendingStake) internal pure returns (uint128) {
    return stake > pendingStake ? stake - pendingStake : 0;
}
  • No behavior change on any reachable state: whenever stake >= pendingStake (always true today) the result is identical to the previous subtraction.
  • Anti-inflation: the result is always <= stake, so the weight fed into vote tallying and the slash/reward base can never exceed the voter's actual stake.
  • Fail-closed to the correct value: if the invariant is ever broken the effective stake saturates to 0 (the voter genuinely had no active stake for that round) instead of reverting and locking funds.

Test

Adds packages/core/test/foundry/data-verification-mechanism/VotingV2.EffectiveStakeGuard.t.sol, asserting:

  1. equality with the raw subtraction on the normal path (no regression),
  2. saturation-to-zero instead of revert on the broken-invariant path,
  3. effectiveStake <= stake over a fuzzed input space (anti-inflation bound).
cd packages/core && forge test --match-contract VotingV2EffectiveStakeGuardTest -vv
# 3 tests passed; 0 failed; 0 skipped

Severity

Low / defense-in-depth. No reachable exploit exists at the current HEAD; this hardens a critical subtraction so its safety no longer depends on a non-local invariant.

🤖 Generated with Claude Code

tcoatswo and others added 3 commits June 5, 2026 22:06
Signed-off-by: Tyler Coatsworth <tyler@coatsworth.me>
…flow

The two effectiveStake computations in VotingV2 (revealVote and
_updateAccountSlashingTrackers) subtract a per-round pendingStakes memo from a
voter's current stake. pendingStakes is only ever incremented and never
cleared, so the subtraction's non-negativity is guaranteed only by a non-local
invariant (update-before-mutate ordering + monotonic resolved-request
traversal). Relying on a downstream/global assumption to keep a critical
subtraction safe is the CVE-2018-17144 anti-pattern: if a future change breaks
that invariant the checked subtraction reverts inside _updateTrackers, which is
reached by every state-changing entry point for the voter, permanently locking
their staked principal.

Enforce the property locally with a saturating _effectiveStake helper. The
result is identical to the prior subtraction on every reachable state today,
is always <= stake (so it can never inflate vote weight or the slash/reward
base), and fails closed to 0 (the correct effective participation) instead of
reverting.

Adds VotingV2.EffectiveStakeGuard.t.sol asserting normal-path equality,
saturation-instead-of-revert on the broken-invariant path, and the
result <= stake anti-inflation bound.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant