OhShii Labs review, 2/8 · 7 findings (#5.1–#5.7): unvalidated input reaching permanent state, and cycle burn with no cross-caller ceiling

OhShii Labs review, 2/8: unvalidated input reaching permanent state, and cycle burn with no cross-caller ceiling

Second of eight. Theme: paths where a free or near-free caller writes unbounded data into stable state, or drives cycle spend, with no global bound. The Menese team's #2 covers the algorithmic side of bounding work; this covers the input side and the cycle side. They do not overlap except where noted.

DEPLOY_MODE = #play (main.mo:93), so inspect (main.mo:7846-7871) ends at not IS_PRODUCTION and admits every non-anonymous principal, registered or not. Its own comment concedes the consequence: "Play caveat, accepted for v1: unregistered spam isn't shed at this gate."

---

1. setUserPreferences persists an unbounded, unvalidated blob per free identity, with no eviction and no purge path

Where: src/backend/mixins/UserAccount.mo:109-113; type at src/backend/lib/Types.mo:198-201; map at src/backend/main.mo:3507

  public shared (msg) func setUserPreferences(prefs : Types.UserPreferences) : async () {
    requireAuth(msg.caller);
    let key = Principal.toText(msg.caller);
    Map.add(userPreferences, Text.compare, key, prefs);
  };

  public type UserPreferences = { recentMarkets : [Text]; lastMarket : ?Text };

Why it matters. requireAuth rejects only the anonymous principal; it is not an authorization boundary. There is no cap on recentMarkets, none on each Text, none on lastMarket. The only ceiling is the ~2 MiB ingress limit. The frontend voluntarily slices to 3 entries (main.js:8509) — a client-side convention, not a control.

This is structurally the one write endpoint reachable before any anti-Sybil control applies: the inspect comment at :7873 names "preference sync on sign-in" as precisely why unregistered principals are admitted.

userPreferences is a plain let in a persistent actor, so implicitly stable. We grepped every occurrence: it is never cleared by performWorldWipe / resetExchange / resetSeason (which clear ~60 maps at :13928-14048), and there is no admin purge method. Recovery would need --mode reinstall, which destroys the exchange ledger.

Roughly 2,700 calls from rotating free identities reaches WASM_MEMORY_LIMIT_BYTES (main.mo:120) — the wall your own comment at :117 records having hit once already ("the order-map leak bricked updates").

Suggested direction. Validate before persisting — reject recentMarkets.size() > 8, reject entries not present in markets, cap lastMarket length; add Map.clear(userPreferences) to performWorldWipe; and add a HARD_CAP on Map.size(userPreferences) that refuses new principals above it. A cheaper blanket fix for this whole class: Motoko's inspect supports the arg : Blob form, so return false when arg.size() exceeds ~16 KiB is one line, needs no per-method variant enumeration (the stated reason the current gate is caller-only), and closes every oversized-payload variant at once.

---

2. Bridge claim(asset : Text) writes a permanent ledger row for an unvalidated asset string before the rejection

Where: src/bridge/main.mo:229-237, :111-117, :40, :338-341

    let l = ledgerOf(msg.caller, asset);          // get-or-CREATE — writes here
    let claimable = if (l.confirmed > l.claimed) { l.confirmed - l.claimed } else { 0 };
    if (claimable == 0) { return #err("Nothing to claim for " # asset) };   // too late

      case null { let l : Ledger = { var confirmed = 0; var pending = 0; var claimed = 0 };
                  Map.add(ledgers, Text.compare, k, l); l };

Why it matters. ASSETS (:40) is referenced only by getSupportedAssets and the devConfirmDeposits loop — never as a membership test — and grep -n "Text.size" src/bridge/main.mo returns nothing. Returning #err from an update is a normal return, not a trap, so the Map.add commits. ledgers is implicitly stable, as the file's own comment at :313 notes; the Bridge has no admin wipe, and postupgrade (:333) clears only claiming/admitting. One authenticated principal, no Sybil needed.

Note the recovery trap this creates: a reinstall erases every user's confirmed/claimed high-water, but the DEX's creditedSeq is not reset, so a reinstalled Bridge restarts seq at 0 and every subsequent creditAndRegister no-ops (seq <= prevSeq → #ok, main.mo:5246). The DEX guard is correct; the operational consequence is that outstanding claimables become permanently unclaimable.

Suggested direction. Validate asset against ASSETS as the first statement of claim and devSimulateDeposit; move ledgerOf below the claimable == 0 check so a no-op claim writes nothing.

Adjacent, worth stating separately: the Bridge has no posture concept at all — grep -n "IS_PRODUCTION\|IS_DEV\|DEPLOY_MODE\|requireDevHook" src/bridge/main.mo returns zero matches. devSimulateDeposit/devConfirmDeposits are labelled "DEV ONLY (the real Bridge has NO such methods)" but carry no gate; their safety on #play is entirely outsourced to the DEX's playDepositCap(), which returns null on both #dev and #production. On a flip to #production those two methods plus claim mint real balance. docs/pre-mainnet-checklist.md has no Bridge step. A posture interlock mirroring UserAccount.withdraw:189 would close it in the code that ships rather than in the checklist.

---

3. createMarginPool(name : Text, …) stores an unbounded name; the 64-pool cap is per-principal

Where: src/backend/main.mo:9241-9264; Pool.name : Text at src/backend/lib/MarginPools.mo:28-34

The comment at :9244-9246 identifies pool count as a DoS amplifier and caps it at MAX_POOLS_PER_OWNER = 64 — but the cap is on count, not payload, and it is per-principal, so free to multiply. name is never length-checked. No funding, no registration, no minimum balance is required to reach it.

24 identities × 64 calls with a large name reaches the same memory wall as item 1, from a different door. Every pool row also creates poolByPrincipal and ownerPoolCount entries and a MarginEngine.open account, none of which are ever deleted for an unfunded pool — and there is no close/delete path anywhere (grep "Map.delete(marginPools" returns nothing) despite the cap message saying "Reuse or close an existing pool."

Suggested direction. Cap name at ~64 chars at entry; require registeredUsers membership (the gate aiComplete already uses at :12199-12202) or a minimum fundMarginPool in the same call; and add a closeMarginPool that deletes from all three maps when a pool has zero debt, zero balance and no open orders.

---

4. aiComplete has no prompt length bound, so per-call cycle cost is caller-controlled — with no global cap and no meaningful floor

Where: src/backend/main.mo:12190 (entry), :12227, :12280-12290, :12076-12080 (limits), :11936 (outcallCycles)

    let guarded = AI_GUARD_PREAMBLE # prompt;          // :12227 — no size check anywhere
    let req : HttpRequestArgs = {
      max_response_bytes = ?100_000;
      body = ?Blob.toArray(Text.encodeUtf8(bodyText));  // :12284 — caller-sized

Why it matters. Outcall cost is base + 400·n·request_bytes + 800·n·max_response_bytes, and request_bytes is dominated by prompt, which has no length check on any path. The rate limiter (10/min, 100/h, 250/24h) bounds call count, not cost.

On a 13-node subnet the fixed floor is 49,140,000 + 100,000 × 10,400 ≈ 1.09 B cycles per call; a 1.5 MB prompt adds 1,550,000 × 5,200 ≈ 8.06 B, giving ≈9.15 B cycles/call against a legitimate frontend prompt of ~8–15 KB (≈1.13 B) — roughly an 8× per-call amplification, and the same ratio holds under single-node pricing if is_replicated = false is honoured. One principal at the rate limit is ~2.29 T/day; ten are ~9.15 T/h. AUTO_FUEL_HEADROOM_MIN is 2 T with a 10-minute cooldown (:10493-10496), so the drain outruns self-funding.

docs/deploy-to-subnet.md:149 already anticipates the shape of this ("launch traffic is many principals") while quoting limits (20/min · 200/h · 500/24h) that no longer match the code's 10/100/250.

Suggested direction. if (Text.size(prompt) > AI_MAX_PROMPT_BYTES) { return #err(...) } with ~32 KiB — four times the real frontend prompt. Also add ("maxOutputTokens", #number(#int(4096))) to the Gemini generationConfig: the Anthropic branch bounds output with max_tokens: 4096 (:12259) but the Gemini branch sets only temperature, so a caller can force a response that exceeds max_response_bytes and is rejected after the full charge is paid.

---

5. There is no global cross-caller rate cap and no cycle-reserve floor on any ingress method

Where: the gate at main.mo:7846-7871; src/backend/lib/RateLimit.mo is used at exactly one site, main.mo:12097

We searched for each defence layer rather than assuming:

• Cycle floor on inbound methods: absent. Cycles.balance() appears at :6163, 6316, 6337, 6429, 6480, 6509, 6821, 7276, 7718, 10515, 11938 — every one is an outbound-funding affordability check. None gates an inbound call.
• Global cross-caller cap: absent. grep -n "globalWindow\|globalCount\|MIN_CYCLES\|GLOBAL_RATE" src/backend/main.mo → zero hits. The only global valve, recomputeShedFloor (:6115-6122), keys off staged-order depth, not call rate, and only affects registered callers.
• Per-caller cap: one method.
• Economic gate: one method.

The cheapest faucet we found is _internet_identity_sign_in_start (.mops/identity-attributes@0.4.1/src/lib.mo:47-49, included at main.mo:8119) — public shared func with no caller binding at all, and Random.blob in mo:core is rawRand (.mops/core@2.5.0/src/Random.mo:29-31), a management-canister call on every invocation. Per call the canister pays ≈6.5–7 M cycles; the caller pays nothing. At 100 calls/s that is ~56–60 T/day; at 1,000 calls/s ~560–600 T/day, i.e. ~21 h and ~2 h respectively to the freezing threshold on 50 T of headroom. Because tickArchiveFuel/tickBridgeFuel/tickArbFuel stop feeding at freezeLimit + 5 T, the archive and bridge starve first. Per main.mo:6250-6258 a frozen archive is a stable state that already caused a live 700k-event backlog on 2026-07-11.

Suggested direction. A MIN_CYCLES_RESERVE floor at the top of every ingress update; one global (windowStartNs, count) tuple checked in every update — the only Sybil-proof primitive and two comparisons; and a caller binding plus registeredUsers on _internet_identity_sign_in_start.

---

6. aiActionExecuted writes the map the aiComplete registration gate exists to protect

Where: main.mo:12313-12317; contrast :12199-12202; map at :12142

aiComplete's own comment states the registration gate exists because otherwise there is "unbounded growth of the never-evicted aiCallLog/aiUsage maps". aiActionExecuted, 120 lines below, writes aiUsage with only requireAuth. The Text.size(method) > 64 check bounds nothing — the argument is discarded, while the caller principal (the actual map key) is unbounded in cardinality. adminClearAiBan clears only aiBanUntil and aiRefusalLog, so there is no operator remedy short of an upgrade that drops the map. Row size is fixed, so this is slow growth rather than the 2 MiB class, but it is a clean bypass of the stated invariant.

Suggested direction. Apply the same registeredUsers gate, and add lazy eviction with a hard cap on both maps.

---

7. A trap in any synchronous heartbeat subtask stops all maintenance permanently

Where: main.mo:6140-6189; notably :6170 (drainLedgerJournal), :6184 (sweepStaleUserOrders), :6174 (tickTier), :6186 (tickDeadman); journal at src/backend/lib/Accounts.mo:25,45

Same failure class as Menese #2 item 1, different trigger — both need fixing. Theirs is processDeferredExpiry's uncapped O(K²) matcher; this is the heartbeat body itself.

We verified with the repo's pinned moc 1.9.0 that ignore f() where f : () -> async T schedules a separate message (output ordering shows the caller's continuation running before the callee's body). So the heartbeat mixes two dispatch styles with very different blast radii: ignore tickAmm() / tickLiquidations() / tickShipEvents() / tickPriceRefresh() are trap-isolated, while reapClosedOrders() / drainLedgerJournal() / tickTier(now) / tickHeatmaps() / tickLeaderboardShard() / settleInsuranceArrears() / sweepStaleUserOrders(now) / tickCandleFill() / tickDeadman() run inside the heartbeat message with no isolation.

Four properties turn one trap into a permanent outage:

1. The throttle stamp rolls back with the trap. _lastTtlSweepNs := now is assigned in the same message, so a later trap undoes it and the next heartbeat re-enters identical work — every round, forever.
2. Calls issued by a trapping message are never sent. tickLiquidations, finaliseExpiredPending (which drives GEPTOR releases), tickShipEvents, tickPriceRefresh, tickAutoFuel all stop. Staged orders never release, so user funds stay locked in reservations.
3. It is self-reinforcing. drainLedgerJournal iterates accounts.journal, an unbounded stable List receiving one entry per balance mutation (Accounts.mo:45, the single write funnel) and cleared only by this drain. Each entry costs a UserEvent construction plus a SHA-256. While the heartbeat traps, trading keeps appending and List.clear is rolled back every round, so the trap point eventually migrates to :6170 and stays there even after the original trigger is gone.
4. No aggregate budget, no breaker. The sync caps are sized individually but never summed, and their cadences (10 s / 30 s / 60 s / 300 s) align every 300 s. There is no consecutive-failure counter and no timer fallback.

Mitigating: _lastHeartbeatNs also rolls back, so a stale heartbeat is visible — detectable, but not recoverable in-canister. setTestTimersPaused(true) stops the trap but also stops maintenance and never drains the journal.

Suggested direction. Chunk drainLedgerJournal (~5,000 entries/beat; journalUnshipped already surfaces the backlog); dispatch every subtask the way the safe ones already are (async + ignore), which alone turns "permanent" into "one skipped tick" because the throttle stamp then lives in the parent message; add per-subtask consecutive-failure breakers; and give sweepStaleUserOrders and tickDeadman the cursor-and-cap treatment every other sweep already has via lib/Shard.mo.

---

— Ravenith, OhShii Labs

Start by scoping the findings across main.mo, src/backend/mixins/UserAccount.mo, src/backend/lib/Types.mo, src/bridge/main.mo, and src/backend/lib/RateLimit.mo; inspect the cited entry points and persistence maps first. Confirm the existing gates, limits, cleanup paths, and heartbeat tasks before choosing a focused change. Done should be demonstrated by bounded inputs or cycle use, protected persistent writes, and recovery or cleanup paths matching the stated invariants.