Fix: Enforce a Minimum Timelock for Contract Upgrades

[jahrulezfrancis]

Problem

The contract admin can set the upgrade timelock to zero and execute a new
WASM implementation immediately, defeating the purpose of the timelock
safeguard entirely.

Severity: High
Location: contracts/batch-vesting/src/lib.rs:774-782,1243-1277

Description

set_config writes any supplied Config, including upgrade_timelock,
directly to storage via set_config_internal with no minimum-value check.
propose_upgrade computes execute_at = timestamp() + config.upgrade_timelock,
and execute_upgrade only rejects execution if timestamp() < execute_at.
With upgrade_timelock == 0, execute_at equals the proposal timestamp, so
that check never triggers — the same admin can propose and execute a new
WASM hash in immediate succession, with no window for anyone to react to a
compromised or malicious admin.

Steps to Reproduce

1. As the current admin, call set_config with upgrade_timelock: 0.
2. Call propose_upgrade with an arbitrary replacement WASM hash.
3. Call execute_upgrade — it succeeds immediately since
   timestamp() < execute_at is false when they're equal.

Expected Behavior

Every upgrade should retain a non-bypassable minimum review delay,
regardless of what the admin sets in configuration.

Suggested Fix Approach

Enforce a hard-coded minimum upgrade_timelock in set_config (reject
values below it), or make timelock reductions themselves subject to the
existing timelock delay before taking effect.

Acceptance Criteria

• set_config rejects (or the contract otherwise prevents) an
  effective upgrade delay below a defined minimum
• Test confirms execute_upgrade cannot succeed in the same or an
  insufficient interval after propose_upgrade regardless of
  upgrade_timelock value supplied
• Existing upgrade-flow tests still pass

---

[d3vobed]

Hi maintainer, I’d like to take this on. I have experience with Rust smart contracts and security fixes. I’ll enforce a minimum upgrade timelock, add regression tests, and verify the existing upgrade flow still passes.

[gregemax]

Hi, I'd like to work on this issue.
Understanding of the bug: The vulnerability is a missing lower-bound check, not a logic error in the comparison itself. set_config trusts upgrade_timelock as raw input and writes it straight to storage. Since execute_at = timestamp() + upgrade_timelock, setting upgrade_timelock = 0 collapses the review window to nothing — propose_upgrade and execute_upgrade can then be called back-to-back by the same admin, which defeats the entire point of a timelock (giving other stakeholders a chance to react to a malicious or compromised admin).
Root cause is really two-fold:
No floor on upgrade_timelock at config-write time.
Even if there were a floor, a malicious admin could lower it right before proposing an upgrade — so the fix needs to think about when a timelock reduction takes effect, not just whether the value is nonzero.
Proposed approach:
Introduce a hard-coded MIN_UPGRADE_TIMELOCK constant and reject any set_config call that sets upgrade_timelock below it.
Additionally (per the suggested fix), make reductions to upgrade_timelock itself subject to the current timelock before taking effect — so an admin can't lower the delay and immediately exploit the new, weaker value. A config change that increases the timelock can apply immediately; one that decreases it queues and only applies after the existing delay elapses.
Add explicit tests:
set_config with upgrade_timelock: 0 (or any value under the minimum) is rejected.
execute_upgrade fails when called immediately after propose_upgrade, regardless of whatever upgrade_timelock was supplied.
A timelock-reduction attempt (e.g., admin sets a high timelock, then tries to lower it and immediately exploit) is still blocked until the original delay passes.
Confirm existing upgrade-flow tests still pass unmodified.
Plan: Start with failing tests reproducing the exact repro steps in the issue, then implement the minimum + delayed-reduction logic in contracts/batch-vesting/src/lib.rs around set_config_internal, propose_upgrade, and execute_upgrade. I'll open a PR with the diff and a short note on the chosen MIN_UPGRADE_TIMELOCK value and why

[grantfox-oss]

🦊 GrantFox — @gregemax has been assigned to this issue as part of the Official Campaign | FWC26 campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #699)
2. Your PR will be reviewed by the Batch-Pay maintainers

Good luck! Track your progress on GrantFox.

[grantfox-oss]

🎉 This issue has been marked as completed on GrantFox as part of the Official Campaign | FWC26 campaign!

@gregemax's PR #718 was approved and merged by @jahrulezfrancis.

🏆 @gregemax: You earned 35 FoxPoints for this contribution! Your current tier: Explorer (188 total points). Track your full progress on GrantFox.

👏 Great work, @gregemax! Keep contributing to Batch-Pay.