[ghost-dao-contracts] Treasury.withdraw has no excess-reserve guardrail: one over-withdrawal bricks rebase/stake/unstake for everyone

Bug Bounty Remediation in Progress
1

Component: ghost-dao-contracts @ b87dff4, src/Treasury.sol:70-77 (withdraw), src/Treasury.sol:259-261 (excessReserves).

Description: GhostTreasury.withdraw is onlyGovernor and reduces totalReserves by tokenValue(token, amount) without touching FTSO supply and without checking that reserves stay above supply. excessReserves() computes totalReserves - (FTSO.totalSupply() - totalDebt) under Solidity 0.8 checked arithmetic. Once a withdrawal pushes totalReserves below totalSupply, excessReserves() reverts with a panic on every call. That bricks treasury.mint, GhostDistributor.distribute() / retrieveBounty(), GhostStaking.rebase() (which calls both distributor functions unconditionally when a distributor is set), and therefore stake() and unstake(). Withdrawing reserves is a routine, honest treasury operation (grants, expenses, rebalancing); the contract gives the operator no guardrail and no warning. The failure is total even for a zero-amount mint: distribute() with rewardRate == 0 still evaluates excessReserves() and reverts.

Impact: Temporary denial of service of the core staking functions (stake, unstake, rebase) for all users. No fund loss; positions are untouched. Recovery requires one governor transaction (deposit reserves back or point the distributor at address(0)).

Reproduction: test/poc/Tim4Round2Withdraw.t.sol: 2 passed. After a governor withdrawal of 60 percent of reserves, treasury.excessReserves() reverts, staking.rebase() reverts, and staking.stake() reverts; after the governor deposits reserves back, rebase() works again.

Severity: Low. Reason: the trigger is a trusted party making an accounting error, the halt is recoverable with no loss of funds, and the same party can already move all reserves. It is reported because the failure mode is disproportionate and silent.

Recommendation: In withdraw, require totalReserves - value >= IFTSO(FTSO).totalSupply() - totalDebt (or at least emit an explicit warning event when the withdrawal eats into backing). Alternatively, make rebase() resilient: skip the distributor leg instead of reverting when the treasury cannot mint.

Verification: VERIFIED (forge PoC executed, passed).

2

Thank you for your report!

A bit more context: the withdraw function is meant to be called by governance to forcefully move funds out of the treasury in case of a hack or abuse of the Treasury contract or other smart contracts that are part of the DAO. This one is arguably questionable in itself, in my opinion, but it’s there for now.

That said, you are correct here - it would stop all interactions, which actually makes sense: nothing could be minted, because the reserves and the FTSO supply are desynced.

There is a commit that avoids errors and silently stops minting from the Treasury side while excessReserves is unavailable. What are your thoughts on this?

3

Thanks for confirming, and for sharing the commit.

I reviewed 479755d96a against the report. The saturating excessReserves() plus the skip-instead-of-revert in mint() resolves the availability side: rebase, stake, unstake and distribute no longer brick when reserves dip below supply, which matches the resilient-rebase option I suggested. I also checked that totalDebt is never written anywhere in the current source, so dropping it from the formula changes nothing in practice.

Two notes from reading the diff:

  1. withdraw() itself is unchanged, so the primary guardrail (or warning event) from the report was not taken. I agree with your framing that withdraw is an emergency tool, so the remaining scenario is now “silent but non-bricking” rather than “silent and bricking”. The warning-event option still stands if you ever want the explicit signal, but I do not consider it a blocker.

  2. One thing worth a second look: Treasury.phantomRefund now returns the full computed gasValue even when the FTSO mint is skipped, and Staking.phantomRefund mints GHST to the receiver based on that return value. In the desynced state that path would mint unbacked GHST (previously it reverted and the Gatekeeper swallowed it). Amounts are gas-sized per call and it needs the desynced state first, so I would call it a hardening note rather than a finding, but having phantomRefund return the actually-minted amount (0 when skipped) would close it cleanly.

Smaller observability point: a skipped mint emits nothing, so ops cannot distinguish “nothing to mint” from “mint skipped because reserves are desynced”. An event on the skip path would keep the failure visible.

Happy to hear your take.

4

Record-keeping note for the first-reporter accounting on this sink. Topic 164, filed 2 October by Sands, covers the same distributor-leg brick: once the distributor reward exceeds excess reserves, stake reverts because mint reverts InsufficientReserves. Its suggested fix was to skip the distribution leg rather than the whole stake when the reward exceeds excess, the same skip-instead-of-revert shape as commit 479755d96a. The trigger there is the honest flow, accrued rewards outgrowing excess reserves, distinct from the governor over-withdrawal trigger filed here on 8 October.

Shahmeer