Skip to content

fix: cap cool_down_period_s to prevent unstake DoS and add missing event#41

Open
Kukrushka wants to merge 1 commit into
orca-so:mainfrom
Kukrushka:fix/cooldown-overflow-and-missing-event
Open

fix: cap cool_down_period_s to prevent unstake DoS and add missing event#41
Kukrushka wants to merge 1 commit into
orca-so:mainfrom
Kukrushka:fix/cooldown-overflow-and-missing-event

Conversation

@Kukrushka

Copy link
Copy Markdown

Summary

Fixes #39.

Set(UpdateCoolDownPeriod) accepted any non-negative i64 value, including i64::MAX. In unstake, the program computes:

let withdrawable_timestamp = current_unix_timestamp
    .checked_add(state.cool_down_period_s)
    .ok_or(ErrorCode::CoolDownOverflow)?;

With a large enough cooldown (e.g. i64::MAX), this addition overflows and every unstake call returns CoolDownOverflow — permanently preventing all users from unstaking their tokens (DoS on withdrawal).

Changes

instructions/set.rs — add upper-bound guard alongside the existing negative-value check:

const MAX_COOL_DOWN_PERIOD_S: i64 = 10 * 365 * 24 * 60 * 60; // ~315_360_000 s (10 years)
if *new_cool_down_period_s < 0 || *new_cool_down_period_s > MAX_COOL_DOWN_PERIOD_S {
    return Err(ErrorCode::InvalidCoolDownPeriod.into());
}

event.rs — add CoolDownPeriodUpdated event so off-chain monitors can detect authority changes to the cooldown without polling state. Previously UpdateCoolDownPeriod was the only admin instruction that emitted no event (inconsistent with UpdateUpdateAuthority).

Test plan

  • Verify Set(UpdateCoolDownPeriod { i64::MAX }) now returns InvalidCoolDownPeriod
  • Verify Set(UpdateCoolDownPeriod { MAX_COOL_DOWN_PERIOD_S }) succeeds
  • Verify Set(UpdateCoolDownPeriod { MAX_COOL_DOWN_PERIOD_S + 1 }) returns InvalidCoolDownPeriod
  • Verify CoolDownPeriodUpdated event is emitted on successful update

🤖 Generated with Claude Code

SetUpdateCoolDownPeriod accepted any non-negative i64, including i64::MAX.
When unstake later computed `current_unix_timestamp.checked_add(cool_down_period_s)`
the addition overflowed, returning CoolDownOverflow for every caller — permanently
bricking unstake for all users (issue orca-so#39).

Fix:
- Reject values above MAX_COOL_DOWN_PERIOD_S (10 years, ~315_360_000 s) in
  set.rs alongside the existing negative-value guard.
- Emit a new CoolDownPeriodUpdated event so off-chain monitors can detect
  authority changes to the cooldown without polling state directly.

Co-Authored-By: Claude Sonnet 4.6 <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.

Set(UpdateCoolDownPerio) can brick future unstake calls by allowing overflow prone cooldown values

1 participant