Skip to content

SECURITY.md

Latest commit

 

History

History
132 lines (82 loc) · 13 KB

SECURITY.md

File metadata and controls

132 lines (82 loc) · 13 KB

Security notes

Static analysis

Run:

slither src/DiemVault.sol \
  --solc-remaps "@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/ forge-std/=lib/forge-std/src/" \
  --exclude-dependencies --filter-paths "lib/"

Current findings

All remaining slither findings reviewed and accepted:

# Detector Location Status
1 incorrect-equality (== 0) harvest(), requestRedeem() False positive. Null-input checks, not balance/state comparisons.
2 reentrancy-benign harvest() Accepted. diemOut is the swap return value, so the state update must follow the external swap call. nonReentrant blocks cross-function reentry, and the SlipStream router is trusted code.
3 timestamp (cooldown comparisons) flush(), unstakeBatch() False positive. Cooldown comparisons are the intended use of block.timestamp.

Resolved

  • reentrancy-no-eth in flush() — fixed by reordering state writes before diem.initiateUnstake().

External audit (GPT 5.5, 2026-05-24)

Findings reproduced as failing fork-tests in test/DiemVaultAudit.t.sol, then fixed; tests now assert the fixed behaviour.

Severity Finding Fix
HIGH Direct DIEM donation inflated totalAssets(), causing previewRedeem to reserve more assets than totalStaked. flush() then underflowed and stranded the batch. Rewrote totalAssets() to totalStaked − batches[currentBatch].totalAssets. Donations are ignored entirely and sit inert in balanceOf.
MED harvest() swapped the vault's live USDC balance, so USDC arriving between off-chain quote and inclusion enlarged the swap beyond what minDiemOut was sized for. harvest(amountIn, minDiemOut) now takes an explicit amountIn. Keeper passes the same value it quoted for; surplus USDC harvests next cycle.
MED depositCap enforced against totalAssets() (which subtracts pending redemptions), so cap room "refilled" once users queued redeems. Gross controlled DIEM could exceed the cap. Cap now enforced against totalStaked + cooldownAmount (gross controlled DIEM). Room only frees once a user claims. Documented as a blast-radius cap, not an active-shares cap.
LOW register-seller.ts deduped existing offers by (model, base_url) without validating payout_address/caps/pricing/active. Stale offers could silently settle elsewhere. Script now treats existing offers as configuration to validate. Mismatches are PATCHed; inactive Venice offers are flagged (not auto-reactivated).
LOW Inherited renounceOwnership() would brick harvest() forever (USDC stranded, no compounding). Overridden to revert with OwnershipRenouncementDisabled. Transfer ownership to a multisig instead.
LOW Surplus settlement may fail after inference runs → seller eats the upstream cost. Already documented; operational not contract. Mitigate via conservative caps and monitoring.

setMinBatchOpenSecs upper bound stays at 7 days. It's a ceiling not a default — the operator only sets it when they need to, and the lower bound is unrestricted.

External audit round 2 (GPT 5.5, 2026-05-24)

Targeted regression check on the round-1 fixes. Three additional findings, all fixed and locked behind test/DiemVaultAudit2.t.sol.

Severity Finding Fix
MED Cap leak via drained-but-unclaimed: after unstakeBatch() decrements cooldownAmount, the owed DIEM sits in balanceOf until the user calls claimRedeem. Cap room appeared to free but gross custody hadn't dropped. Added drainedClaimable state. Incremented in unstakeBatch, decremented in claimRedeem. maxDeposit now uses totalStaked + cooldownAmount + drainedClaimable.
LOW register-seller.ts dedupe collapsed duplicate offers per model — only one was reconciled, the rest could route stale. Strict URL equality also broke on normalization drift. Group active offers by (normalized_url, model) and PATCH every duplicate. Strip trailing slashes, lower-case both sides when comparing seller_base_url.
LOW Cap-clearing PATCH used undefined, which JSON-stringifies to nothing → Surplus saw an empty patch and left the cap unchanged. Allow null in UpdateOfferInput; emit explicit null when intended cap is empty. Warn at end of run if any cap-clears were sent (Surplus's null-acceptance is undocumented; operator should verify).
Design Q Harvest with totalSupply()==0 produced DIEM nobody could claim (absorbed by decimal-offset virtual shares). harvest() reverts with NoShareholdersToCompound when totalSupply is zero.

Round-2 status: clean. 24/24 fork tests pass.

ERC-1271 authorizedSigner (post-audit, pre-launch)

Vault implements IERC1271.isValidSignature so Venice's headless API-key flow can verify contract-signed challenges. The check is: signature recovers to authorizedSigner (a separate EOA from the Safe owner, settable via setAuthorizedSigner). Returns the magic value 0x1626ba7e if match, 0xffffffff otherwise.

Separating authorizedSigner from owner is intentional. The Safe owner manages admin (cap, slippage param, ownership transfer). The signer EOA is purely a Venice-signing credential. Compromise of the signer key gives an attacker the right to generate new Venice API keys for the vault's stake — bounded to the same blast radius as any vault-issued API key — but doesn't let them touch the vault's DIEM, USDC, or admin functions. The Safe can rotate to a fresh signer at any time.

Pattern adapted from AntSeed's DiemStakingProxy (verified, live on Base, 125 DIEM staked). Same shape; we split signer from owner where they bundled them.

Harvest is permissioned (keeper / owner) — round-4 audit response

Initial round-4 design made harvest() permissionless with an on-chain spot-quoter slippage floor. GPT 5.5 round 4 raised a HIGH: the spot quote is observable from the same block as the swap, so a flashloan attacker can manipulate pool price → call harvest at the manipulated price → unwind. The 1% slippage cap only constrains drift between the quote and the swap, not fair-value loss.

Reversion: harvest(amountIn) now requires msg.sender == keeper || msg.sender == owner(). Keeper is a separate EOA settable by owner via setKeeper. Owner is always allowed as a fallback (so a Safe can manually harvest if the keeper key is unavailable).

redeemWithHarvest removed — same attack vector indirectly (attacker holding 1 wei of vDIEM could trigger harvest at manipulated price, capturing the loss for the vault). Users now use plain requestRedeem; unharvested USDC stays in the vault for remaining shareholders. Acceptable trade-off (internal redistribution > external value loss).

Residual MEV risk: even keeper-submitted harvests can be sandwiched if visible in the public mempool. The keeper SHOULD submit harvest transactions via private mempool (Flashbots Protect on Ethereum mainnet; on Base, MEV-Share or any RPC with private bundle support). This isn't a contract-level enforcement — operational hygiene.

v1.1 path: add TWAP-anchored slippage floor by reading the SlipStream pool's observe() over a 30-min window. Manipulating a 30-min TWAP is expensive (must hold the position for the window). Once TWAP-protected, re-open harvest() to anyone and re-introduce redeemWithHarvest. ~4-6 hours work + its own audit pass.

External audit round 3 (GPT 5.5, 2026-05-24)

Narrow-scope sweep of the round-2 diff only. Contracts came back clean — drainedClaimable accounting verified across multi-user / partial-claim / out-of-order-drain scenarios, the totalSupply()==0 harvest guard is sound. Two LOW findings, both in register-seller.ts only, both applied:

Severity Finding Fix
LOW Stale offers not reconciled — Venice-backed Surplus offers for models no longer in Venice's discovery catalog were silently skipped. They can still route requests with old payout/caps. After the main loop, scan veniceOffers for active offers whose model isn't in discovery. Print a strong warning with offer IDs + config so the operator can review. Don't auto-deactivate (might be intentionally grandfathered).
LOW PATCH self-verification missing — the reconciler trusted PATCH responses without confirming Surplus had actually applied the change. Particularly dangerous for null cap-clearing, where Surplus's semantics are undocumented. Use the Offer returned by patchOffer() to recompute drift after each patch. If drift remains, mark that offer as failed and exit non-zero at end-of-run with a summary line per failure.

This concludes the LLM-audit phase. Next quality bar is human review (informal Base/DeFi peer) or a paid contest (Sherlock) at meaningful TVL.

Trust model

  • Owner can: call harvest(), set depositCap, set minBatchOpenSecs, transfer ownership.
  • Owner CANNOT: drain user funds, change the underlying DIEM/USDC/swap router/tickSpacing (all immutable), pause the contract.
  • Worst-case owner grief: call harvest(minDiemOut = 0) so the swap gets sandwiched/zeroed. Bounded loss = vault's USDC balance at that moment. User principal in DIEM is unaffected. Mitigate by running a multisig owner.

Operating model: emergency response

There is no pause() function, by explicit design. A global pause is a single owner-controlled lever that can freeze user withdrawals as easily as deposits — exactly the blast radius we've spent the audit rounds eliminating. The vault's safety story is "the owner cannot touch user funds," and a pause would undercut it. So we don't have one.

Instead, the owner has soft-pause levers that throttle inflows and compounding without ever blocking exits:

  • setDepositCap(0) — halts new deposits. maxDeposit is enforced against gross controlled DIEM (totalStaked + cooldownAmount + drainedClaimable), so a zero cap rejects all new deposits while leaving existing positions and the redemption queue fully functional.
  • setKeeper(0x0) — halts harvest. harvest() requires msg.sender == keeper || msg.sender == owner(); zeroing the keeper EOA stops automated compounding. The owner remains a fallback caller, so harvest can still be performed manually if needed, or not at all — unharvested USDC simply sits in the vault for remaining shareholders.

Neither lever can strand a user. The redemption lifecycle stays live because its three steps are permissionless by design — anyone can poke a pending redemption forward, so liveness never depends on the owner or keeper being available or honest:

  • flush() — opens/settles the current batch's cooldown. Permissionless.
  • unstakeBatch() — completes the unstake once cooldown elapses. Permissionless.
  • claimRedeem() — pays out claimable DIEM. Permissionless to call, but internally permissioned to the batch's owner via a msg.sender check, so a third party can never redirect another user's payout. Anyone can advance the lifecycle; only the rightful owner receives the funds.

The combination — soft-pause the way in, never the way out — means the worst an owner can do in an "emergency" is stop the vault from growing. They can't trap what's already inside.

Upstream trust

  • Venice DIEM (0xF4d97F2da56e8c3098f3a8D538DB630A2606a024) — MINTER_BURNER_ROLE via OpenZeppelin AccessControl. Admin could in principle mint or modify behavior. Same trust as holding DIEM directly.
  • Aerodrome SlipStream router (0xBE6D8f0d05cC4be24d5167a3eF062215bE6D18a5) — production Aerodrome contract.
  • Surplus Intelligence holds our Venice API key encrypted at rest (AES-256-GCM, app secret in Vercel env). A breach drains our daily inference credit but cannot cost more than the staked DIEM's daily $1/DIEM budget.

Operational notes

DIEM/USDC liquidity (resolved)

Real USDC/DIEM liquidity lives on the Aerodrome SlipStream pool at 0xBc3231036Ee1ECa03E5F67FEceDC640D21610823 (tickSpacing 100). Recent snapshot: ~$128k TVL ($102k USDC + ~14.6 DIEM at $1,812 each), $378k 24h volume. Healthy.

Earlier exploration of the vanilla Aerodrome v2 USDC/DIEM pool found near-zero reserves — that pool is effectively unused. The vault is hardcoded to the SlipStream pool via the SlipStream router + tickSpacing 100.

Pool depth + slippage

With $102k USDC on one side of the SlipStream pool, a harvest of ~$100 USDC moves price by ~0.1%. Our default keeper slippage floor is 100 bps (1%), giving meaningful headroom while still rejecting catastrophic swaps. For larger harvests (>$1k) consider raising HARVEST_MIN_USDC to batch more, or lowering HARVEST_SLIPPAGE_BPS to enforce tighter execution.

DIEM price

At time of writing, 1 DIEM ≈ $1,812 USDC (market prices in ~5 years of forward $1/day inference credit). Implied yield if Venice's full $1/day per staked DIEM is captured = ~20% APY in DIEM-equivalent. The vault is unlikely to realize the full $1; expect ~$0.30–$0.80/day per DIEM after Surplus market dynamics. Resulting APY in DIEM-equivalent likely lands 5–15%.

Audit status

Not audited. v0 ships under a conservative depositCap until either a Sherlock contest or informal review by a Base/DeFi peer.