Repository navigation
Add StakeDAO Escrow - #139
Conversation
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, reopen this pull request to trigger a review.
N E M E S I S — Verified Findings: StakeDaoEscrow.sol (current branch)Re-run confirmation — 2026-06-03Independently re-ran the full Nemesis iterative loop (Feynman ↔ State, 4 passes, converged) against the current
Scope
Diff vs prior auditsTwo prior audit files exist in this directory:
What still applies from prior audits and is re-verified against the current code:
What is new in this audit (introduced by, or only surfaced after, the recent fixes):
Phase 0 Recon — Attacker Hit ListAttack goals:
Novel surface vs
Phase 1 Cross-Reference (Nemesis Map)
Coupled state pairs
Verification Summary
No CRITICAL or HIGH findings in the current code. The previously-CRITICAL NM-001 — Conditional bad debt:
|
| Candidate | Verdict | Reason |
|---|---|---|
Front-run initialize() on a freshly-created proxy |
False positive | Market.getEscrow() (Market.sol:253-259) deploys via CREATE2 and calls initialize atomically in the same tx — no mempool window. |
Front-run initialize() on the implementation contract |
Non-issue | Impl contract holds no funds. Attacker would also need a contract whose collateral() returns rewardVault.asset() to pass L77 — and even then, the impl proxy has no shares/tokens to drain. |
Reentry from IMarket(msg.sender).collateral() at L77 (after market = msg.sender at L76) |
False positive | Reentry into initialize is blocked by L74 (market != 0). Reentry into pay/onDeposit/balance reverts because token is still address(0) at this point. Reentry into claim/setClaimer reverts because beneficiary is still address(0) and the caller is not allowlisted. No state mutation is observable before the call, and the call is fail-closed. |
Market.collateral() triggers arbitrary code during initialize |
False positive | Market.collateral is IERC20 public immutable (Market.sol:41) — the auto-generated getter is a pure SLOAD of an immutable bytecode constant, no fallback or hook. |
claim(tokens) with tokens.length != amounts.length returned by vault |
False positive | The escrow emits amounts directly from rewardVault.claim; mismatched length is not exploitable, only emits noisy events. |
| Donation of tokens or shares directly to the escrow inflates collateral | Working as intended | Donations increase the borrower's effective collateral. Net positive to the borrower; the donor pays. Cannot be exploited against the borrower. |
onDeposit callable by anyone enables griefing |
Intentional | Documented in NatSpec ("should remain callable by anyone"); matches peer design. Direct transfers become a donation to the beneficiary. |
pay() ignores rewardVault.withdraw return value |
False positive | ERC4626 withdraw(assets, receiver, owner) guarantees assets of the underlying are delivered to receiver or the call reverts. The returned shares value is informational. The subsequent safeTransfer enforces final solvency. |
accountant.claim(_gaugeArray(), new bytes[](0), to) with mismatched array lengths |
False positive | Empirically verified by fork test testAllowlistedClaimerReceivesCrvBaseRewardToken against the live mainnet accountant (block 25_076_112) — call succeeds and CRV reaches the receiver. The deployed Stake DAO accountant accepts the empty harvestData array. |
setClaimer race — beneficiary revokes after auto-claimer tx is mined |
Not a bug | Standard allowlist semantics. |
gaugeArray could be larger than 1 |
Working as intended | Single-vault design — each escrow corresponds to one Stake DAO vault with one gauge. |
Multi-Transaction Adversarial Sequences (Phase 5)
For each TRUE POSITIVE, the minimal trigger sequence:
NM-001:
Market.deposit(100)— escrow stakes 100 in vault, shares = 100,previewRedeem(100) = 100.Market.borrow(80)— protocol believes collateral = 100, debt = 80.- Vault enters paused or capped state (external action — not attacker-controlled, but possible).
previewRedeem(100) == 100still, butvault.withdraw(N)reverts for any positiveN. - Position becomes under-collateralized due to price move;
Market.liquidate(user, debt)→escrow.pay(liquidator, ~88)→vault.withdraw(88)reverts → liquidation reverts. - Bad debt accrues until vault unpauses.
NM-002 (alone): No adversarial sequence — only an information-disclosure / observability gap. Rewards are recoverable on next claim.
NM-002 × NM-004 (combined):
- Many users open positions; each gets an escrow with
gauge = G_oldcached. - Stake DAO governance migrates the vault to
G_new. - Users call
claim→try accountant.claim([G_old], ...)reverts (gauge stale) → catch swallows →rewardVault.claim(extras, to)proceeds → event emitted with no base reward delivered. - Users see "Claim" event with no CRV/BAL transfer and have no way to recover via the escrow.
Feedback-Loop Discoveries
- NM-002 was a cross-feed finding. Pass 1 Feynman did not flag the try/catch because the catch's purpose is plausible (tolerate "no pending rewards" reverts). Pass 2 State did not flag it because no internal escrow state is corrupted by a swallow. Only Pass 3 Feynman targeted re-interrogation — asking "WHY does the dev assume every revert from
accountant.claimis benign?" — surfaced the silent-failure surface. Pass 4 State then linked it to NM-004 (the stale-gauge case where the catch becomes permanent loss). - NM-004 emerged from Pass 3 when Feynman asked "why are
gaugeandaccountantcached at all, given they're already proxied viarewardVault?". Pass 4 State then traced the cache as a frozen coupled state with no reconciliation path.
These two findings are the iterative loop's value-add over running either auditor alone.
False Positives Eliminated
See "Findings explicitly considered and dismissed" above (10 items). The most important are:
- Implementation-contract initialization hijack (not exploitable — impl holds no funds).
- Reentry during
IMarket(msg.sender).collateral()(every callable path inside the escrow is fail-closed at this point in initialize). - Length-mismatched
harvestDataarray onaccountant.claim(empirically verified by fork test on real mainnet vault).
Summary
| Severity | Count |
|---|---|
| CRITICAL | 0 |
| HIGH | 0 |
| MEDIUM | 0–1 (NM-001, conditional on deployed vault) |
| LOW | 1 (NM-002) |
| INFO | 5 (NM-003, NM-004, NM-005, NM-006, NM-007) |
The two CRITICAL/HIGH findings present in the prior audit reports for older versions of this contract (initialize always-revert; base-reward claim using msg.sender) have both been fixed in the current branch. The current code's only material concern is the conditional bad-debt risk from previewRedeem ↔ withdraw asymmetry (NM-001) — same trust assumption as any vault-backed escrow, mitigable by clamping with maxWithdraw. NM-002 + NM-004 are paired operational concerns about silent failure if Stake DAO ever reconfigures the vault's gauge or accountant; worth adding an event and/or reading gauge dynamically to eliminate the silent-loss case.
|
|
Nemesis audit: StakeDaoEscrow.sol Key context discovery: A thorough prior Nemesis report already exists in .audit/findings/nemesis-verified.md (dated 2026-06-03, NM-001…NM-007). But it predates commit Result: 0 Critical / 0 High / 0 Medium new findings The new feature is sound. The headline reason: the contract derives balance() from live holdings (previewRedeem(shares) + token.balanceOf) instead of a mutable New findings (addendum):
Verified-safe coupled pair: stakeDaoDepositsEnabled ↔ {fund location, vault allowance, balance branch} — every flag transition updates allowance and relocates 100% of Dismissed as false positives: guardian stealing collateral (disable only relocates funds inside the escrow; only onlyMarket pay() can move them out), toggle |
|
No findings on v12 |
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository because its GitHub organization is not linked to your Claude organization.
To turn it back on, one person needs both roles: owner of the GitHub organization (or the account owner, for a personal repository) and admin or owner of your Claude organization. If that is not you, pass this on to someone who is.
Open Organization settings, Git providers and click Link next to the GitHub organization's name. If it is not listed, click Enable GitHub integration or Connect your GitHub first, whichever the page shows.
New pull requests are then reviewed as before. To get a review on one that is already open, someone with write access can comment @claude review.
Tip: disable this comment in your organization's Code Review settings.
Escrow interfacing with the StakeDAO rewardVault model