OhShii Labs review, 5/8 · 4 findings (#8.1–#8.4): privacy and scoping — three surfaces that contradict their own documented guarantees

[rvnt9999]

OhShii Labs review, 5/8: privacy and scoping — three surfaces that contradict their own documented guarantees

Fifth of eight. Theme: SECURITY.md names "reading or acting on another principal's balances, orders, positions, or history (including through the OQL query surface and its per-caller row scoping)" as in scope. These are three routes to the same outcome, each of which is documented as impossible.

We want to be clear up front that the OQL executor itself is, in our reading, the best-defended surface in the codebase. We tried hard to break it and could not: auth resolution is a total function of (level, caller) that no query field can influence; joins build each target's index at that target's own resolved access (#deny → Map.empty), so a permitted entity cannot join out a denied one and a denied target is not even an existence oracle; aggregates fold over the post-scope set; and the classic secondary-index scope bypass is explicitly gated (Executor.mo:97-106, startSubject == null) with the reason stated in-code. The findings below are all upstream of the executor — data that was placed on public surfaces before scoping could apply.

---

1. archiveExecute returns other users' rows, and two documentation surfaces state the opposite

(Mailed to multidex@dfinity.org on 2026-07-31, observing rows for hxsn7-… — the Arbitrageur principal — with kind, token and amount populated. Repeated here with the source trace.)

Where: main.mo:14591-14605; the claims at main.mo:9079 and src/frontend/src/docs.js:1417-1419

Unlike execute(), which enforces per-entity TableAuth through the executor, archiveExecute pre-scopes rows outside the executor — and deliberately adds a second, unscoped set:

      // (2) Every OTHER user's deposits & withdrawals (public money-flow
      // ledger) from the archive's kind-filtered index — real seq, complete
      // history once backfilled. Skip the caller's own D/W (already pulled
      // above) to avoid duplicates. Bounded by ARCHIVE_DW_CAP / page cap.
      if (dwAdded < ARCHIVE_DW_CAP) {
        ...
          let page = await full.getDepositWithdrawals(dwOff, 200);
          for (e in page.vals()) {
            var mine = false;
            for (p in principals.vals()) { if (Principal.equal(p, e.user)) { mine := true } };
            if (not mine and dwAdded < ARCHIVE_DW_CAP) { List.add(evs, e); dwAdded += 1 };

Note the filter's polarity — if (not mine …) keeps rows that are not the caller's. The userEvent entity is then registered #public_ with archiveAccess hard-coded to the anonymous principal, so the executor applies no row scoping at all, by design, because "scoping happened upstream". For set (2) it did not.

The arbitrageur appears because fundArbitrageur (:4862), skimArbitrageur (:4911) and extMarketSwap (:4957-4958, :4972-4973) all call appendDeposit on the arb principal, so its entire flow history is in the D/W index and is served to every caller.

What is contradicted. getApiDoc() tells every API consumer and the in-app assistant:

"archiveExecute(...) reaches deep history. Other users' private rows are filtered out server-side."

and the in-app documentation tells every user:

"…with row-level scoping enforced server-side. Public data is public; your rows are yours; other users' private rows simply never leave the canister."

Both are false for deposit/withdrawal rows. Whether a money-flow ledger should be public is a defensible product decision — the codebase argues elsewhere that proof-of-reserves requires it — but two user-facing surfaces assert the opposite, so users and integrators are making privacy decisions on a false premise.

Suggested direction. Pick one and make all three agree: either drop set (2) and serve other users' D/W through an explicitly-named public endpoint, or correct main.mo:9079 and docs.js:1419 to state that the D/W ledger is public and principal-attributed, and say why. If the latter, see item 3 — the exact amount values are what make the join below work.

---

2. getEventsRange / getDepositWithdrawals republish the per-principal index that the owner-gate on getEventsForPrincipals was added to close

Where: src/backend/ArchiveCanister.mo:326-341 and :376-389; the control at :287-305; event shape at src/backend/lib/Types.mo:189-196; archive ids from main.mo:6753

// ArchiveCanister.mo:294-300 — the stated control
//   OWNER-GATED: only the exchange canister … may call this directly.
//   … This closes the targeted vector — arbitrary by-principal lookups
//   (leaderboard → rival's principal → position → liq price). The public
//   whole-tape reads (getEventsRange, getDepositWithdrawals) stay open …
//   they carry no easy per-human attribution index.
public query func getEventsRange(startSeq : Nat, limit : Nat) : async [Types.UserEvent] { ... }

public type UserEvent = { seq : Nat; ts : Int; user : Principal;
                          counterparty : ?Principal; kind : UserEventKind; prevHash : ?Blob };

Why it matters. The justification does not hold: every row carries user and counterparty, across #fill, #liquidation, #borrow, #repay, #delta, #debtDelta, #lpShareDelta. Paging the tape reconstructs the per-principal index offline in one pass.

The cost premise in docs/oracle-settlement-and-transparency.md:68-79 ("expensive") is inverted on IC: query calls bill the caller nothing, stats() (ArchiveCanister.mo:346) publishes eventCount so the page count is known in advance, and getArchives() (main.mo:6753) is an unauthenticated query enumerating every archive canister id. The entire cost falls on the canister.

Chain, anonymous throughout: backend.getArchives() → every archive id; archive.stats() → firstSeq/nextSeq; loop archive.getEventsRange(seq, 200) — your own tests/test_archive_replay.sh:161 does exactly this with --identity anonymous; filter on user == <target>. #delta folding then reproduces any account's balances by construction, and #fill rows reconstruct each margin-pool principal's net position and entry, from which liq prices follow using the public MarginPools math.

Identity linkage is genuinely broken elsewhere — the leaderboard withholds user (main.mo:14904-14916) — but liquidation hunting does not need identity, only a principal, a size and a level.

Suggested direction. Decide which invariant wins and make both surfaces agree. Either redact user/counterparty on getEventsRange for all kinds except the PoR-load-bearing ones (#delta/#debtDelta/#lpShareDelta/#insShareDelta), preserving proof-of-reserves while removing trade/liquidation attribution; or drop the owner gate as ineffective and document the tape as fully public. Either way the two comments and the design-doc paragraph need correcting — they currently assert a protection that does not exist, and future work will be built on it.

---

3. An OQL aggregate over the public deposit tape reconstructs capitalUsd exactly, rejoining the username↔principal split

Where: main.mo:9905-9918 (hodlBaselineUsd), :7906-7918 (appendDeposit), :14906-14917 (leaderboard, #public_), :14503-14536 + :14591-14605

The two halves are written by one function from one record, so they cannot diverge:

  func appendDeposit(user : Principal, record : Types.DepositRecord) {
    UserStatus.appendDeposit(userDeposits, user, record);
    switch (record.kind) {
      case (#deposit)    { recordExternalFlow(user, record.token, record.amount) };
      case (#withdrawal) { recordExternalFlow(user, record.token, -(record.amount : Int)) };
    };
    emitEvent(user, null, #deposit(record));      // → the public tape
  };

  func hodlBaselineUsd(k : Text) : Int {
      let px : Int = if (token == Types.QUOTE_TOKEN) { Fixed.SCALE }   // exactly 1.0

Why it matters. The codebase states this invariant three times and enforces it in three places — PublicLeaderRow drops the principal (:9934-9957: "Names here, principals on the tape, and no surface that joins the two"), usernameOf stopped publishing a principal prefix (:9967-9980), and the OQL leaderboard entity omits user (:14903-14905).

But capitalUsd is a pure function of the deposit/withdrawal ledger, and that ledger is published principal-attributed on the same surface. For any account funded only in Types.QUOTE_TOKEN — priced at exactly Fixed.SCALE, no oracle estimate — capitalUsd == Σ(signed deposit amounts) exactly, an e8-precision integer that is a near-unique fingerprint. Per :10009-10021, every leaderboard-eligible account on #play is funded through the Bridge deposit path, and controller-seeded sim bots have no profile and are excluded, so the two sides cover the same population.

Two anonymous query calls, no state change:

{"start":"userEvent","where":{"in":{"field":"kind","value":["deposit","withdrawal"]}},"groupBy":["user","kind","token"],"aggregate":[{"fn":"sum","field":"amount","as":"net"}],"limit":1000}

{"start":"leaderboard","select":["rank","username","capitalUsd","equityUsd","profitUsd","returnBps"],"orderBy":[{"field":"capitalUsd","dir":"desc"}],"limit":1000}

Compute Σ deposits − Σ withdrawals per principal and match against capitalUsd / 1e8; an exact match links username ↔ principal. Beyond the ARCHIVE_DW_CAP = 2000 fold, page getDepositWithdrawals(offset, 200) directly.

This is exactly the walk the code says must not be possible: "leaderboard → principal → that trader's entire attributed financial history, then hunt them." It chains straight into item 2.

Suggested direction — any one closes the exact join; the first two are cheapest:

1. Coarsen leaderboard.capitalUsd/equityUsd/profitUsd in the #public_ projection — bucket to 2–3 significant figures, or publish only returnBps, which is already lossy. The board's purpose survives.
2. Bucket userEvent.amount for other users' D/W rows while keeping the caller's own exact.
3. Refuse groupBy/aggregate on userEvent.user unless the caller is the subject.

Confidence note: the channel is confirmed — both sides are demonstrably on #public_ entities and the identity is exact and single-sourced. The uniqueness of the match is data-dependent and we did not test it, since that would mean probing the deployment.

---

4. Two smaller items in the same area

order is documented "PUBLIC but UNATTRIBUTED", but a partially-filled resting order is attributable in real time. main.mo:14859-14864 correctly omits owner/ownerName, but the row exposes id and filled, and the moment an order takes its first partial fill an attributed #fill{ orderId } event lands on the public tape (emitFillEvents, :7063-7077). Joining order.id == fill.orderId recovers the owner of every partially-filled resting order while it is still resting — the exact scenario the unattributed design targets. Only never-filled orders keep the intended anonymity. Dropping id from the projection would close it; nothing in the depth/slippage use case needs it.

_nameEntropySeeded is a stable latch set before an await with no postupgrade reset. main.mo:6152-6160, declared at :8133. This is the shape src/bridge/main.mo:310-336 documents and fixes; main.mo has no system func postupgrade at all, so the remedy was never mirrored. An upgrade landing between the call and the Random.blob() reply destroys the continuation while the latch survives as true, and nothing re-seeds. Profiles.usernameFromDraws exists (per its own comment at :8123-8126) precisely so usernames are not reconstructible offline — with an unseeded pool the draw sequence becomes a deterministic function of public registration timestamps, making the same username↔principal join computable by a third route. system func postupgrade() { _nameEntropySeeded := false }, or treat _nameEntropy == 0 as the real unseeded test so the retry is idempotent.

---

— Ravenith, OhShii Labs

[dominicwilliams]

Confirmed. The documentation half is fixed; one of the code-side gaps is only half-closed, and I want to name which half.

#8.1 — getApiDoc() and docs.js both claim other users' rows are filtered server-side and "never leave the canister". Confirmed. archiveExecute deliberately serves every other user's deposits and withdrawals. Under this project's transparency doctrine the publicness is defensible — a public, principal-attributed tape is a design requirement, not an accident — but the claims are not, and main.mo already contradicted itself on this about 600 lines below. The docs are corrected: they now describe what the canister actually serves.

#8.2 — the owner gate on getEventsForPrincipals. Confirmed. The gate says it "closes the targeted vector" while getEventsRange/getDepositWithdrawals are public and every row carries user and counterparty. Fixing the text alone would have been a concession that the gate is decorative, which is why it is documented as bounded-by-design rather than defended.

#8.4b — the _nameEntropySeeded latch with no postupgrade. Confirmed, and fixed with the remedy you proposed. Worth recording why: our first cut improvised a cleverer fix, keying the retry on _nameEntropy == 0. That re-enters the branch on every heartbeat whenever the randomness beacon is unavailable, and because the await splits the heartbeat message, the rest of the beat stops running. It silently killed tickHeatmaps on a local replica where Random.blob() does not resolve. The shipped fix is your original system func postupgrade() — the same shape src/bridge/main.mo already documented for its own set-before-await latch. A good argument for taking a reporter's suggested remedy seriously rather than improvising past it.

#8.4a — the order.id ↔ fill.orderId join. Confirmed, only half-closed. id cannot simply be dropped from the order projection: it is that entity's declared primary key and the executor traps on a hidden pk. The OQL half is shut — the public userEvent projection carries no orderId — but the archive's raw getEventsRange still returns the whole #fill variant, so the join survives there. Closing it properly means per-kind redaction on the archive surface, which is a larger change than this pass. It is open, and it is tracked as such rather than counted as done.

#8.3 — the capitalUsd join. Confirmed as a code-side privacy gap that does not conflict with the doctrine. Also open. (The uniqueness caveat on it came from the Menese team's report; credit is split there.)

The code is in #30, the first push to this mirror since the July snapshot; your line numbers against 0241cba still resolve.