OhShii Labs review, 1/8 · 4 findings (#4.1–#4.4): client-side integrity — third-party script, absent CSP, and a ledger verifier whose certificate check never runs

[rvnt9999]

OhShii Labs review, 1/8: client-side integrity — third-party script, absent CSP, and a ledger verifier whose certificate check never runs

We reviewed the published tip of MULTI/DEX as invited. This is the first of eight grouped write-ups, split by theme rather than one issue per finding, following the shape the Menese DeFi Team used in #2/#3.

Two notes before the findings.

On reporting privately. We hit the same wall the Menese team documented in #2: SECURITY.md:7 points at https://github.com/dfinity/multidex/security/advisories/new, but this repository is dfinity/public-multidex and dfinity/multidex does not resolve. There is no working private channel, which is why this is public. We had previously mailed multidex@dfinity.org on 2026-07-11 and 2026-07-31; items already sent there are marked below.

On severity framing. Everything here is scoped to the live #play posture. Play-money balances genuinely cap the value-theft findings — but they do not cap this group. The findings below are session-integrity and client-trust issues that land at full severity on a real, publicly advertised service with real users.

---

1. The app loads executable JavaScript from unpkg.com with no SRI (highest impact in this group)

Where: src/frontend/index.html:2109

2109  <script src="https://unpkg.com/lightweight-charts@5.1.0/dist/lightweight-charts.standalone.production.js"></script>
2110  <script type="module" src="/src/main.js"></script>

Why it matters. grep -rn "integrity=" src/frontend/ returns nothing, and lightweight-charts appears in neither package.json nor package-lock.json — so this tag is the only delivery path and it is unpinned. The library is consumed as a global (main.js:3001, :3015).

This voids the property the asset canister exists to provide. The repo already reasons correctly about exactly this risk one file away, in src/frontend/public/.ic-assets.json5, where allow_raw_access: false is set because a raw URL "can serve modified JS to a signed-in trader with no cryptographic check". The certified page then voluntarily loads uncertified third-party code on every visit. Cache-Control: public, max-age=0, must-revalidate on the HTML means the tag is re-evaluated every load; the JS freshness is unpkg's decision, not yours.

An attacker able to influence what unpkg returns — npm publish-rights compromise, a CDN edge, DNS or TLS position on unpkg.com — executes in the https://multidex.ai origin. No MULTI/DEX account required.

What that reaches, precisely. docs/session-policy.md states the base key is a non-extractable WebCrypto ECDSA key, so "page JavaScript (and therefore XSS) can use it but never read it out". Correct — and using it is sufficient. An injected script can drive withdraw, placeMarketOrder, withdrawMarginPool, closePosition as the signed-in user for the whole delegation TTL.

There is a second reach that we think is under-appreciated, tied to the Google login. The play anti-Sybil gate keys the allowance on a salted hash of the user's verified Google email, and that email transits page JavaScript as an II-signed attribute bundle before reaching the canister:

      attributesPromise = authClient.requestAttributes({          // main.js:1986
        keys: [GOOGLE_VERIFIED_EMAIL_KEY],                        // "openid:https://accounts.google.com:verified_email"
        nonce: mintNonceEphemeral(),
      });
      ...
          const out = await presentAttributesToBackend(await attributesPromise);   // main.js:2002

So an injected script can also read the user's verified Google email address in transit — the identifier the design deliberately never stores raw — joining a real-world identity to an on-chain principal.

To be precise about the boundary, since overstating it would be unhelpful: it cannot read the user's Google password or accounts.google.com cookies, and cannot authenticate to Google elsewhere; the OpenID exchange happens inside id.ai, a different origin. "Google session compromise" would be wrong. "Harvest the verified Google identity and take full control of the DEX session" is accurate.

Suggested direction. The package is npm-installable and Vite already builds the app:

npm i lightweight-charts@5.1.0

import { createChart, CandlestickSeries, HistogramSeries, LineSeries } from "lightweight-charts";

and delete line 2109. Land it together with item 2 — security_policy: "standard" sets script-src 'self', which makes this class of regression structurally impossible but breaks the chart until it is bundled. If the tag must survive short-term, integrity="sha384-…" crossorigin="anonymous" helps, but SRI does not survive a version bump and does nothing about the per-visit IP/Referer disclosure to a third party.

---

2. The .ic-assets.json5 that ships declares no security_policy, so the deployed app serves no CSP

(Reported to multidex@dfinity.org on 2026-07-11, and raised on the DFINITY forum and on X at the same time. Repeated here with the root cause, which we did not have then.)

Where: src/frontend/public/.ic-assets.json5:22-28 versus the never-shipped sibling src/frontend/.ic-assets.json5:2-9

The file that ships:

22    {
23      "match": "**/*",
24      "headers": {
25        "Cache-Control": "public, max-age=0, must-revalidate",
26      },
27      "allow_raw_access": false,
28    },

The sibling that does not ship has the line that went missing:

 4      "security_policy": "standard",

Why it matters. security_policy is optional in ic-asset and defaults to disabled — absent it, the canister stores only the headers listed. The root cause is visible in the shipping file's own header comment: the caching and raw-access rules were migrated into public/ on 2026-07-23 and security_policy was left behind.

Missing on **/*: Content-Security-Policy (so item 1 is entirely unconstrained, as is any future injection), frame-ancestors and X-Frame-Options (the whole trading UI — order entry, withdraw form, margin close — is framable; ai-connect.html is the only path that gets frame-ancestors 'none'), X-Content-Type-Options (the extension-less .well-known files have no reliable content type), Referrer-Policy (full URLs leak, including ?dxq= share links), Permissions-Policy, Strict-Transport-Security.

Suggested direction.

  {
    "match": "**/*",
    "security_policy": "standard",
    "headers": { "Cache-Control": "public, max-age=0, must-revalidate" },
    "allow_raw_access": false,
  },

ai-connect.html carries an inline <script> and needs its own policy with a 'sha256-…' hash. Note the app emits inline onerror="this.style.display='none'" in 8 places (main.js:113, 1641, 3733, 4297, 4786, 6779, 7738, 8530); standard blocks these, whose only effect is that broken token logos stop auto-hiding — replace with an addEventListener("error", …) pass or accept the cosmetic change. index.html itself has zero inline handlers, so script-src 'self' is otherwise a drop-in. Deleting the dead sibling file would stop the trap recurring.

---

3. The in-browser ledger verifier's certificate check never actually runs — and the repo already diagnosed this in its own CLI

Where: src/frontend/src/ledger.js:139-147, :153-158, :269-274; contrast scripts/verify_ledger.mjs:430-435

Browser:

139    const cert = await Certificate.create({
140      certificate: new Uint8Array(certBytes),
141      rootKey: rk,
142      principal: cid,                    // bare Principal
146      disableTimeVerification: true,
147    });
...
157      return { ok: null, why: (err && err.message) || String(err) };   // swallowed

CLI, same repository, same job, already fixed, with the diagnosis written out:

430      // TAGGED principal — the SDK dispatches on `canisterId`/`subnetId` being
431      // present and throws its internal UNREACHABLE_ERROR for a bare Principal.
432      // Passing one bare made every certificate check fail with "unreachable",
433      // which the catch below reported as an environment limitation — so this
434      // check, the one the root key exists to anchor, never actually ran.
435      principal: { canisterId: cid },

Why it matters. validateCertifiedHead throws on every call, is caught, and returns {ok: null}. verifyChainClient then takes the else branch at :274 and notes "certificate check unavailable … head taken from the query response" — while verify() still sets out.className = "lg-verify-out lg-ok" and prints the green ✓ N links verified. What is actually verified is a self-consistent hash chain over data one uncertified query replica returned.

The browser's catch attributes the throw to pocket-ic's BLS verifier, but the CLI three files away establishes the real cause is the bare-Principal dispatch, which fails on mainnet too.

index.html:1980-1981 claims the opposite of what happens: "re-computes the hash chain in your browser from the raw event bytes and compares the result against the IC-certified head — nothing is taken on the exchange's word."

A replica or gateway serving the archive queries can therefore serve a fabricated tape — canonicalEvent is public in ledger.js:32-101, so recomputing a self-consistent chain over it is trivial — return certificate = [], and the user clicking "Verify in my browser" gets a green tick.

Suggested direction. principal: { canisterId: cid }; drop disableTimeVerification: true on non-local hosts (with it on, a replayed old certificate over an old head also passes, so truncation stays invisible); and fail closed — certCheck.ok !== true should not render lg-ok/✓ off localhost.

Related, same file: verifyChain on the archive side (ArchiveCanister.mo:231-259) leaves one link per page unchecked while reporting ok = true — the Menese team quantified this precisely in #3 item 2, and we confirm their reading.

---

4. The ledger verifier fetches the IC root key from the host it is verifying

Where: src/frontend/src/main.js:964-968; contrast scripts/verify_ledger.mjs:211-221

964      rawAgent = await HttpAgent.create({ host: window.location.origin, rootKey: canisterEnv?.IC_ROOT_KEY });
967      try { await rawAgent.fetchRootKey(); } catch { /* mainnet: built-in key */ }

The CLI, correctly guarded, with the rationale at :200-210: "calling it unconditionally means a hostile gateway hands us its own key, signs a forged head with the matching secret, and this verifier prints 'certificate VALID'".

Why it matters. There is no host check. On https://multidex.ai the call goes to the app's own origin at /api/v2/status, and whatever root_key comes back overwrites the SDK's built-in mainnet key. The comment /* mainnet: built-in key */ assumes the call fails on mainnet; it does not — the IC HTTP gateway proxies /api/v2/status. So the anchor of the certificate check is supplied by the party being checked.

Together with item 3 this means the Ledger page cannot presently detect a hostile replica or gateway at all — which is the specific adversary it is built for.

Suggested direction. Apply the CLI's guard: call fetchRootKey() only when the hostname is localhost / 127.0.0.1 / *.localhost. The other five HttpAgent.create sites (main.js:1139, 1165, 1346, 1398, 6196) correctly pass rootKey: canisterEnv?.IC_ROOT_KEY and never call it — only this one differs.

---

Items 1 and 2 should land in one change. We are happy to open PRs.

— Ravenith, OhShii Labs

[dominicwilliams]

Confirmed and fixed, with one item still open that I want to be precise about rather than claim.

#4.3 + #4.4 — the ledger verifier has never verified anything. Confirmed exactly as filed. ledger.js:139-147 passed a bare Principal where the SDK dispatches on { canisterId }, so it threw on every call in every environment; the throw was swallowed at :153-158 into {ok: null}; and verify() painted lg-ok and "✓ N links verified" regardless. Meanwhile index.html claimed nothing is taken on the exchange's word. A hostile replica could serve a fabricated tape with certificate = [] and the page ticked green.

The same bug had already been diagnosed and fixed in scripts/verify_ledger.mjs, with the reasoning written out in a comment. It was fixed in the tool nobody runs and left broken in the one every user sees.

A sequencing hazard your report surfaced indirectly, which is the part worth recording. Verification found that main.js:975-978 called fetchRootKey() unconditionally with no hostname guard — so on mainnet the root key would come from the very host being verified. That was latent only because the certificate check always threw. Fixing your finding alone would have promoted it from latent to a live "certificate VALID" bypass, which is strictly worse than the honest failure you found. The two had to land together, and did: the call is now gated on an exact-hostname isLocalReplicaOrigin() check (never a substring, so localhost.evil.example is not mistaken for a local replica), ported from the CLI's guard.

Pinned by tests/frontend_security.test.mjs, 53 assertions, which fails on the pre-fix tree.

Still open, and I would rather say so than let the drop imply otherwise: the CSP is verified by configuration, not by response headers. security_policy: "standard" is set on the shipping asset config, and the build was audited for what standard would break — no eval, no WASM, no cross-origin fetches, no external fonts. But nothing has been curl -sD-'d against a deployed asset canister yet. Until someone does that, your finding is answered on paper. It is on the list for the next remote deploy.

The code is in #30, the first push to this mirror since the July snapshot; your line numbers against 0241cba still resolve. The SECURITY.md advisory link that 404'd — the reason this and the delegation finding in #11 landed publicly — is fixed in that same drop.