Pre-deploy security review: 7 contract findings (Doubloon burn god-mode + 5 WarBow / sale-flow gaps + DoubPresaleVesting rescue gap) #131

Closed
opened 2026-05-04 08:14:01 +00:00 by Brouie · 14 comments
Brouie commented 2026-05-04 08:14:01 +00:00 (Migrated from gitlab.com)

second-pass security review using Opus 4.7 with 1M context against full contracts/src tree. all findings verified against source by hand. dedup'd against your audit MD, my prior #126/#127/#128/#129/#130, and your already-filed batch #116-125. these are NEW.

ordered by severity descending.


Finding 1 — Doubloon: MINTER_ROLE is also unrestricted BURNER (Medium)

Doubloon.sol:20:

function burn(address from, uint256 amount) external onlyRole(MINTER_ROLE) {
    _burn(from, amount);
}

no allowance check from from. anyone with MINTER_ROLE can destroy any wallet's DOUB unilaterally. this is non-standard ERC20 burn semantics — OZ's ERC20Burnable.burnFrom requires per-account allowance, this one doesn't.

today only RabbitTreasury holds MINTER_ROLE and only burns msg.sender, so live behavior is safe. but the audit's H-01 narrative frames the danger as mint inflation; it does not capture that the same role grant is also a god-mode burn switch over every DOUB holder. a future migration contract, a subsequent Burrow vault, a buggy contract that gets MINTER_ROLE, or a multisig compromise can wipe arbitrary user balances.

mitigation: split into MINTER_ROLE and BURNER_ROLE, restrict BURNER_ROLE to RabbitTreasury (no further grants). or change burn(from, amount) to require allowance from from (delegate to _spendAllowance then _burn). add unit test asserting MINTER_ROLE alone cannot burn an account that did not approve.


Finding 2 — TimeCurve: setBuyFeeRoutingEnabled mid-sale freeze locks in podium leaders (Low–Medium)

TimeCurve.sol:287-290 (setter), TimeCurve.sol:413/574/628/655 (gate sites), TimeCurve.sol:555 (claimWarBowFlag does NOT gate on this — line 321 docstring confirms it's intentional).

owner can call setBuyFeeRoutingEnabled(false) mid-sale. this single toggle:

  • blocks all _buy (no charm purchases, no timer extension)
  • blocks warbowSteal, warbowRevenge, warbowActivateGuard (no defense for victims)
  • does NOT block claimWarBowFlag — existing flag holders can still claim BP

attack path: compromised or self-dealing owner flips false at the moment a desired podium configuration is in place. no one can outbid for last-buyer / WarBow / time-booster / defended-streak. sale auto-expires within initialTimerSec. owner-aligned addresses pocket the four reserve-pool slices.

audit H-01 covers admin power generically via upgrade/role talk. this is the sharper concrete consequence: one toggle, no upgrade required, much easier to execute.

mitigation: gate the setter behind on-chain delay (e.g. require(block.timestamp >= flipScheduledAt + 6h)). or restrict setBuyFeeRoutingEnabled(false) to pre-saleStart and post-endSale only. document this switch as incident-response, not tuning.


Finding 3 — TimeCurve: distributePrizes silent return on prizePool == 0 leaves prizesDistributed stuck false (Low)

TimeCurve.sol:743-746:

uint256 prizePool = acceptedAsset.balanceOf(address(podiumPool));
if (prizePool == 0) {
    return;
}

the early return:

  • does not set prizesDistributed = true
  • does not emit PrizesDistributed
  • bypasses the reservePodiumPayoutsEnabled gate

consequences:

  1. rounds where FeeRouter had PodiumPool weight set to 0 leave prizesDistributed false permanently, with no on-chain signal. indexers waiting for PrizesDistributed (canonical 'round closed' signal per docs/operations/final-signoff-and-value-movement.md) hang forever.
  2. anyone donating CL8Y to the PodiumPool address AFTER the sale ends would let owner re-trigger distributePrizes and route that donation through the four-category split. follow-on griefing or laundering channel since the function is owner-only but the route is reusable.

mitigation: after the early-return condition unconditionally set prizesDistributed = true and emit PrizesDistributed once endSale has run. if continued accept-funding is desired, gate on a separate explicit owner action.


Finding 4 — TimeCurve: warbowSteal 2x rule fails open for fresh-wallet attackers (Low)

TimeCurve.sol:596:

require(vbp > 0 && vbp >= 2 * abp, "TimeCurve: steal 2x rule");

when attacker's abp == 0, the condition reduces to vbp > 0. any fresh wallet can steal from any BP-positive victim. the '2x outranks' framing is meaningless for fresh wallets.

sybil sweep: attacker spawns N fresh wallets. each pays WARBOW_STEAL_BURN_WAD (1 CL8Y) and takes 10% of victim's BP. per-victim daily cap is 3 (line 605-607). attacker drains ~27.1% of victim's BP for 3 CL8Y. then a 4th wallet pays 50 CL8Y for another 10% via bypass burn. with sybil wallets coordinating on a high-BP target during closing phase, attacker reliably crashes them off the WarBow podium for ~3 CL8Y while the WarBow prize slice (25% of podium pool) likely dwarfs that cost.

mitigation: change rule to use cumulative BP earned (not current BP) or require abp > 0 && vbp >= 2 * abp. or require a minimum number of buys (buyCount[msg.sender] >= K) before warbowSteal is callable.


Finding 5 — TimeCurve: revenge slot unconditional overwrite enables cheap collusion shielding (Low)

TimeCurve.sol:609-610:

warbowPendingRevengeStealer[victim] = msg.sender;
warbowPendingRevengeExpiry[victim] = block.timestamp + WARBOW_REVENGE_WINDOW_SEC;

each new stealer overwrites the prior one's slot.

collusion shielding attack: attacker A steals from V at t=0. confederate B steals from V at t=0+1s for cost of WARBOW_STEAL_BURN_WAD (1 CL8Y). V's revenge slot now points at B; A is shielded permanently. A and B rotate to permanently shield each other for ~1 CL8Y per cover. fully neutralizes the documented revenge mechanic for the cost of a few CL8Y per round.

mitigation: keep a list (e.g. mapping(victim => address[]) capped at small N) of recent stealers eligible for revenge. or use per-stealer revenge slot keyed by (victim, stealer) so revenge against any stealer within the 24h window stays valid until consumed.


Finding 6 — TimeCurve: WarBow paths missing block.timestamp < deadline check (Low)

TimeCurve.sol:555 (claimWarBowFlag), 570 (warbowSteal), 624 (warbowRevenge), 651 (warbowActivateGuard) — all four check !ended but none check block.timestamp < deadline. endSale (line 710+) is permissionless but unprivileged callers may delay calling it.

in the post-deadline pre-endSale window, BP and flag bonus can still mutate, then endSale snapshots the resulting state into prize-distribution computations.

attack: a wallet leading WarBow at t == deadline is exposed to up to 3 fresh-wallet steals (compounds with Finding 4) before someone calls endSale. conversely an attacker who knows they are off-podium can race with claimWarBowFlag (no deadline check, only WARBOW_FLAG_SILENCE_SEC since their flag plant) to mint themselves +1000 BP before endSale, potentially flipping the WarBow podium.

mitigation: add require(block.timestamp < deadline) to all four functions. or auto-finalize by calling _finalizeLastBuyers and setting ended inside the first _buy or claimWarBowFlag that observes block.timestamp >= deadline.


Finding 7 — DoubPresaleVesting: no rescue path for excess DOUB or unrelated tokens (Low)

contracts/src/vesting/DoubPresaleVesting.sol — no rescueERC20 or sweep function. claim only pays out to beneficiaries up to allocationOf[b], totals are immutable post-init.

locked categories:

  • DOUB transferred into the contract beyond totalAllocated — claims cap at allocation, vesting state cannot accept new beneficiaries. unrecoverable absent UUPS upgrade.
  • any non-DOUB ERC20 sent to this contract — same lock.

audit's vesting section says 'no unprivileged asset extraction path was found' but does not flag the asymmetric lock on excess deposits.

same shape as my #128 (TimeCurve unredeemed launchedToken stranding) — different contract, same locked-funds pattern.

mitigation: add owner-gated rescueERC20(IERC20 token, address to, uint256 amount). for token == DOUB, refuse to transfer beyond bal - max(0, totalAllocated - sumClaimed). for any other token, unconditional. emit RescueERC20 event for indexer visibility.


Pre-deploy relevance

all 7 findings are in v1 deployment scope (Doubloon, TimeCurve, DoubPresaleVesting all ship tonight). severity ranges Low to Medium — none are immediate launch-blockers individually, but Finding 1 + Finding 2 + Finding 4-5-6 each have clear attack paths that materially change game-theoretic balance. worth a triage pass before mainnet.

cc @PlasticDigits

second-pass security review using Opus 4.7 with 1M context against full contracts/src tree. all findings verified against source by hand. dedup'd against your audit MD, my prior #126/#127/#128/#129/#130, and your already-filed batch #116-125. these are NEW. ordered by severity descending. --- ## Finding 1 — Doubloon: MINTER_ROLE is also unrestricted BURNER (Medium) `Doubloon.sol:20`: ```solidity function burn(address from, uint256 amount) external onlyRole(MINTER_ROLE) { _burn(from, amount); } ``` no allowance check from `from`. anyone with MINTER_ROLE can destroy any wallet's DOUB unilaterally. this is non-standard ERC20 burn semantics — OZ's `ERC20Burnable.burnFrom` requires per-account allowance, this one doesn't. today only RabbitTreasury holds MINTER_ROLE and only burns msg.sender, so live behavior is safe. but the audit's H-01 narrative frames the danger as mint inflation; it does not capture that the same role grant is also a god-mode burn switch over every DOUB holder. a future migration contract, a subsequent Burrow vault, a buggy contract that gets MINTER_ROLE, or a multisig compromise can wipe arbitrary user balances. **mitigation**: split into MINTER_ROLE and BURNER_ROLE, restrict BURNER_ROLE to RabbitTreasury (no further grants). or change `burn(from, amount)` to require allowance from from (delegate to `_spendAllowance` then `_burn`). add unit test asserting MINTER_ROLE alone cannot burn an account that did not approve. --- ## Finding 2 — TimeCurve: setBuyFeeRoutingEnabled mid-sale freeze locks in podium leaders (Low–Medium) `TimeCurve.sol:287-290` (setter), `TimeCurve.sol:413/574/628/655` (gate sites), `TimeCurve.sol:555` (claimWarBowFlag does NOT gate on this — line 321 docstring confirms it's intentional). owner can call `setBuyFeeRoutingEnabled(false)` mid-sale. this single toggle: - blocks all `_buy` (no charm purchases, no timer extension) - blocks `warbowSteal`, `warbowRevenge`, `warbowActivateGuard` (no defense for victims) - does NOT block `claimWarBowFlag` — existing flag holders can still claim BP **attack path**: compromised or self-dealing owner flips false at the moment a desired podium configuration is in place. no one can outbid for last-buyer / WarBow / time-booster / defended-streak. sale auto-expires within `initialTimerSec`. owner-aligned addresses pocket the four reserve-pool slices. audit H-01 covers admin power generically via upgrade/role talk. this is the sharper concrete consequence: one toggle, no upgrade required, much easier to execute. **mitigation**: gate the setter behind on-chain delay (e.g. `require(block.timestamp >= flipScheduledAt + 6h)`). or restrict `setBuyFeeRoutingEnabled(false)` to pre-saleStart and post-endSale only. document this switch as incident-response, not tuning. --- ## Finding 3 — TimeCurve: distributePrizes silent return on prizePool == 0 leaves prizesDistributed stuck false (Low) `TimeCurve.sol:743-746`: ```solidity uint256 prizePool = acceptedAsset.balanceOf(address(podiumPool)); if (prizePool == 0) { return; } ``` the early return: - does not set `prizesDistributed = true` - does not emit `PrizesDistributed` - bypasses the `reservePodiumPayoutsEnabled` gate **consequences**: 1. rounds where FeeRouter had PodiumPool weight set to 0 leave `prizesDistributed` false permanently, with no on-chain signal. indexers waiting for `PrizesDistributed` (canonical 'round closed' signal per `docs/operations/final-signoff-and-value-movement.md`) hang forever. 2. anyone donating CL8Y to the PodiumPool address AFTER the sale ends would let owner re-trigger `distributePrizes` and route that donation through the four-category split. follow-on griefing or laundering channel since the function is owner-only but the route is reusable. **mitigation**: after the early-return condition unconditionally set `prizesDistributed = true` and emit `PrizesDistributed` once `endSale` has run. if continued accept-funding is desired, gate on a separate explicit owner action. --- ## Finding 4 — TimeCurve: warbowSteal 2x rule fails open for fresh-wallet attackers (Low) `TimeCurve.sol:596`: ```solidity require(vbp > 0 && vbp >= 2 * abp, "TimeCurve: steal 2x rule"); ``` when attacker's `abp == 0`, the condition reduces to `vbp > 0`. any fresh wallet can steal from any BP-positive victim. the '2x outranks' framing is meaningless for fresh wallets. **sybil sweep**: attacker spawns N fresh wallets. each pays `WARBOW_STEAL_BURN_WAD` (1 CL8Y) and takes 10% of victim's BP. per-victim daily cap is 3 (line 605-607). attacker drains ~27.1% of victim's BP for 3 CL8Y. then a 4th wallet pays 50 CL8Y for another 10% via bypass burn. with sybil wallets coordinating on a high-BP target during closing phase, attacker reliably crashes them off the WarBow podium for ~3 CL8Y while the WarBow prize slice (25% of podium pool) likely dwarfs that cost. **mitigation**: change rule to use cumulative BP earned (not current BP) or require `abp > 0 && vbp >= 2 * abp`. or require a minimum number of buys (`buyCount[msg.sender] >= K`) before `warbowSteal` is callable. --- ## Finding 5 — TimeCurve: revenge slot unconditional overwrite enables cheap collusion shielding (Low) `TimeCurve.sol:609-610`: ```solidity warbowPendingRevengeStealer[victim] = msg.sender; warbowPendingRevengeExpiry[victim] = block.timestamp + WARBOW_REVENGE_WINDOW_SEC; ``` each new stealer overwrites the prior one's slot. **collusion shielding attack**: attacker A steals from V at t=0. confederate B steals from V at t=0+1s for cost of `WARBOW_STEAL_BURN_WAD` (1 CL8Y). V's revenge slot now points at B; A is shielded permanently. A and B rotate to permanently shield each other for ~1 CL8Y per cover. fully neutralizes the documented revenge mechanic for the cost of a few CL8Y per round. **mitigation**: keep a list (e.g. `mapping(victim => address[])` capped at small N) of recent stealers eligible for revenge. or use per-stealer revenge slot keyed by `(victim, stealer)` so revenge against any stealer within the 24h window stays valid until consumed. --- ## Finding 6 — TimeCurve: WarBow paths missing block.timestamp < deadline check (Low) `TimeCurve.sol:555` (claimWarBowFlag), `570` (warbowSteal), `624` (warbowRevenge), `651` (warbowActivateGuard) — all four check `!ended` but none check `block.timestamp < deadline`. `endSale` (line 710+) is permissionless but unprivileged callers may delay calling it. in the post-deadline pre-endSale window, BP and flag bonus can still mutate, then `endSale` snapshots the resulting state into prize-distribution computations. **attack**: a wallet leading WarBow at `t == deadline` is exposed to up to 3 fresh-wallet steals (compounds with Finding 4) before someone calls `endSale`. conversely an attacker who knows they are off-podium can race with `claimWarBowFlag` (no deadline check, only WARBOW_FLAG_SILENCE_SEC since their flag plant) to mint themselves +1000 BP before `endSale`, potentially flipping the WarBow podium. **mitigation**: add `require(block.timestamp < deadline)` to all four functions. or auto-finalize by calling `_finalizeLastBuyers` and setting `ended` inside the first `_buy` or `claimWarBowFlag` that observes `block.timestamp >= deadline`. --- ## Finding 7 — DoubPresaleVesting: no rescue path for excess DOUB or unrelated tokens (Low) `contracts/src/vesting/DoubPresaleVesting.sol` — no `rescueERC20` or sweep function. `claim` only pays out to beneficiaries up to `allocationOf[b]`, totals are immutable post-init. **locked categories**: - DOUB transferred into the contract beyond `totalAllocated` — claims cap at allocation, vesting state cannot accept new beneficiaries. unrecoverable absent UUPS upgrade. - any non-DOUB ERC20 sent to this contract — same lock. audit's vesting section says 'no unprivileged asset extraction path was found' but does not flag the asymmetric lock on excess deposits. same shape as my #128 (TimeCurve unredeemed launchedToken stranding) — different contract, same locked-funds pattern. **mitigation**: add owner-gated `rescueERC20(IERC20 token, address to, uint256 amount)`. for token == DOUB, refuse to transfer beyond `bal - max(0, totalAllocated - sumClaimed)`. for any other token, unconditional. emit `RescueERC20` event for indexer visibility. --- ## Pre-deploy relevance all 7 findings are in v1 deployment scope (Doubloon, TimeCurve, DoubPresaleVesting all ship tonight). severity ranges Low to Medium — none are immediate launch-blockers individually, but Finding 1 + Finding 2 + Finding 4-5-6 each have clear attack paths that materially change game-theoretic balance. worth a triage pass before mainnet. cc @PlasticDigits
PlasticDigits commented 2026-05-04 08:34:54 +00:00 (Migrated from gitlab.com)

Finding 2 - No change needed, acceptable governance risk
Finding 4 - The override 50 cl8y should be per wallet, not global. Document that as an invariant. Additionally, attacks should require at least 10% of the defender's BP.
Finding 5 use per stealer revenge slot with a nonce. Make sure the indexer is updated and frontend updated to show ALL eligible revenges not just one, And tests should cover cases up to 7 different stealers at once with fuzzing

Finding 2 - No change needed, acceptable governance risk Finding 4 - The override 50 cl8y should be per wallet, not global. Document that as an invariant. Additionally, attacks should require at least 10% of the defender's BP. Finding 5 use per stealer revenge slot with a nonce. Make sure the indexer is updated and frontend updated to show ALL eligible revenges not just one, And tests should cover cases up to 7 different stealers at once with fuzzing
PlasticDigits commented 2026-05-04 08:35:57 +00:00 (Migrated from gitlab.com)

mentioned in issue #132

mentioned in issue #132
PlasticDigits commented 2026-05-04 08:36:06 +00:00 (Migrated from gitlab.com)

mentioned in issue #133

mentioned in issue #133
PlasticDigits commented 2026-05-04 08:36:09 +00:00 (Migrated from gitlab.com)

mentioned in issue #134

mentioned in issue #134
PlasticDigits commented 2026-05-04 08:36:13 +00:00 (Migrated from gitlab.com)

mentioned in issue #135

mentioned in issue #135
PlasticDigits commented 2026-05-04 08:36:25 +00:00 (Migrated from gitlab.com)

mentioned in issue #136

mentioned in issue #136
PlasticDigits commented 2026-05-04 08:36:29 +00:00 (Migrated from gitlab.com)

mentioned in issue #137

mentioned in issue #137
PlasticDigits commented 2026-05-04 08:36:45 +00:00 (Migrated from gitlab.com)

Triaged → child issues

Finding 2 — No separate issue: acceptable governance risk ([comment](note link)).

Per maintainer/product notes where applicable (#134 stealth bypass per-wallet + ≥10% defender BP; #135 per-stealer revenge + nonce, indexer + UI list all eligible targets, fuzz ≥7 stealers).

Child issues opened with problem statement, recommended fix, acceptance criteria, verification checklist, and repo touch matrix:

  • Finding 1 (Doubloon MINTER / burn) → #132
  • Finding 3 (distributePrizes zero pool) → #133
  • Finding 4 (WarBow steal sybil / 2× / bypass) → #134
  • Finding 5 (WarBow revenge overwrite / collusion) → #135
  • Finding 6 (WarBow deadline vs ended gap) → #136
  • Finding 7 (DoubPresaleVesting rescue) → #137

Closing this umbrella tracker; implementation discussion should move to the scoped issues.

## Triaged → child issues **Finding 2** — No separate issue: acceptable governance risk ([comment](note link)). Per maintainer/product notes where applicable (**#134** stealth bypass **per-wallet** + **≥10% defender BP**; **#135** **per-stealer revenge + nonce**, indexer + UI list **all** eligible targets, fuzz **≥7 stealers**). Child issues opened with problem statement, recommended fix, acceptance criteria, verification checklist, and repo touch matrix: - **Finding 1 (Doubloon MINTER / burn)** → #132 - **Finding 3 (distributePrizes zero pool)** → #133 - **Finding 4 (WarBow steal sybil / 2× / bypass)** → #134 - **Finding 5 (WarBow revenge overwrite / collusion)** → #135 - **Finding 6 (WarBow deadline vs ended gap)** → #136 - **Finding 7 (DoubPresaleVesting rescue)** → #137 Closing this umbrella tracker; implementation discussion should move to the scoped issues.
PlasticDigits commented 2026-05-04 08:36:54 +00:00 (Migrated from gitlab.com)

Correction: Finding 2 has no child issue — maintainer decision: acceptable governance risk (see discussion above). Ignore the broken link fragment in the prior automation note.

**Correction:** Finding 2 has **no child issue** — maintainer decision: acceptable governance risk (see discussion above). Ignore the broken link fragment in the prior automation note.
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-05-04 08:36:54 +00:00
Brouie commented 2026-05-04 08:44:27 +00:00 (Migrated from gitlab.com)

mentioned in issue #138

mentioned in issue #138
PlasticDigits commented 2026-05-04 08:51:59 +00:00 (Migrated from gitlab.com)

mentioned in issue #141

mentioned in issue #141
PlasticDigits commented 2026-05-04 08:52:44 +00:00 (Migrated from gitlab.com)

mentioned in issue #146

mentioned in issue #146
Brouie commented 2026-05-05 02:21:46 +00:00 (Migrated from gitlab.com)

mentioned in issue #148

mentioned in issue #148
Brouie commented 2026-05-05 02:49:22 +00:00 (Migrated from gitlab.com)

mentioned in issue #149

mentioned in issue #149
Sign in to join this conversation.
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#131
No description provided.