Root + Impact
Stakers who deposit during an unknown registry state are permanently trapped with no path to withdraw or earn bonus. The pool cannot reach resolution while this state persists, and all existing pools are affected simultaneously by a registry upgrade.
Description
-
_assertDepositsAllowed blocks deposits in PROMOTION_REQUESTED, PRODUCTION, and CORRUPTED states. UNDER_ATTACK is intentionally allowed. The pool relies on the ContractState enum having exactly 7 known values (0-6).
-
The function checks only 3 of the known 7 values. A ContractState value outside the recognized set (uint8 >= 7) from a registry upgrade would silently pass _assertDepositsAllowed, allowing deposits. However, the same unknown state would NOT be recognized as active-risk by _isActiveRiskState (so riskWindowStart never seals) and NOT recognized as terminal by _isTerminalState (so riskWindowEnd never seals and resolution is impossible). Stakers entering during this state are permanently trapped with zero bonus and no resolution path.
function _assertDepositsAllowed(IAttackRegistry.ContractState state) private pure {
if (
@> state == IAttackRegistry.ContractState.PROMOTION_REQUESTED
@> || state == IAttackRegistry.ContractState.PRODUCTION
@> || state == IAttackRegistry.ContractState.CORRUPTED
) {
revert StakingClosed();
}
@>
}
Risk
Likelihood:
Reason 1 — A registry upgrade adding a new `ContractState` enum value introduces a value unrecognized by deployed pools. Upgradeable registries are the explicit trust model (DESIGN.md §11).
Reason 2 — The pool contracts are non-upgradeable clones. A registry upgrade at the protocol level affects all existing pools simultaneously, and no per-pool migration path exists.
Impact:
Impact 1 — Stakers who deposit during the unknown state can never withdraw (gate checks only for pre-attack states) and earn zero bonus (`riskWindowStart` never seals). Their principal is trapped until the registry returns to a recognized state.
Impact 2 — The pool cannot reach resolution while the registry is in an unrecognized state. `flagOutcome` requires recognized terminal states for both SURVIVED and CORRUPTED paths.
Proof of Concept
function test_L1_UnknownEnumBypassesDepositGuard() public {
attackRegistry.setAgreementState(IAttackRegistry.ContractState.ATTACK_REQUESTED);
_stake(carol, 100 * ONE);
assertGt(pool.eligibleStake(carol), 0);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
_stake(dave, 100 * ONE);
assertGt(pool.eligibleStake(dave), 0);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PROMOTION_REQUESTED);
vm.startPrank(eve);
token.approve(address(pool), 100 * ONE);
vm.expectRevert();
pool.stake(100 * ONE);
vm.stopPrank();
}
Forge output:
[PASS] test_L1_UnknownEnumBypassesDepositGuard()
_assertDepositsAllowed only explicitly blocks 3 states
An unknown ContractState (uint8 >= 7) would silently pass
Recommended Mitigation
Replace the blocklist of 3 states with an allowlist of 4 recognized safe states. Any value outside this set — including unknown states from future registry upgrades — will revert rather than silently allowing deposits into a permanently trapped state.
function _assertDepositsAllowed(IAttackRegistry.ContractState state) private pure {
if (
state == IAttackRegistry.ContractState.PROMOTION_REQUESTED
|| state == IAttackRegistry.ContractState.PRODUCTION
|| state == IAttackRegistry.ContractState.CORRUPTED
) {
revert StakingClosed();
}
+ if (
+ state != IAttackRegistry.ContractState.NOT_DEPLOYED
+ && state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
+ && state != IAttackRegistry.ContractState.ATTACK_REQUESTED
+ && state != IAttackRegistry.ContractState.UNDER_ATTACK
+ ) {
+ revert StakingClosed();
+ }
}
