Distributor bounty never reset: one setBounty becomes every-epoch emission forever

Bug Bounty
1

Component: ghost-dao-contracts @ b87dff4, src/StakingDistributor.sol (GhostDistributor), retrieveBounty() lines 58-62.

Description: retrieveBounty() mints bounty to the staking contract every time rebase() runs but never resets bounty to zero:

function retrieveBounty() external override returns (uint256) {
    if (msg.sender != staking) revert OnlyStaking();
    if (bounty > 0) ITreasury(treasury).mint(staking, bounty);
    return bounty;
}

The function is named “retrieve”, and no code path resets bounty except a manual setBounty() by the governor. So one setBounty(X) call prints X FTSO EVERY epoch until the governor notices and sets it back to 0. This is also not a keeper incentive: the mint goes to the staking contract, not to the rebase() caller, so economically this is a recurring emission stream to stakers, not a one-time bounty. Additionally, if the treasury’s excessReserves() falls below bounty, treasury.mint() reverts (InsufficientReserves), and since GhostStaking.rebase() calls retrieveBounty() unconditionally, the whole rebase() reverts: stake()/unstake() are stuck until the governor calls setBounty(0).

Impact: Unbounded per-epoch FTSO emission (drains excess reserves, dilutes backing); worst case: rebase bricks (staking functions DoS) until the governor acts.

Reproduction: Local forge PoC (PASS). test_poc_bountyIsMintedEveryEpochAndNeverReset: deploy GhostDistributor with a mock treasury, governor calls setBounty(1000e9), call distribute() + retrieveBounty() twice (simulating two epochs). Result: distributor.bounty() still 1000e9; mock treasury recorded 2 * bounty minted. test_poc_persistentBountyBricksRebaseWhenReservesDepleted proves retrieveBounty() reverts when the treasury mint fails (simulated depleted excess reserves); the revert propagation into rebase() reads directly from the code.

Severity: Medium. Reason: real economic impact (recurring emission plus potential rebase DoS), but the trigger is governor configuration, not a permissionless attacker path. A governor who believes this is a one-time bounty would unknowingly drain reserves.

Recommendation: Add bounty = 0; after the mint in retrieveBounty() (one-time semantics matching the name). If recurring emission is intended, document it explicitly, rename to e.g. recurringEmission, and make rebase() resilient to a failing bounty mint (try/catch, or skip when bounty > excessReserves()).

Verification: VERIFIED (forge PoC passed; 209 existing tests passed before adding PoCs, 214 after, no regressions).

2

hank you for your report!

You are right here, but only partially. This part of the code should be removed once we’ve removed all logic related to random incentive minting.

In any case, this function is guarded to be callable only from staking, and it can fail once the protocol reaches a 1:1 token-to-reserve ratio - which is expected behavior, since we need it to stop at that point. Or do you see some other implications of this?

3

Thanks for the partial confirmation. A few implications I see beyond the leftover code:

  1. rebase() is permissionless (Staking.rebase() is public), so after a single setBounty(X) the recurring mint is triggered by anyone calling rebase() each epoch. No further governor action is needed for the emission to repeat indefinitely.

  2. The mint goes to the staking contract, not the rebase() caller. In Olympus-fork semantics a “bounty” is a keeper incentive paid to the caller; here it provides zero keeper incentive and is purely recurring inflation to stakers. The name actively misleads a governor into thinking it is a one-time caller reward.

  3. On the 1:1 point: the brick does not need the 1:1 ratio. Treasury.mint reverts whenever bounty > excessReserves(), and rebase() calls retrieveBounty() unconditionally, so a stale non-zero bounty bricks the whole rebase() (including distribute(), which runs just before it in the same call) as soon as reserves dip below the bounty value. Worse, the never-reset bounty drains excessReserves() a little every epoch, so the leftover value actively drives the system toward that revert condition. The protocol then needs a governor transaction (setBounty(0)) to unstick epoch advancement.

So the leftover is not just dead code: one stale setBounty gives recurring permissionless-triggered inflation that degrades into a rebase halt.

4

That makes sense now!

Given some deviations from the original code and the restructuring it went through, how do you envision the fix? A few options:

  1. Use tx.origin in StakingDistributor.sol:retrieveBounty(), which isn’t really good in my view.
  2. Extend the function interface to accept the rebase caller address, passing msg.sender from Staking.sol:rebase() as an argument to StakingDistributor.sol:retrieveBounty(msg.sender).
  3. Any other, better options?
5

On the fix, my take:

The simplest correct fix is the one-liner from my report: bounty = 0 after the mint in retrieveBounty(). That gives one-time semantics matching the function name, and it is the smallest change.

Of your two options: I would avoid tx.origin (option 1). It breaks with contract callers, multisigs, and account abstraction, and it is generally discouraged. Option 2 (pass the rebase caller address through) is cleaner and restores actual keeper-incentive semantics: the caller who pays for the rebase transaction gets the bounty.

But note the tension: option 2 changes the economics (caller gets paid) while the one-liner keeps current economics (stakers get it, once). Since you said this code is leftover from random incentive minting and slated for removal, the one-liner or outright removal seems most honest. If you want to keep a keeper incentive, option 2.

My recommendation: either remove it as planned, or add bounty = 0. Avoid tx.origin.

6

We’ve discussed your report internally and have some clarifications. Let’s take another look at the stake function interface:

function rebase() public override returns (uint256 bounty)

It returns the bounty to the caller. But if you look more closely at the stake function and the unstake function, both use it:

function stake(
    uint256 amount,
    address to,
    bool isRebase,
    bool isClaim
) external override returns (uint256 returnAmount) {
    if (!unlocks[to] && to != msg.sender) revert ExternalDepositsLocked();

    returnAmount = amount + rebase();
    IERC20(ftso).safeTransferFrom(msg.sender, address(this), amount);

    if (isClaim && warmupPeriod == 0) {
        returnAmount = _sendStnkBased(returnAmount, to, isRebase);
    } else {
        uint48 expiry = epoch.number + warmupPeriod;
        IGhostWarmup(warmup).addToWarmup(returnAmount, to, expiry);
    }
    emit Staked(msg.sender, to, amount, isRebase, isClaim);
}

// and

function unstake(
    uint256 amount,
    address to,
    bool isTrigger,
    bool isRebase
) external override returns (uint256) {
    amount += isTrigger ? rebase() : 0;
    if (isRebase) {
        ISTNK(stnk).safeTransferFrom(msg.sender, address(this), amount);
    } else {
        IGHST(ghst).burn(msg.sender, amount);
        amount = IGHST(ghst).balanceFrom(amount);
    }

    if (amount > IERC20(ftso).balanceOf(address(this))) revert InsufficientBalance();
    IERC20(ftso).safeTransfer(to, amount);
    emit Unstaked(msg.sender, to, amount, isTrigger, isRebase);
    return amount;
}

Both are used to apply the bounty to their own balances, which is why staking is the receiver of the additionally minted funds. As for permanently minting the bounty, that’s a governance decision to reward those who trigger the rebase via stake or unstake - while calling them directly would do nothing but dilute the protocol, which would happen in any case.

7

Fair enough, and thanks for laying out the code. You are right: stake() adds rebase() into returnAmount and unstake() adds it when isTrigger is set, so the bounty does reach the caller. That makes the “zero keeper incentive” framing in my report wrong, and your “governance decision to reward those who trigger the rebase” reading coherent with the code.

I will treat this one as closed/intended on my side. Appreciate the direct engagement on it.

8

The devs took inspiration from your report.

In this commit, they changed the bounty so it’s only collected during direct interaction with the protocol - for example, stake and unstake. Thanks for being proactive!