code/yieldomega

Watch

Fork 0

DoubPresaleVesting: setClaimsEnabled is bidirectional, owner can disable claims mid-vesting and freeze beneficiary DOUB #130

New issue

Closed

opened 2026-05-04 07:58:31 +00:00 by Brouie · 2 comments

Brouie commented

2026-05-04 07:58:31 +00:00

(Migrated from gitlab.com)

found during the post-audit security review pass (followup to #126).

Summary

setClaimsEnabled(bool enabled) (line 130) is bidirectional — owner can flip claims on AND off at any time, including AFTER vesting has started and beneficiaries are mid-claim. Disabling claims mid-vesting freezes beneficiaries' allocations until owner re-enables.

Same shape as #127 (FeeRouter governor DoS via setDistributableToken): privileged toggle that can brick legitimate user flows.

Code

DoubPresaleVesting.sol:130-133:

function setClaimsEnabled(bool enabled) external onlyOwner {
    claimsEnabled = enabled;
    emit ClaimsEnabled(enabled);
}

DoubPresaleVesting.sol:191-192 (claim gate):

if (vestingStart == 0) revert DoubVesting__NotStarted();
if (!claimsEnabled) revert DoubVesting__ClaimsNotEnabled();

Attack / footgun paths

Compromised owner key — adversary calls setClaimsEnabled(false) mid-vesting, all beneficiaries' active claims revert. recovery requires owner key recovery + re-enable tx. value frozen during gap.
Malicious owner exit — owner deliberately disables claims to extract negotiating leverage from beneficiaries.
Operational mistake — multisig signs wrong toggle, flips claims off when intent was on. legitimate beneficiaries can't claim until next signing window.

DOUB stays in the contract during the freeze, but vesting clock keeps ticking. vestedAt continues accruing. When claims re-enable, beneficiaries get larger lump-sum claims (no actual loss) but the freeze is still a real DoS for everyone counting on cashflow timing.

Severity

Medium. privileged DoS, no fund extraction (DOUB stays in contract). audit H-01 covers privileged upgrade/role centralization generically; this is the specific concrete consequence on the user-facing claim path.

Mitigations

One-way ratchet — once claimsEnabled is set true, it stays true. setter reverts on false after first true.

function setClaimsEnabled(bool enabled) external onlyOwner {
    require(enabled, "DoubVesting: claims one-way");
    require(!claimsEnabled, "DoubVesting: already enabled");
    claimsEnabled = true;
    emit ClaimsEnabled(true);
}

Remove the disable path entirely — split into separate enableClaims() and disableClaims() and remove the latter post-deploy.
Add a permanent "frozen" lock — finalizeClaimsEnabled() admin call that locks the current state. operationally awkward but preserves emergency-pause flexibility until lock.

option 1 is cleanest — minimal surface change, preserves the existing setter shape, eliminates the DoS vector.

Pre-deploy relevance

DoubPresaleVesting is in v1 deployment scope (the /vesting route + presale +15% CHARM tie-in is live in the new code dev pushed today). worth a fix or explicit operational policy (governor key cold-stored, only thawed via emergency rotation procedure) before mainnet.

cc @PlasticDigits

found during the post-audit security review pass (followup to #126). ## Summary `setClaimsEnabled(bool enabled)` (line 130) is bidirectional — owner can flip claims on AND off at any time, including AFTER vesting has started and beneficiaries are mid-claim. Disabling claims mid-vesting freezes beneficiaries' allocations until owner re-enables. Same shape as #127 (FeeRouter governor DoS via setDistributableToken): privileged toggle that can brick legitimate user flows. ## Code `DoubPresaleVesting.sol:130-133`: ```solidity function setClaimsEnabled(bool enabled) external onlyOwner { claimsEnabled = enabled; emit ClaimsEnabled(enabled); } ``` `DoubPresaleVesting.sol:191-192` (claim gate): ```solidity if (vestingStart == 0) revert DoubVesting__NotStarted(); if (!claimsEnabled) revert DoubVesting__ClaimsNotEnabled(); ``` ## Attack / footgun paths 1. **Compromised owner key** — adversary calls `setClaimsEnabled(false)` mid-vesting, all beneficiaries' active claims revert. recovery requires owner key recovery + re-enable tx. value frozen during gap. 2. **Malicious owner exit** — owner deliberately disables claims to extract negotiating leverage from beneficiaries. 3. **Operational mistake** — multisig signs wrong toggle, flips claims off when intent was on. legitimate beneficiaries can't claim until next signing window. DOUB stays in the contract during the freeze, but vesting clock keeps ticking. `vestedAt` continues accruing. When claims re-enable, beneficiaries get larger lump-sum claims (no actual loss) but the freeze is still a real DoS for everyone counting on cashflow timing. ## Severity **Medium**. privileged DoS, no fund extraction (DOUB stays in contract). audit H-01 covers privileged upgrade/role centralization generically; this is the specific concrete consequence on the user-facing claim path. ## Mitigations 1. **One-way ratchet** — once `claimsEnabled` is set true, it stays true. setter reverts on `false` after first true. ```solidity function setClaimsEnabled(bool enabled) external onlyOwner { require(enabled, "DoubVesting: claims one-way"); require(!claimsEnabled, "DoubVesting: already enabled"); claimsEnabled = true; emit ClaimsEnabled(true); } ``` 2. **Remove the disable path entirely** — split into separate `enableClaims()` and `disableClaims()` and remove the latter post-deploy. 3. **Add a permanent "frozen" lock** — `finalizeClaimsEnabled()` admin call that locks the current state. operationally awkward but preserves emergency-pause flexibility until lock. option 1 is cleanest — minimal surface change, preserves the existing setter shape, eliminates the DoS vector. ## Pre-deploy relevance DoubPresaleVesting is in v1 deployment scope (the `/vesting` route + presale +15% CHARM tie-in is live in the new code dev pushed today). worth a fix or explicit operational policy (governor key cold-stored, only thawed via emergency rotation procedure) before mainnet. cc @PlasticDigits

Brouie commented

2026-05-04 08:14:03 +00:00

(Migrated from gitlab.com)

mentioned in issue #131

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-05-04 08:27:12 +00:00

PlasticDigits commented

2026-05-04 08:27:13 +00:00

(Migrated from gitlab.com)

Acceptable governance risk, closing

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

code/yieldomega#130

No description provided.

Rows
Columns