Security review #341

New issue

Closed

opened 2026-06-20 04:15:12 +00:00 by PlasticDigits · 3 comments

PlasticDigits commented 2026-06-20 04:15:12 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

Full stack security review.

Full stack security review.

PlasticDigits commented 2026-06-20 04:15:26 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

made the item visible to everyone

made the item visible to everyone

PlasticDigits commented 2026-06-21 03:43:52 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

Internal Full-Stack Security Audit — GitLab #341

File: audits/INTERNAL_COMPOSER_1782013334.md
Date: 2026-06-21
Reviewer: Cursor Cloud agent (Composer)
Commit: 8f695247562c29822c912a83052bfa5d01101f56
Issue: #341 Security review

Executive summary

Arena v2 (TimeArena, PodiumVaults, ReferralRegistry, TimeArenaBuyRouter, PlayCred, Doubloon) is the current onchain authority. The indexer and frontend are derived read/signing layers.

Under standard non-rebasing ERC-20 assumptions and correct deployment configuration, no Critical unprivileged path was found that would let an arbitrary attacker drain protocol custody (podium vault ledgers, prize pools, or mint authority outside intended game rules).

The codebase shows material hardening aligned with prior audits and GitLab invariants: nonReentrant on value-moving entrypoints, balance-delta ERC-20 parity on DOUB ingress (#123), Ownable2StepUpgradeable on UUPS proxies (#329), production indexer fail-closed guards (#142, #156, #326), parameterized SQL, per-IP rate limiting (#328), Slither CI, and Gitleaks.

Highest residual risks are cross-layer and operational:

1. Wallet approval blast radius — frontend always sizes DOUB/CL8Y approvals to type(uint256).max.
2. Blind-signing class — default gas-estimation fallback submits txs without a buffered gas override when RPC simulation fails.
3. Misconfigured public indexer — permissive CORS when INDEXER_PRODUCTION is unset on a non-loopback bind.
4. Trusted governance — owner can pause, re-anchor charm price, UUPS-upgrade, and one-shot grandfatherProgression (accepted trust model; high impact if keys compromised).

---

Scope and methodology

In scope

Layer	Components reviewed
Contracts	TimeArena, PodiumVaults, ReferralRegistry, TimeArenaBuyRouter, PlayCred, Doubloon, DoubAirdrop, ArenaCharmPriceTwap, routing/timer/XP libraries, DeployDev / DeployProduction
Indexer	api.rs, api_arena.rs, api_validate.rs, config.rs, cors_config.rs, rate_limit.rs, ingestion.rs, reorg.rs, persist.rs, migrations
Frontend	Arena buy/WarBow flows, approval sizing, writeContractWithGasBuffer, chain-mismatch guard, indexer API client, referral storage
Bots	bots/timearena config and key handling
CI / ops	.github/workflows/slither.yml, gitleaks.yml, deploy scripts, AGENTS.md dev-key policy
Docs	docs/onchain/security-and-threat-model.md, invariants map

Trust assumptions (per maintainer / prior audits)

• Governance/admin/UUPS upgrade authority is trusted when used as intended.
• Doubloon and configured pay tokens are standard ERC-20 (no fee-on-transfer / rebasing in production).
• Registry JSON and frontend env point at canonical ERC-1967 proxies, not implementation addresses.

Automated verification (this audit)

Suite	Command	Result
Contracts	forge test excluding DoubAirdropMegaethFork + WarbowBenchmark	221 passed, 0 failed
Indexer unit	cargo test --lib	80 passed
Frontend	cd frontend && npm test -- --run	743 passed (141 files)

Notes: Four DoubAirdropMegaethFork tests require gitignored airdrop/doub.csv (expected). TimeArenaWarbowBenchmark tests fail in this VM without benchmark fixtures — not a security regression signal.

---

Findings

F-01 — Medium — Unlimited ERC-20 approvals hardcoded on Arena spend paths

Field	Detail
Severity	Medium (user-funds exposure; not protocol drain)
Location	frontend/src/lib/arenaDoubApprovalPreference.ts (readArenaDoubUnlimitedApproval always true); consumers: useArenaSaleSession.ts, useArenaWarbowHero.ts, useArenaProtocolDonatePools.ts, ensureDoubTimeArenaAllowance.ts, ensureCl8yKumbayaAllowance.ts
Issue	All Arena DOUB/CL8Y approval flows approve maxUint256 to TimeArena / router. The prior opt-in unlimited toggle (#143/#277) is effectively removed — useCl8yArenaUnlimitedApproval is a no-op stub.
Impact	If the canonical TimeArena proxy is upgraded maliciously, compromised at the implementation layer, or the user is phished into approving a wrong spender address via a fake frontend, the entire token balance approved can be drained in one transfer. Industry pattern (e.g. BadgerDAO-style allowance drainers) targets long-lived unlimited approvals.
Reproduction / attack path	1) User loads a typosquat frontend with VITE_TIME_ARENA_ADDRESS pointing at attacker contract. 2) User approves maxUint256 DOUB. 3) Attacker calls transferFrom. Even with correct addresses, a future malicious UUPS implementation retains allowance until revoked.
Recommendation	Restore user-visible exact-amount + headroom default (#143); keep unlimited as explicit opt-in. Surface current spender + amount in wallet prompts. Document revocation path in wallet-connection docs. Consider periodic allowance expiry UX for power users.

---

F-02 — Medium — Gas estimation failure falls through to wallet default estimator

Field	Detail
Severity	Medium (user harm / blind signing)
Location	frontend/src/lib/writeContractWithGasBuffer.ts — default onEstimateRevert: "submit-without-override"; Arena buy/WarBow/flag paths in useArenaSaleSession.ts, useArenaWarbowHero.ts do not override
Issue	When estimateContractGas fails (RPC error, simulation revert, flaky MegaETH RPC), the helper submits via writeContractAsync without a gas override.
Impact	Wallet may prompt for a tx that reverts on-chain or uses incorrect gas semantics; users may approve without understanding failure mode. Under adversarial RPC, estimates can be omitted systematically.
Reproduction	Point VITE_RPC_URL at a broken endpoint; attempt buy — observe submission proceeds after estimate failure instead of blocking with a clear error.
Recommendation	Use onEstimateRevert: "rethrow" for all user-value txs (buys, WarBow, claimCred, donations). Reserve "submit-without-override" only for known-safe retry paths with explicit user messaging.

---

F-03 — Medium — Public indexer bind without INDEXER_PRODUCTION enables cross-origin data reads

Field	Detail
Severity	Medium (misconfiguration / privacy & deception enabler)
Location	indexer/src/cors_config.rs, indexer/src/config.rs (warn_public_bind_without_production)
Issue	Without INDEXER_PRODUCTION=1, CORS is fully permissive (Access-Control-Allow-Origin: *). A startup warning is logged for non-loopback binds, but the process still serves.
Impact	Any website can fetch public indexer JSON in the victim's browser (leaderboards, wallet stats, timer head). Combined with a malicious frontend, attackers can mix legitimate indexer data with fraudulent contract addresses — harder for users to detect mixed-trust UI. Does not move funds directly.
Reproduction	Run indexer with LISTEN_ADDR=0.0.0.0:3100 and no INDEXER_PRODUCTION. From evil.example, fetch('http://indexer:3100/v1/arena/timers') succeeds with ACAO *.
Recommendation	For internet-facing deploys: enforce INDEXER_PRODUCTION=1 + CORS_ALLOWED_ORIGINS in deployment templates (Render/systemd). Optional: fail-fast (not just warn) when LISTEN_ADDR is non-loopback and production mode is off.

---

F-04 — Low — INDEXER_TRUST_PROXY enables rate-limit bypass if reverse proxy is misconfigured

Field	Detail
Severity	Low (availability / abuse)
Location	indexer/src/rate_limit.rs — SmartIpKeyExtractor when INDEXER_TRUST_PROXY=1
Issue	Rate limits key off X-Forwarded-For when trust-proxy is enabled. If the edge proxy does not overwrite client-supplied X-Forwarded-For, attackers can rotate spoofed headers to get fresh buckets.
Impact	Elevated scraping / DB load / RPC fan-out via indexer; not auth bypass (API is unauthenticated reads).
Recommendation	Document that INDEXER_TRUST_PROXY=1 requires a trusted proxy that strips inbound X-Forwarded-For. Prefer connecting limiter to ConnectInfo peer when indexer is loopback-only behind nginx.

---

F-05 — Low — Indexer-first Arena display trusts offchain head for UX-critical timing

Field	Detail
Severity	Low (cross-layer deception)
Location	Product invariant INV-FRONTEND-301-INDEXER-FIRST-DISPLAY; frontend arena hooks polling GET /v1/arena/*
Issue	Production Arena UI intentionally avoids browser RPC mirrors for podium/timer/sale head when VITE_INDEXER_URL is set. Onchain state remains authoritative, but users may act on stale indexer rows after reorgs, lag, or indexer compromise.
Impact	Mis-timed buys, WarBow actions, or prize expectations; funds still move per onchain rules — user harm is economic/strategic, not direct theft via indexer.
Reproduction	Pause ingestion; observe UI timers frozen while chain advances; user buys based on stale deadline display.
Recommendation	Show block watermark + ingestion_alive / lag badge (#168). For high-stakes actions, optional client-side eth_call sanity check against TimeArena.deadline() / doubOwedForBuy.

---

F-06 — Low — Owner break-glass charm re-anchor (setEpochCharmAnchorWad)

Field	Detail
Severity	Low (centralization — trusted admin)
Location	contracts/src/arena/TimeArena.sol — setEpochCharmAnchorWad, _reanchorEpochCharmPrice on hard-reset buys
Issue	Owner can set epoch anchor arbitrarily at any time. Hard-reset buys also sample live TWAP/spot via _sampleCharmAnchor() on MegaETH (#303/#315).
Impact	Compromised owner key can shift DOUB buy pricing discontinuously, affecting user costs and referral registration burn (ReferralRegistry reads epochCharmAnchorWad). TWAP path mitigates same-block sandwich for hard-reset buys via doubOwedForBuy sampling at tx time (#315).
Recommendation	Operational: multisig + timelock when volume threshold met (already documented intent in #325/#329). Monitor LastBuyEpochCharmAnchored events.

---

F-07 — Low — UUPS upgrade authority concentrated in single owner

Field	Detail
Severity	Low (centralization — trusted admin)
Location	TimeArena, ReferralRegistry — _authorizeUpgrade → onlyOwner; Ownable2StepUpgradeable (#329)
Issue	Owner can upgrade implementation to arbitrary logic, including allowance-consuming hooks if combined with F-01.
Impact	Full protocol takeover on key compromise — exceeds scope of “unprivileged attacker” but is the dominant tail risk.
Recommendation	External audit before mainnet upgrades; on-chain timelock/multisig per roadmap; storage layout discipline + __gap preserved.

---

F-08 — Informational — Referral code registration is MEV-competitive (documented)

Field	Detail
Severity	Informational
Location	contracts/src/ReferralRegistry.sol — registerCode; docs/product/referrals.md
Issue	First successful onchain registration wins; observers can race popular slugs.
Impact	Economic griefing / squatting; DOUB burn is the deliberate deterrent.
Recommendation	Keep product disclosure; no contract change required unless policy shifts.

---

F-09 — Informational — MEV and same-block ordering are inherent to game design

Field	Detail
Severity	Informational
Location	TimeArena last-buyer ring, podium scoring, WarBow BP
Issue	Block builder ordering determines winners and timer extensions.
Impact	Searchers can profit from predictable buy races; not a contract bug.
Recommendation	UX/docs: treat proposer ordering as part of the game (docs/onchain/security-and-threat-model.md).

---

F-10 — Informational — PodiumVaults uses IERC20.transfer not SafeERC20

Field	Detail
Severity	Informational
Location	contracts/src/arena/PodiumVaults.sol — payPodiumWinners, rollEpochTranches
Issue	Payout uses bare transfer. Safe for standard Doubloon; would revert/fail on non-standard tokens.
Impact	None if Doubloon remains standard; prize payout could fail silently on exotic tokens if pools were repointed.
Recommendation	Keep Doubloon standard; if external pool addresses are ever wired via setActivePool, document ERC-20 requirements.

---

F-11 — Informational — DoubAirdrop lacks per-recipient fee-on-transfer parity

Field	Detail
Severity	Informational
Location	contracts/src/DoubAirdrop.sol
Issue	Pulls total once, then safeTransfer per recipient — FoT tokens could cause later recipients to receive less than values[i].
Impact	Low; utility contract, not core custody path.
Recommendation	Document “standard ERC-20 only”; or add balance-delta checks if FoT support is ever required.

---

F-12 — Informational — GET /healthz excluded from rate limiting

Field	Detail
Severity	Informational
Location	indexer/src/api.rs — health router merged outside rate-limited subtree
Issue	Unauthenticated health endpoint can be scraped at high QPS.
Impact	Minor DoS / noise; no sensitive data.
Recommendation	Acceptable for k8s probes; optional edge rate limit at reverse proxy.

---

F-13 — Informational — Reorg handling capped at 128 blocks

Field	Detail
Severity	Informational
Location	indexer/src/reorg.rs — MAX_REORG_DEPTH = 128
Issue	Deeper reorgs abort ingestion until manual intervention.
Impact	Stale API until operator replays; onchain funds unaffected.
Recommendation	Runbook for rollback_after + reindex; monitor chain_pointer via ops metrics (#306).

---

Positive controls observed

Control	Evidence
Reentrancy guards	TimeArena, TimeArenaBuyRouter, DoubAirdrop — nonReentrant on external value paths
ERC-20 ingress parity	TimeArena._pullDoubExact, ReferralRegistry.registerCode, router stable ingress
Router isolation	buyFor gated to timeArenaBuyRouter; path validation in _validatePath
SQL injection resistance	Parameterized queries; reorg.rs table names from static ARENA_INDEX_TABLES allowlist
Input validation	valid_0x_address20, normalize_tx_hash_param on sensitive routes (#329)
Error hygiene	PUBLIC_INTERNAL_DB_ERROR — no SQL echo (#157)
Production fail-closed	INDEXER_PRODUCTION CORS, DB URL, registry validation (#142, #156)
CI security	Slither (fail-on: high), Gitleaks on push/PR
Dev key hygiene	Anvil keys documented as local-only; deploy-megaeth-contracts.sh uses hidden prompt
Bots	YIELDOMEGA_PRIVATE_KEY from env only; dry-run default

---

Areas reviewed — no material issue

• PlayCred non-transferability enforced in _update; mint/burn only MINTER_ROLE (granted to TimeArena in production deploy).
• Doubloon — MINTER_ROLE cannot burnFrom others (#132).
• claimCred — requires settled epoch (epoch < lastBuyEpoch); clears weight before mint.
• WarBow — steal band checks, daily limits, bypass burn, guard reduces drain BPS.
• Frontend XSS — no dangerouslySetInnerHTML / eval in frontend/src.
• Wrong-network writes — chainMismatchWriteMessage gates contract writes (#95).

---

Recommendations summary (priority)

1. Medium — F-01: Re-enable exact-amount approvals by default; unlimited opt-in only.
2. Medium — F-02: Fail closed on gas estimate errors for value txs.
3. Medium — F-03: Mandatory production CORS guards on public indexer deploys (fail-fast optional).
4. Low — F-04/F-05: Proxy header hygiene + indexer lag watermarks.
5. Ops — F-06/F-07: Multisig/timelock roadmap for owner/UUPS; monitor anchor events.
6. Process: Independent external audit before mainnet financial launch; bug bounty when ready (per docs/onchain/security-and-threat-model.md).

---

End of report.

# Internal Full-Stack Security Audit — GitLab #341 **File:** `audits/INTERNAL_COMPOSER_1782013334.md` **Date:** 2026-06-21 **Reviewer:** Cursor Cloud agent (Composer) **Commit:** `8f695247562c29822c912a83052bfa5d01101f56` **Issue:** [#341 Security review](https://gitlab.com/PlasticDigits/yieldomega/-/issues/341) ## Executive summary Arena v2 (`TimeArena`, `PodiumVaults`, `ReferralRegistry`, `TimeArenaBuyRouter`, `PlayCred`, `Doubloon`) is the current onchain authority. The indexer and frontend are derived read/signing layers. Under **standard non-rebasing ERC-20** assumptions and **correct deployment configuration**, no **Critical** unprivileged path was found that would let an arbitrary attacker **drain protocol custody** (podium vault ledgers, prize pools, or mint authority outside intended game rules). The codebase shows **material hardening** aligned with prior audits and GitLab invariants: `nonReentrant` on value-moving entrypoints, **balance-delta ERC-20 parity** on DOUB ingress (#123), `Ownable2StepUpgradeable` on UUPS proxies (#329), production indexer **fail-closed** guards (#142, #156, #326), parameterized SQL, per-IP rate limiting (#328), Slither CI, and Gitleaks. **Highest residual risks** are cross-layer and operational: 1. **Wallet approval blast radius** — frontend always sizes DOUB/CL8Y approvals to `type(uint256).max`. 2. **Blind-signing class** — default gas-estimation fallback submits txs without a buffered `gas` override when RPC simulation fails. 3. **Misconfigured public indexer** — permissive CORS when `INDEXER_PRODUCTION` is unset on a non-loopback bind. 4. **Trusted governance** — owner can pause, re-anchor charm price, UUPS-upgrade, and one-shot `grandfatherProgression` (accepted trust model; high impact if keys compromised). --- ## Scope and methodology ### In scope | Layer | Components reviewed | |-------|---------------------| | **Contracts** | `TimeArena`, `PodiumVaults`, `ReferralRegistry`, `TimeArenaBuyRouter`, `PlayCred`, `Doubloon`, `DoubAirdrop`, `ArenaCharmPriceTwap`, routing/timer/XP libraries, `DeployDev` / `DeployProduction` | | **Indexer** | `api.rs`, `api_arena.rs`, `api_validate.rs`, `config.rs`, `cors_config.rs`, `rate_limit.rs`, `ingestion.rs`, `reorg.rs`, `persist.rs`, migrations | | **Frontend** | Arena buy/WarBow flows, approval sizing, `writeContractWithGasBuffer`, chain-mismatch guard, indexer API client, referral storage | | **Bots** | `bots/timearena` config and key handling | | **CI / ops** | `.github/workflows/slither.yml`, `gitleaks.yml`, deploy scripts, `AGENTS.md` dev-key policy | | **Docs** | `docs/onchain/security-and-threat-model.md`, invariants map | ### Trust assumptions (per maintainer / prior audits) - Governance/admin/UUPS upgrade authority is **trusted** when used as intended. - `Doubloon` and configured pay tokens are **standard ERC-20** (no fee-on-transfer / rebasing in production). - Registry JSON and frontend env point at **canonical ERC-1967 proxies**, not implementation addresses. ### Automated verification (this audit) | Suite | Command | Result | |-------|---------|--------| | Contracts | `forge test` excluding `DoubAirdropMegaethFork` + `WarbowBenchmark` | **221 passed**, 0 failed | | Indexer unit | `cargo test --lib` | **80 passed** | | Frontend | `cd frontend && npm test -- --run` | **743 passed** (141 files) | Notes: Four `DoubAirdropMegaethFork` tests require gitignored `airdrop/doub.csv` (expected). `TimeArenaWarbowBenchmark` tests fail in this VM without benchmark fixtures — not a security regression signal. --- ## Findings ### F-01 — Medium — Unlimited ERC-20 approvals hardcoded on Arena spend paths | Field | Detail | |-------|--------| | **Severity** | Medium (user-funds exposure; not protocol drain) | | **Location** | `frontend/src/lib/arenaDoubApprovalPreference.ts` (`readArenaDoubUnlimitedApproval` always `true`); consumers: `useArenaSaleSession.ts`, `useArenaWarbowHero.ts`, `useArenaProtocolDonatePools.ts`, `ensureDoubTimeArenaAllowance.ts`, `ensureCl8yKumbayaAllowance.ts` | | **Issue** | All Arena DOUB/CL8Y approval flows approve **`maxUint256`** to `TimeArena` / router. The prior opt-in unlimited toggle (#143/#277) is effectively removed — `useCl8yArenaUnlimitedApproval` is a no-op stub. | | **Impact** | If the canonical `TimeArena` proxy is upgraded maliciously, compromised at the implementation layer, or the user is phished into approving a **wrong spender address** via a fake frontend, the entire token balance approved can be drained in one transfer. Industry pattern (e.g. BadgerDAO-style allowance drainers) targets long-lived unlimited approvals. | | **Reproduction / attack path** | 1) User loads a typosquat frontend with `VITE_TIME_ARENA_ADDRESS` pointing at attacker contract. 2) User approves `maxUint256` DOUB. 3) Attacker calls `transferFrom`. Even with correct addresses, a future malicious UUPS implementation retains allowance until revoked. | | **Recommendation** | Restore user-visible **exact-amount + headroom** default (#143); keep unlimited as explicit opt-in. Surface current spender + amount in wallet prompts. Document revocation path in wallet-connection docs. Consider periodic allowance expiry UX for power users. | --- ### F-02 — Medium — Gas estimation failure falls through to wallet default estimator | Field | Detail | |-------|--------| | **Severity** | Medium (user harm / blind signing) | | **Location** | `frontend/src/lib/writeContractWithGasBuffer.ts` — default `onEstimateRevert: "submit-without-override"`; Arena buy/WarBow/flag paths in `useArenaSaleSession.ts`, `useArenaWarbowHero.ts` do not override | | **Issue** | When `estimateContractGas` fails (RPC error, simulation revert, flaky MegaETH RPC), the helper submits via `writeContractAsync` **without** a `gas` override. | | **Impact** | Wallet may prompt for a tx that **reverts on-chain** or uses incorrect gas semantics; users may approve without understanding failure mode. Under adversarial RPC, estimates can be omitted systematically. | | **Reproduction** | Point `VITE_RPC_URL` at a broken endpoint; attempt `buy` — observe submission proceeds after estimate failure instead of blocking with a clear error. | | **Recommendation** | Use `onEstimateRevert: "rethrow"` for all user-value txs (buys, WarBow, `claimCred`, donations). Reserve `"submit-without-override"` only for known-safe retry paths with explicit user messaging. | --- ### F-03 — Medium — Public indexer bind without `INDEXER_PRODUCTION` enables cross-origin data reads | Field | Detail | |-------|--------| | **Severity** | Medium (misconfiguration / privacy & deception enabler) | | **Location** | `indexer/src/cors_config.rs`, `indexer/src/config.rs` (`warn_public_bind_without_production`) | | **Issue** | Without `INDEXER_PRODUCTION=1`, CORS is **fully permissive** (`Access-Control-Allow-Origin: *`). A startup warning is logged for non-loopback binds, but the process still serves. | | **Impact** | Any website can fetch public indexer JSON in the victim's browser (leaderboards, wallet stats, timer head). Combined with a **malicious frontend**, attackers can mix legitimate indexer data with fraudulent contract addresses — harder for users to detect mixed-trust UI. Does **not** move funds directly. | | **Reproduction** | Run indexer with `LISTEN_ADDR=0.0.0.0:3100` and no `INDEXER_PRODUCTION`. From `evil.example`, `fetch('http://indexer:3100/v1/arena/timers')` succeeds with ACAO `*`. | | **Recommendation** | For internet-facing deploys: enforce `INDEXER_PRODUCTION=1` + `CORS_ALLOWED_ORIGINS` in deployment templates (Render/systemd). Optional: fail-fast (not just warn) when `LISTEN_ADDR` is non-loopback and production mode is off. | --- ### F-04 — Low — `INDEXER_TRUST_PROXY` enables rate-limit bypass if reverse proxy is misconfigured | Field | Detail | |-------|--------| | **Severity** | Low (availability / abuse) | | **Location** | `indexer/src/rate_limit.rs` — `SmartIpKeyExtractor` when `INDEXER_TRUST_PROXY=1` | | **Issue** | Rate limits key off `X-Forwarded-For` when trust-proxy is enabled. If the edge proxy does not **overwrite** client-supplied `X-Forwarded-For`, attackers can rotate spoofed headers to get fresh buckets. | | **Impact** | Elevated scraping / DB load / RPC fan-out via indexer; not auth bypass (API is unauthenticated reads). | | **Recommendation** | Document that `INDEXER_TRUST_PROXY=1` requires a trusted proxy that strips inbound `X-Forwarded-For`. Prefer connecting limiter to `ConnectInfo` peer when indexer is loopback-only behind nginx. | --- ### F-05 — Low — Indexer-first Arena display trusts offchain head for UX-critical timing | Field | Detail | |-------|--------| | **Severity** | Low (cross-layer deception) | | **Location** | Product invariant `INV-FRONTEND-301-INDEXER-FIRST-DISPLAY`; `frontend` arena hooks polling `GET /v1/arena/*` | | **Issue** | Production Arena UI intentionally avoids browser RPC mirrors for podium/timer/sale head when `VITE_INDEXER_URL` is set. Onchain state remains authoritative, but users may **act** on stale indexer rows after reorgs, lag, or indexer compromise. | | **Impact** | Mis-timed buys, WarBow actions, or prize expectations; funds still move per **onchain** rules — user harm is economic/strategic, not direct theft via indexer. | | **Reproduction** | Pause ingestion; observe UI timers frozen while chain advances; user buys based on stale deadline display. | | **Recommendation** | Show block watermark + `ingestion_alive` / lag badge (#168). For high-stakes actions, optional client-side `eth_call` sanity check against `TimeArena.deadline()` / `doubOwedForBuy`. | --- ### F-06 — Low — Owner break-glass charm re-anchor (`setEpochCharmAnchorWad`) | Field | Detail | |-------|--------| | **Severity** | Low (centralization — trusted admin) | | **Location** | `contracts/src/arena/TimeArena.sol` — `setEpochCharmAnchorWad`, `_reanchorEpochCharmPrice` on hard-reset buys | | **Issue** | Owner can set epoch anchor arbitrarily at any time. Hard-reset buys also sample live TWAP/spot via `_sampleCharmAnchor()` on MegaETH (#303/#315). | | **Impact** | Compromised owner key can shift DOUB buy pricing discontinuously, affecting user costs and referral registration burn (`ReferralRegistry` reads `epochCharmAnchorWad`). TWAP path mitigates same-block sandwich for **hard-reset** buys via `doubOwedForBuy` sampling at tx time (#315). | | **Recommendation** | Operational: multisig + timelock when volume threshold met (already documented intent in #325/#329). Monitor `LastBuyEpochCharmAnchored` events. | --- ### F-07 — Low — UUPS upgrade authority concentrated in single owner | Field | Detail | |-------|--------| | **Severity** | Low (centralization — trusted admin) | | **Location** | `TimeArena`, `ReferralRegistry` — `_authorizeUpgrade` → `onlyOwner`; `Ownable2StepUpgradeable` (#329) | | **Issue** | Owner can upgrade implementation to arbitrary logic, including allowance-consuming hooks if combined with F-01. | | **Impact** | Full protocol takeover on key compromise — exceeds scope of “unprivileged attacker” but is the dominant tail risk. | | **Recommendation** | External audit before mainnet upgrades; on-chain timelock/multisig per roadmap; storage layout discipline + `__gap` preserved. | --- ### F-08 — Informational — Referral code registration is MEV-competitive (documented) | Field | Detail | |-------|--------| | **Severity** | Informational | | **Location** | `contracts/src/ReferralRegistry.sol` — `registerCode`; `docs/product/referrals.md` | | **Issue** | First successful onchain registration wins; observers can race popular slugs. | | **Impact** | Economic griefing / squatting; DOUB burn is the deliberate deterrent. | | **Recommendation** | Keep product disclosure; no contract change required unless policy shifts. | --- ### F-09 — Informational — MEV and same-block ordering are inherent to game design | Field | Detail | |-------|--------| | **Severity** | Informational | | **Location** | `TimeArena` last-buyer ring, podium scoring, WarBow BP | | **Issue** | Block builder ordering determines winners and timer extensions. | | **Impact** | Searchers can profit from predictable buy races; not a contract bug. | | **Recommendation** | UX/docs: treat proposer ordering as part of the game (`docs/onchain/security-and-threat-model.md`). | --- ### F-10 — Informational — `PodiumVaults` uses `IERC20.transfer` not `SafeERC20` | Field | Detail | |-------|--------| | **Severity** | Informational | | **Location** | `contracts/src/arena/PodiumVaults.sol` — `payPodiumWinners`, `rollEpochTranches` | | **Issue** | Payout uses bare `transfer`. Safe for standard `Doubloon`; would revert/fail on non-standard tokens. | | **Impact** | None if `Doubloon` remains standard; prize payout could fail silently on exotic tokens if pools were repointed. | | **Recommendation** | Keep `Doubloon` standard; if external pool addresses are ever wired via `setActivePool`, document ERC-20 requirements. | --- ### F-11 — Informational — `DoubAirdrop` lacks per-recipient fee-on-transfer parity | Field | Detail | |-------|--------| | **Severity** | Informational | | **Location** | `contracts/src/DoubAirdrop.sol` | | **Issue** | Pulls `total` once, then `safeTransfer` per recipient — FoT tokens could cause later recipients to receive less than `values[i]`. | | **Impact** | Low; utility contract, not core custody path. | | **Recommendation** | Document “standard ERC-20 only”; or add balance-delta checks if FoT support is ever required. | --- ### F-12 — Informational — `GET /healthz` excluded from rate limiting | Field | Detail | |-------|--------| | **Severity** | Informational | | **Location** | `indexer/src/api.rs` — health router merged outside rate-limited subtree | | **Issue** | Unauthenticated health endpoint can be scraped at high QPS. | | **Impact** | Minor DoS / noise; no sensitive data. | | **Recommendation** | Acceptable for k8s probes; optional edge rate limit at reverse proxy. | --- ### F-13 — Informational — Reorg handling capped at 128 blocks | Field | Detail | |-------|--------| | **Severity** | Informational | | **Location** | `indexer/src/reorg.rs` — `MAX_REORG_DEPTH = 128` | | **Issue** | Deeper reorgs abort ingestion until manual intervention. | | **Impact** | Stale API until operator replays; onchain funds unaffected. | | **Recommendation** | Runbook for `rollback_after` + reindex; monitor `chain_pointer` via ops metrics (#306). | --- ## Positive controls observed | Control | Evidence | |---------|----------| | Reentrancy guards | `TimeArena`, `TimeArenaBuyRouter`, `DoubAirdrop` — `nonReentrant` on external value paths | | ERC-20 ingress parity | `TimeArena._pullDoubExact`, `ReferralRegistry.registerCode`, router stable ingress | | Router isolation | `buyFor` gated to `timeArenaBuyRouter`; path validation in `_validatePath` | | SQL injection resistance | Parameterized queries; `reorg.rs` table names from static `ARENA_INDEX_TABLES` allowlist | | Input validation | `valid_0x_address20`, `normalize_tx_hash_param` on sensitive routes (#329) | | Error hygiene | `PUBLIC_INTERNAL_DB_ERROR` — no SQL echo (#157) | | Production fail-closed | `INDEXER_PRODUCTION` CORS, DB URL, registry validation (#142, #156) | | CI security | Slither (`fail-on: high`), Gitleaks on push/PR | | Dev key hygiene | Anvil keys documented as local-only; `deploy-megaeth-contracts.sh` uses hidden prompt | | Bots | `YIELDOMEGA_PRIVATE_KEY` from env only; dry-run default | --- ## Areas reviewed — no material issue - **PlayCred** non-transferability enforced in `_update`; mint/burn only `MINTER_ROLE` (granted to `TimeArena` in production deploy). - **Doubloon** — `MINTER_ROLE` cannot `burnFrom` others (#132). - **`claimCred`** — requires settled epoch (`epoch < lastBuyEpoch`); clears weight before mint. - **WarBow** — steal band checks, daily limits, bypass burn, guard reduces drain BPS. - **Frontend XSS** — no `dangerouslySetInnerHTML` / `eval` in `frontend/src`. - **Wrong-network writes** — `chainMismatchWriteMessage` gates contract writes (#95). --- ## Recommendations summary (priority) 1. **Medium — F-01:** Re-enable exact-amount approvals by default; unlimited opt-in only. 2. **Medium — F-02:** Fail closed on gas estimate errors for value txs. 3. **Medium — F-03:** Mandatory production CORS guards on public indexer deploys (fail-fast optional). 4. **Low — F-04/F-05:** Proxy header hygiene + indexer lag watermarks. 5. **Ops — F-06/F-07:** Multisig/timelock roadmap for owner/UUPS; monitor anchor events. 6. **Process:** Independent external audit before mainnet financial launch; bug bounty when ready (per `docs/onchain/security-and-threat-model.md`). --- *End of report.*

PlasticDigits (Migrated from gitlab.com) closed this issue 2026-06-25 14:13:38 +00:00

PlasticDigits commented 2026-06-25 14:13:39 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

No items block launch or need tl be addressed at the expected volume/tvl. Closing.

No items block launch or need tl be addressed at the expected volume/tvl. Closing.

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

cac-design-issue-544

renovate/configure

No results found.

Labels

Clear labels   

agent:fix_bugfix

  

agent:fix_conflicts

  

agent:fix_security

  

agent:gap_analysis

  

agent:implement

  

agent:open_issues

  

agents

  

agent:security_audit

  

agent:verify

  

area

timecurve

  

arena

  

arena-v2

  

audit

  

block:feature_review

  

block:security

  

bug

  

contracts

  

design

  

devops

  

docs

Documentation

  

documentation

  

enhancement

  

epic

Epic / umbrella issue

  

feature

  

frontend

  

indexer

  

low priority

Low priority, investigate other issues first

  

megaeth

  

operations

  

ops

Deploy and operations

  

performance

  

QA

  

qa checklist

  

referrals

  

removal

Legacy removal

  

security

  

smart-contracts

  

tech-debt

  

testing

  

time-arena

TimeArena v2 revamp

  

timecurve

  

visual-qa

No labels

agent:fix_bugfix

agent:fix_conflicts

agent:fix_security

agent:gap_analysis

agent:implement

agent:open_issues

agents

agent:security_audit

agent:verify

area

timecurve

arena

arena-v2

audit

block:feature_review

block:security

bug

contracts

design

devops

docs

documentation

enhancement

epic

feature

frontend

indexer

low priority

megaeth

operations

ops

performance

QA

qa checklist

referrals

removal

security

smart-contracts

tech-debt

testing

time-arena

timecurve

visual-qa

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

code/yieldomega#341

No description provided.