From 96f0608a98270c19ee0880585f20979d6ca84efd Mon Sep 17 00:00:00 2001 From: cdonnachie Date: Tue, 1 Sep 2026 10:34:10 -0400 Subject: [PATCH 1/2] fix(app): sanitise untrusted request text before display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A connect request supplies `app`, `origin` and the challenge, and the approval screen renders all three so the user can decide what they are agreeing to. `cleanString` rejects C0 controls and DEL, which stops a request breaking the layout — but not from reordering it. Bidirectional overrides (U+202A-202E, U+2066-2069) reverse the visual order of what follows, so a crafted request can read as one thing on screen while the signature covers another. No invalid UTF-8 and no control character is needed, and today such a request is accepted and rendered verbatim. Adds `@lib/displayText`: `sanitizeForDisplay` replaces bidi controls, C1, and invisible spaces with U+FFFD, and `hasUnsafeDisplayChars` reports whether anything was there. Replacing rather than stripping is deliberate — a silently cleaned string fails the user the same way the raw one does, because either way they cannot tell something was removed. Applied at every site that renders request-supplied text: the sign, PSBT, mint and swap request panels, and the two action pages. The sign panel also warns when the request carried hidden formatting, so the replacement characters are explained rather than merely puzzling. U+200C ZWNJ and U+200D ZWJ are deliberately left alone: they are joiners, not directional controls, they cannot reorder text, and they are load-bearing in Devanagari and emoji sequences. Validation still happens in the protocol layer and sanitisation only here, so nothing changes about which requests are accepted, and the bytes signed remain exactly the bytes the request supplied. Also corrects the header comments in `sign.ts` and its test: signing is deterministic (RFC 6979 `deterministicK`), not random-k as documented. --- .../components/connect/MintRequestPanel.tsx | 7 +- .../connect/SwapAcceptRequestPanel.tsx | 7 +- .../connect/SwapCancelRequestPanel.tsx | 7 +- .../connect/SwapOfferRequestPanel.tsx | 7 +- packages/app/src/pages/Connect.tsx | 33 ++++++- packages/app/src/pages/SignAction.tsx | 5 +- packages/app/src/pages/SignTxAction.tsx | 5 +- .../lib/src/__tests__/displayText.test.ts | 97 +++++++++++++++++++ packages/lib/src/__tests__/sign.test.ts | 6 +- packages/lib/src/displayText.ts | 64 ++++++++++++ packages/lib/src/sign.ts | 8 +- 11 files changed, 220 insertions(+), 26 deletions(-) create mode 100644 packages/lib/src/__tests__/displayText.test.ts create mode 100644 packages/lib/src/displayText.ts diff --git a/packages/app/src/components/connect/MintRequestPanel.tsx b/packages/app/src/components/connect/MintRequestPanel.tsx index 0d645b9..8b35def 100644 --- a/packages/app/src/components/connect/MintRequestPanel.tsx +++ b/packages/app/src/components/connect/MintRequestPanel.tsx @@ -30,6 +30,7 @@ import Card from "@app/components/Card"; import { feeRate as feeRateSignal } from "@app/signals"; import { embeddableContentBytes } from "@app/svgSanitize"; import type { MintRequest } from "@app/connect/protocol"; +import { sanitizeForDisplay } from "@lib/displayText"; /** * Data URL for the preview, built from the bytes that will actually be @@ -137,8 +138,8 @@ export default function MintRequestPanel({ Requested by - {request.app ? `${request.app} — ` : ""} - {request.origin ?? "(no origin provided)"} + {request.app ? `${sanitizeForDisplay(request.app)} — ` : ""} + {request.origin ? sanitizeForDisplay(request.origin) : "(no origin provided)"} ) : ( @@ -248,7 +249,7 @@ export default function MintRequestPanel({ {autoReturn && ( After approving you will be sent back to{" "} - {request.app || "the app"} at {request.origin}, which + {request.app ? sanitizeForDisplay(request.app) : "the app"} at {sanitizeForDisplay(request.origin ?? "")}, which receives the result automatically. )} diff --git a/packages/app/src/components/connect/SwapAcceptRequestPanel.tsx b/packages/app/src/components/connect/SwapAcceptRequestPanel.tsx index 884b5ae..7fd1991 100644 --- a/packages/app/src/components/connect/SwapAcceptRequestPanel.tsx +++ b/packages/app/src/components/connect/SwapAcceptRequestPanel.tsx @@ -27,6 +27,7 @@ import { previewSwapAccept, type SwapAcceptPreview } from "@app/connect/swapFlow import { electrumStatus } from "@app/signals"; import { ElectrumStatus } from "@app/types"; import type { SwapAcceptRequest } from "@app/connect/protocol"; +import { sanitizeForDisplay } from "@lib/displayText"; export default function SwapAcceptRequestPanel({ request, @@ -98,8 +99,8 @@ export default function SwapAcceptRequestPanel({ Requested by - {request.app ? `${request.app} — ` : ""} - {request.origin ?? "(no origin provided)"} + {request.app ? `${sanitizeForDisplay(request.app)} — ` : ""} + {request.origin ? sanitizeForDisplay(request.origin) : "(no origin provided)"} ) : ( @@ -216,7 +217,7 @@ export default function SwapAcceptRequestPanel({ {autoReturn && ( After approving you will be sent back to{" "} - {request.app || "the app"} at {request.origin}, which + {request.app ? sanitizeForDisplay(request.app) : "the app"} at {sanitizeForDisplay(request.origin ?? "")}, which receives the result automatically. )} diff --git a/packages/app/src/components/connect/SwapCancelRequestPanel.tsx b/packages/app/src/components/connect/SwapCancelRequestPanel.tsx index c09a241..dc1d855 100644 --- a/packages/app/src/components/connect/SwapCancelRequestPanel.tsx +++ b/packages/app/src/components/connect/SwapCancelRequestPanel.tsx @@ -23,6 +23,7 @@ import TokenContent from "@app/components/TokenContent"; import db from "@app/db"; import { SwapStatus } from "@app/types"; import { photonsToRXD } from "@lib/format"; +import { sanitizeForDisplay } from "@lib/displayText"; import type { SwapCancelRequest } from "@app/connect/protocol"; export default function SwapCancelRequestPanel({ @@ -64,8 +65,8 @@ export default function SwapCancelRequestPanel({ Requested by - {request.app ? `${request.app} — ` : ""} - {request.origin ?? "(no origin provided)"} + {request.app ? `${sanitizeForDisplay(request.app)} — ` : ""} + {request.origin ? sanitizeForDisplay(request.origin) : "(no origin provided)"} ) : ( @@ -126,7 +127,7 @@ export default function SwapCancelRequestPanel({ {autoReturn && ( After approving you will be sent back to{" "} - {request.app || "the app"} at {request.origin}, which + {request.app ? sanitizeForDisplay(request.app) : "the app"} at {sanitizeForDisplay(request.origin ?? "")}, which receives the result automatically. )} diff --git a/packages/app/src/components/connect/SwapOfferRequestPanel.tsx b/packages/app/src/components/connect/SwapOfferRequestPanel.tsx index dd42ba1..a3369f4 100644 --- a/packages/app/src/components/connect/SwapOfferRequestPanel.tsx +++ b/packages/app/src/components/connect/SwapOfferRequestPanel.tsx @@ -22,6 +22,7 @@ import Card from "@app/components/Card"; import TokenContent from "@app/components/TokenContent"; import db from "@app/db"; import type { SwapOfferRequest } from "@app/connect/protocol"; +import { sanitizeForDisplay } from "@lib/displayText"; export default function SwapOfferRequestPanel({ request, @@ -58,8 +59,8 @@ export default function SwapOfferRequestPanel({ Requested by - {request.app ? `${request.app} — ` : ""} - {request.origin ?? "(no origin provided)"} + {request.app ? `${sanitizeForDisplay(request.app)} — ` : ""} + {request.origin ? sanitizeForDisplay(request.origin) : "(no origin provided)"} ) : ( @@ -118,7 +119,7 @@ export default function SwapOfferRequestPanel({ {autoReturn && ( After approving you will be sent back to{" "} - {request.app || "the app"} at {request.origin}, which + {request.app ? sanitizeForDisplay(request.app) : "the app"} at {sanitizeForDisplay(request.origin ?? "")}, which receives the offer automatically. )} diff --git a/packages/app/src/pages/Connect.tsx b/packages/app/src/pages/Connect.tsx index e4792cf..652b45f 100644 --- a/packages/app/src/pages/Connect.tsx +++ b/packages/app/src/pages/Connect.tsx @@ -81,6 +81,7 @@ import { import { withSwapWif, withWif } from "@app/wallet"; import { signMessageWithWif } from "@lib/sign"; import { PsbtError, psbtFromBase64, type Psbt } from "@lib/psbt"; +import { hasUnsafeDisplayChars, sanitizeForDisplay } from "@lib/displayText"; import { buildCallbackUrl, buildErrorCallbackUrl, @@ -776,6 +777,13 @@ function RequestPanel({ onReject: () => void; }) { const recognized = isRecognizedConnectChallenge(request.challenge); + // Bidi overrides and invisible characters can make this screen read as one + // thing while the signature covers another. They are replaced below; this + // says so, because a silently cleaned string is still a lie by omission. + const hiddenFormatting = + hasUnsafeDisplayChars(request.challenge) || + hasUnsafeDisplayChars(request.app ?? "") || + hasUnsafeDisplayChars(request.origin ?? ""); const addressMismatch = !!request.address && request.address !== signerAddress; @@ -811,8 +819,8 @@ function RequestPanel({ Requested by - {request.app ? `${request.app} — ` : ""} - {request.origin ?? "(no origin provided)"} + {request.app ? `${sanitizeForDisplay(request.app)} — ` : ""} + {request.origin ? sanitizeForDisplay(request.origin) : "(no origin provided)"} ) : ( @@ -839,6 +847,21 @@ function RequestPanel({ )} + {hiddenFormatting && ( + + + + Contains hidden formatting + + This request contains characters that can hide or reorder text + on screen. They are shown as {"\uFFFD"} below. Read what you are + signing carefully, and reject it if it does not look like what + you expected. + + + + )} + Message to sign @@ -851,7 +874,7 @@ function RequestPanel({ whiteSpace="pre-wrap" wordBreak="break-all" > - {request.challenge} + {sanitizeForDisplay(request.challenge)} @@ -874,8 +897,8 @@ function RequestPanel({ {autoReturn && ( - After signing you will be sent back to {request.app || "the app"} at{" "} - {request.origin}, which receives your address and signature + After signing you will be sent back to {request.app ? sanitizeForDisplay(request.app) : "the app"} at{" "} + {sanitizeForDisplay(request.origin ?? "")}, which receives your address and signature automatically. )} diff --git a/packages/app/src/pages/SignAction.tsx b/packages/app/src/pages/SignAction.tsx index 8ec86f8..ac8602e 100644 --- a/packages/app/src/pages/SignAction.tsx +++ b/packages/app/src/pages/SignAction.tsx @@ -83,6 +83,7 @@ import { isNonceConsumed, consumeNonce } from "@app/connect/consumedNonces"; import SignTxAction from "@app/pages/SignTxAction"; import type { SelectableInput } from "@lib/coinSelect"; import type { UnfinalizedInput } from "@lib/types"; +import { sanitizeForDisplay } from "@lib/displayText"; const DEV = import.meta.env.DEV === true; /** Bound both API fetches; a hung request must not hold a spend page open. */ @@ -576,10 +577,10 @@ function SignCoreAction() { verify a page there initiated it. `origin` is attacker-writable, so "requested by" would assert provenance we can't prove. */} Signing for - {req.origin} + {sanitizeForDisplay(req.origin ?? "")} Any website can open this screen. Only continue if you just started this - action on {req.origin}. + action on {sanitizeForDisplay(req.origin ?? "")}. diff --git a/packages/app/src/pages/SignTxAction.tsx b/packages/app/src/pages/SignTxAction.tsx index b6bd6a2..fd6292b 100644 --- a/packages/app/src/pages/SignTxAction.tsx +++ b/packages/app/src/pages/SignTxAction.tsx @@ -59,6 +59,7 @@ import { p2pkhScript } from "@lib/script"; import { photonsToRXD } from "@lib/format"; import { transferRadiant } from "@lib/transfer"; import type { SelectableInput } from "@lib/coinSelect"; +import { sanitizeForDisplay } from "@lib/displayText"; import { useLiveQuery } from "dexie-react-hooks"; import { isNativePlatform } from "@app/platform"; import { @@ -420,10 +421,10 @@ export default function SignTxAction() { Signing for - {req.origin} + {sanitizeForDisplay(req.origin ?? "")} Any website can open this screen. Only continue if you just started this send - on {req.origin}. + on {sanitizeForDisplay(req.origin ?? "")}. diff --git a/packages/lib/src/__tests__/displayText.test.ts b/packages/lib/src/__tests__/displayText.test.ts new file mode 100644 index 0000000..398e9f5 --- /dev/null +++ b/packages/lib/src/__tests__/displayText.test.ts @@ -0,0 +1,97 @@ +/** + * Display sanitisation for untrusted request text. + * + * The attack these guard against needs no invalid UTF-8 and no control + * character: a bidirectional override reverses the visual order of everything + * after it, so an approval screen can read as one thing while the bytes say + * another. `cleanString` in the connect protocol accepts such a string + * happily, because by its own rules there is nothing wrong with it. + */ +import { describe, expect, it } from "vitest"; + +import { hasUnsafeDisplayChars, sanitizeForDisplay } from "../displayText"; + +const RLO = "\u202E"; // U+202E right-to-left override +const PDF = "\u202C"; // U+202C pop directional formatting +const LRI = "\u2066"; // U+2066 left-to-right isolate +const ZWSP = "\u200B"; // U+200B zero-width space +const BOM = "\uFEFF"; // U+FEFF zero-width no-break space +const ZWJ = "\u200D"; // U+200D zero-width joiner +const ZWNJ = "\u200C"; // U+200C zero-width non-joiner +const C1 = "\u0085"; // U+0085 next line, a C1 control +const REPLACEMENT = "\uFFFD"; + +describe("hasUnsafeDisplayChars", () => { + it("passes ordinary text, including non-Latin scripts and emoji", () => { + for (const text of [ + "Photonic Wallet", + "https://hashmark.rxd.zone", + "\u0645\u062D\u0641\u0638\u0629", + "\u30A6\u30A9\u30EC\u30C3\u30C8", + "a" + ZWJ + "b", + "a" + ZWNJ + "b", + ]) { + expect(hasUnsafeDisplayChars(text)).toBe(false); + } + }); + + it("catches every class of hiding or reordering character", () => { + for (const text of [ + "app" + RLO + "moc.live", + "app" + PDF, + "app" + LRI, + "app" + ZWSP + "name", + BOM + "app", + "app" + C1 + "name", + "line" + String.fromCharCode(0x2028) + "break", + ]) { + expect(hasUnsafeDisplayChars(text)).toBe(true); + } + }); + + it("is not left stateful by a previous call", () => { + // A /g/ regex carries lastIndex between calls. Two identical calls must + // give identical answers, or the second request of a session renders + // differently from the first. + const hostile = "app" + RLO + "x"; + expect(hasUnsafeDisplayChars(hostile)).toBe(true); + expect(hasUnsafeDisplayChars(hostile)).toBe(true); + expect(hasUnsafeDisplayChars(hostile)).toBe(true); + }); +}); + +describe("sanitizeForDisplay", () => { + it("leaves ordinary text byte-identical", () => { + const text = "GlyphGalaxy - https://example.org"; + expect(sanitizeForDisplay(text)).toBe(text); + }); + + it("replaces rather than strips, so removal stays visible", () => { + // Stripping would render "gpj.exe" as innocuous text. The user must be + // able to see that something was taken out. + const hostile = "invoice" + RLO + "fdp.exe"; + const clean = sanitizeForDisplay(hostile); + expect(clean).not.toContain(RLO); + expect(clean).toContain(REPLACEMENT); + expect(clean).toBe("invoice" + REPLACEMENT + "fdp.exe"); + }); + + it("replaces every occurrence, not just the first", () => { + expect(sanitizeForDisplay(RLO + "a" + RLO + "b" + RLO)).toBe( + REPLACEMENT + "a" + REPLACEMENT + "b" + REPLACEMENT + ); + }); + + it("keeps joiners, so emoji and Indic text survive", () => { + const family = "\uD83D\uDC68\u200D\uD83D\uDC69\u200D\uD83D\uDC67"; + expect(sanitizeForDisplay(family)).toBe(family); + expect(sanitizeForDisplay("a" + ZWNJ + "b")).toBe("a" + ZWNJ + "b"); + }); + + it("never changes the length of the signed message it is not applied to", () => { + // Guards the module contract: sanitisation is display-only. Callers must + // pass the original string to the signer, never this output. + const original = "photonic:wallet-connect:v1:nonce:label"; + expect(sanitizeForDisplay(original)).toBe(original); + }); +}); diff --git a/packages/lib/src/__tests__/sign.test.ts b/packages/lib/src/__tests__/sign.test.ts index 306925b..3eeb2f2 100644 --- a/packages/lib/src/__tests__/sign.test.ts +++ b/packages/lib/src/__tests__/sign.test.ts @@ -7,8 +7,10 @@ * (the verifier the dApp/indexer actually runs), and every failure mode * (tamper, wrong address, malformed sig, control chars) must behave safely. * - * NOTE: `Message.sign` is NON-deterministic (random k). Tests therefore assert - * *verification*, never byte-equality of two signatures. + * NOTE: `Message.sign` derives `k` per RFC 6979, so it is deterministic in + * practice. Tests still assert *verification* rather than byte-equality, because + * the contract a dApp relies on is that a signature verifies - not that a + * particular byte string comes back. */ import { it, expect, describe } from "vitest"; import rjs from "@radiant-core/radiantjs"; diff --git a/packages/lib/src/displayText.ts b/packages/lib/src/displayText.ts new file mode 100644 index 0000000..e4e6060 --- /dev/null +++ b/packages/lib/src/displayText.ts @@ -0,0 +1,64 @@ +/** + * Making untrusted text safe to *show*. + * + * A connect request carries strings the wallet did not author - `app`, + * `origin`, and the challenge itself - and the approval screen renders them so + * the user can decide what they are agreeing to. That decision is only as good + * as the rendering. + * + * `cleanString` in the connect protocol already rejects C0 control characters + * and DEL, which stops a request breaking the layout. It does not stop a + * request **reordering** it. Unicode bidirectional overrides (U+202A-202E, + * U+2066-2069) change the visual order of the characters that follow, so a + * crafted `app` or challenge can display as one thing and be signed as + * another. That is the "Trojan Source" trick, and it needs no invalid UTF-8 + * and no control character to work. + * + * So: validate for *acceptance* in the protocol layer, sanitize for *display* + * here. The two are different jobs and must not be conflated. In particular + * this module is never applied to a message before signing: the bytes signed + * are always exactly the bytes the request supplied, and only what is painted + * on screen is altered - visibly. + */ + +/** + * Characters that can hide or reorder what is rendered. + * + * \u0000-\u001F C0 controls + * \u007F-\u009F DEL and C1 controls + * \u061C Arabic letter mark + * \u200B zero-width space + * \u200E-\u200F LRM / RLM + * \u2028-\u2029 line / paragraph separator + * \u202A-\u202E bidi embeddings and overrides + * \u2066-\u2069 bidi isolates + * \uFEFF zero-width no-break space + * + * Deliberately NOT included: U+200C ZWNJ and U+200D ZWJ. Both are joiners + * rather than directional controls - they cannot reorder text - and both are + * load-bearing in legitimate content, from Devanagari to emoji families (a + * family emoji is several people joined by ZWJ). Replacing them would corrupt + * honest app names to defend against an attack they cannot mount. + */ +const UNSAFE_DISPLAY_RE = + // eslint-disable-next-line no-control-regex + /[\u0000-\u001F\u007F-\u009F\u061C\u200B\u200E-\u200F\u2028-\u2029\u202A-\u202E\u2066-\u2069\uFEFF]/g; + +/** True if `text` contains anything that could hide or reorder its rendering. */ +export function hasUnsafeDisplayChars(text: string): boolean { + UNSAFE_DISPLAY_RE.lastIndex = 0; + return UNSAFE_DISPLAY_RE.test(text); +} + +/** + * Replace every hiding or reordering character with U+FFFD. + * + * Replaced rather than stripped, on purpose. Stripping would silently turn a + * hostile string into a clean-looking one, which fails the user the same way + * rendering it unchanged does: either way they cannot tell that anything was + * there. A visible replacement character says "something was removed here" + * without pretending to explain what. + */ +export function sanitizeForDisplay(text: string): string { + return text.replace(UNSAFE_DISPLAY_RE, "\uFFFD"); +} diff --git a/packages/lib/src/sign.ts b/packages/lib/src/sign.ts index e7e3d68..209865e 100644 --- a/packages/lib/src/sign.ts +++ b/packages/lib/src/sign.ts @@ -26,9 +26,11 @@ * and gating on explicit, per-request approval. * * The returned signature is a base64 compact recoverable signature, byte-for- - * byte what `Message.sign` produces and `Message.verify` consumes. Signing is - * NON-deterministic (random k), so two signatures over the same message - * differ; both verify. + * byte what `Message.sign` produces and `Message.verify` consumes. `k` is + * derived per RFC 6979 (`ECDSA.deterministicK`), so the same key and message + * produce the same bytes every time; `signRandomK` exists in radiantjs but + * this path does not use it. Verifiers must not rely on that either way - + * identify a signature by what it recovers to, never by its bytes. */ import rjs from "@radiant-core/radiantjs"; From e305509610cf9668dd4659ee51c3c2b8c98bcd4f Mon Sep 17 00:00:00 2001 From: cdonnachie Date: Tue, 1 Sep 2026 10:34:27 -0400 Subject: [PATCH 2/2] feat(app): show what an OP_RETURN output actually carries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The approval screen rendered every unrecognised locking script as "(non-standard output)". For a data carrier that is the least useful thing it could say: the output holds 0 satoshis, so nothing about the money is interesting, and the payload — the part that is published permanently and cannot be taken back — was the one thing not shown. Adds `@lib/dataOutput`, which describes such an output without interpreting it: total size, the raw payload, and the pushes it splits into when it is push-structured. Deciding that some bytes are a timestamp, a token or a message is a claim about someone else's protocol, and a wallet asserting it would be vouching for something it cannot check. `PsbtOutputSummary` carries the description, and the PSBT panel renders "Data output · N bytes" with the contents behind a toggle. Text is offered for a push only when it decodes as UTF-8 and contains nothing that could hide or reorder its own display, and is rendered through `sanitizeForDisplay` even then; hex is always shown beside it, because hex cannot misrepresent itself. Parsing is fail-open in the honest direction: a payload that is not a clean push sequence, is truncated, or carries an implausible number of pushes falls back to raw hex rather than a guess. --- .../components/connect/PsbtRequestPanel.tsx | 99 ++++++++++- packages/lib/src/__tests__/dataOutput.test.ts | 95 ++++++++++ packages/lib/src/dataOutput.ts | 163 ++++++++++++++++++ packages/lib/src/index.ts | 2 + packages/lib/src/psbt/analyze.ts | 11 ++ 5 files changed, 364 insertions(+), 6 deletions(-) create mode 100644 packages/lib/src/__tests__/dataOutput.test.ts create mode 100644 packages/lib/src/dataOutput.ts diff --git a/packages/app/src/components/connect/PsbtRequestPanel.tsx b/packages/app/src/components/connect/PsbtRequestPanel.tsx index 59ca645..532478c 100644 --- a/packages/app/src/components/connect/PsbtRequestPanel.tsx +++ b/packages/app/src/components/connect/PsbtRequestPanel.tsx @@ -24,9 +24,11 @@ import { Text, Button, } from "@chakra-ui/react"; +import { useState } from "react"; import { MdCloudUpload, MdUndo, MdWarning } from "react-icons/md"; import Card from "@app/components/Card"; import { photonsToRXD } from "@lib/format"; +import { sanitizeForDisplay } from "@lib/displayText"; import type { PsbtSignRequest } from "@app/connect/protocol"; import type { EnrichedPsbt } from "@app/connect/psbtFlow"; import type { PsbtInputSummary, PsbtOutputSummary } from "@lib/psbt"; @@ -95,6 +97,87 @@ function InputRow({ ); } +/** + * What an `OP_RETURN` output actually carries. + * + * This output pays nobody and cannot be spent, so its 0 RXD says nothing about + * what approving it does. The payload is the whole of it, and it is permanent — + * which makes it the one thing on this screen most worth showing, and the thing + * that used to read only as "(non-standard output)". + * + * Described, never interpreted. The wallet does not claim to know what another + * application's bytes mean; it says how big they are, how they are structured, + * and which parts happen to be readable. Text is rendered through + * `sanitizeForDisplay` because it is supplied by the requesting app, and hex is + * always available beside it for anything the text form cannot be trusted with. + */ +function DataOutputDetail({ data }: { data: NonNullable }) { + const [open, setOpen] = useState(false); + + return ( + + + + Data output + + + {data.size} bytes + + + + Publishes data permanently. Pays no one and can never be spent. + + + + {open && ( + + {data.pushes ? ( + data.pushes.map((push, index) => ( + + {push.text !== undefined && ( + + {sanitizeForDisplay(push.text)} + + )} + + {push.hex} + + + )) + ) : ( + + {data.payloadHex} + + )} + + Content supplied by the requesting app. Photonic shows it; it does + not vouch for what it means. + + + )} + + ); +} + function OutputRow({ output }: { output: PsbtOutputSummary }) { return ( - - {output.address ?? "(non-standard output)"} - + {output.data ? ( + + ) : ( + + {output.address ?? "(unrecognised script)"} + + )} {output.mine && ( To your wallet @@ -184,8 +271,8 @@ export default function PsbtRequestPanel({ Requested by - {request.app ? `${request.app} — ` : ""} - {request.origin ?? "(no origin provided)"} + {request.app ? `${sanitizeForDisplay(request.app)} — ` : ""} + {request.origin ? sanitizeForDisplay(request.origin) : "(no origin provided)"} ) : ( @@ -288,7 +375,7 @@ export default function PsbtRequestPanel({ {autoReturn && ( After approving you will be sent back to{" "} - {request.app || "the app"} at {request.origin}, which + {request.app ? sanitizeForDisplay(request.app) : "the app"} at {sanitizeForDisplay(request.origin ?? "")}, which receives the result automatically. )} diff --git a/packages/lib/src/__tests__/dataOutput.test.ts b/packages/lib/src/__tests__/dataOutput.test.ts new file mode 100644 index 0000000..25d1ebf --- /dev/null +++ b/packages/lib/src/__tests__/dataOutput.test.ts @@ -0,0 +1,95 @@ +/** + * Describing an OP_RETURN payload for the approval screen. + * + * The rule these tests hold the module to: describe, never interpret, and never + * offer text that could misrepresent itself. A payload is supplied by whoever + * built the request, so every branch here is reachable by an app that wants it + * to be. + */ +import { describe, expect, it } from "vitest"; + +import { readDataOutput } from "../dataOutput"; + +/** A real mainnet HashMark v1 record: magic, header, 32-byte digest. */ +const HASHMARK_V1 = + "6a08484153484d41524b02010120" + + "e2c55efb34b6e9d6db008ee72d56bf86456ab3f55ae76488ff677fda88df1f1e"; + +describe("readDataOutput", () => { + it("ignores anything that is not a data output", () => { + // A plain P2PKH locking script. + expect( + readDataOutput("76a91426ba056431ec69cf27eabeaab250d99ddbd895d288ac"), + ).toBeUndefined(); + expect(readDataOutput("")).toBeUndefined(); + expect(readDataOutput("zz")).toBeUndefined(); + }); + + it("describes a real HashMark record without interpreting it", () => { + const data = readDataOutput(HASHMARK_V1); + expect(data).toBeDefined(); + expect(data!.size).toBe(46); + expect(data!.pushes).toHaveLength(3); + // The magic reads as text; the digest does not, and is not forced to. + expect(data!.pushes![0]!.text).toBe("HASHMARK"); + expect(data!.pushes![2]!.text).toBeUndefined(); + expect(data!.pushes![2]!.hex).toHaveLength(64); + }); + + it("always reports the raw payload, even when pushes parse", () => { + const data = readDataOutput(HASHMARK_V1); + expect(data!.payloadHex).toBe(HASHMARK_V1.slice(2)); + }); + + it("falls back to raw hex when the payload is not push-structured", () => { + // OP_RETURN followed by OP_DUP, which is legal and not a push. + const data = readDataOutput("6a76"); + expect(data).toBeDefined(); + expect(data!.pushes).toBeUndefined(); + expect(data!.payloadHex).toBe("76"); + }); + + it("falls back to raw hex on a truncated push rather than guessing", () => { + // Claims 4 bytes, supplies 2. + const data = readDataOutput("6a04dead"); + expect(data!.pushes).toBeUndefined(); + expect(data!.payloadHex).toBe("04dead"); + }); + + it("offers no text for bytes that are not valid UTF-8", () => { + const data = readDataOutput("6a02fffe"); + expect(data!.pushes![0]!.text).toBeUndefined(); + expect(data!.pushes![0]!.hex).toBe("fffe"); + }); + + it("offers no text for a push that could reorder its own display", () => { + // "ab" + U+202E RIGHT-TO-LEFT OVERRIDE, valid UTF-8 and displayable - + // which is exactly why it must not be offered as text. + const bidi = Buffer.from("ab\u202E", "utf8").toString("hex"); + const push = (bidi.length / 2).toString(16).padStart(2, "0"); + const data = readDataOutput("6a" + push + bidi); + expect(data!.pushes![0]!.text).toBeUndefined(); + expect(data!.pushes![0]!.hex).toBe(bidi); + }); + + it("offers no text for a push carrying a newline", () => { + // A newline in a rendered payload can fabricate a line that looks like a + // field of its own. + const data = readDataOutput("6a0361_0a62".replace("_", "")); + expect(data!.pushes![0]!.text).toBeUndefined(); + }); + + it("refuses to describe an implausible number of pushes", () => { + const payload = "0141".repeat(40); // 40 single-byte pushes + const data = readDataOutput("6a" + payload); + expect(data!.pushes).toBeUndefined(); + expect(data!.payloadHex).toBe(payload); + }); + + it("never throws, whatever bytes it is given", () => { + const seeds = ["6a", "6a4c", "6a4d01", "6a4e00000001", "6aff", "6a00"]; + for (const hex of seeds) { + expect(() => readDataOutput(hex)).not.toThrow(); + } + }); +}); diff --git a/packages/lib/src/dataOutput.ts b/packages/lib/src/dataOutput.ts new file mode 100644 index 0000000..497d91f --- /dev/null +++ b/packages/lib/src/dataOutput.ts @@ -0,0 +1,163 @@ +/** + * Reading an `OP_RETURN` data output well enough to show it. + * + * The approval screen used to render every unrecognised locking script as + * "(non-standard output)". For a data carrier that is the least useful thing it + * could say: the output carries 0 satoshis, so nothing about the *money* is + * interesting, and the payload — the part that will be published permanently + * and cannot be taken back — was the one thing not shown. + * + * This module describes such an output. It does not interpret it. Deciding that + * some bytes are a timestamp, a token or a message is a claim about someone + * else's protocol, and a wallet asserting it would be vouching for something it + * cannot check. What it reports is structural: how big, how many pushes, and + * which of those pushes happen to be readable text. + */ + +const OP_RETURN = 0x6a; +const OP_PUSHDATA1 = 0x4c; +const OP_PUSHDATA2 = 0x4d; +const OP_PUSHDATA4 = 0x4e; +const MAX_DIRECT_PUSH = 0x4b; + +/** Refuse to describe an implausible payload rather than build a huge list. */ +const MAX_PUSHES = 32; + +export type DataPush = { + /** The push contents, lowercase hex. Always present. */ + readonly hex: string; + /** + * The same bytes as text, when they decode as UTF-8 and contain nothing that + * could hide or reorder what is rendered. + * + * Absent means "not safely displayable as text", never "empty". + */ + readonly text?: string; +}; + +export type DataOutput = { + /** Size of the whole scriptPubKey in bytes. */ + readonly size: number; + /** Everything after the leading OP_RETURN, lowercase hex. */ + readonly payloadHex: string; + /** + * The payload split into pushes, when it is a clean sequence of them. + * + * Absent when the payload is not push-structured, which is legal and not an + * error: the caller should fall back to showing `payloadHex`. + */ + readonly pushes?: readonly DataPush[]; +}; + +function hexToBytes(hex: string): Uint8Array | undefined { + if (hex.length % 2 !== 0 || !/^[0-9a-f]*$/i.test(hex)) return undefined; + const out = new Uint8Array(hex.length / 2); + for (let i = 0; i < out.length; i++) { + out[i] = parseInt(hex.slice(i * 2, i * 2 + 2), 16); + } + return out; +} + +function bytesToHex(bytes: Uint8Array): string { + let hex = ""; + for (const byte of bytes) hex += byte.toString(16).padStart(2, "0"); + return hex; +} + +/** + * Characters that can hide or reorder rendered text, plus C0/C1 controls. + * + * Kept in step with `sanitizeForDisplay` in ./displayText. Here it decides + * whether text is offered at all; there it is the second line of defence at the + * point of rendering. + */ +const UNSAFE_TEXT_RE = + // eslint-disable-next-line no-control-regex + /[\u0000-\u001F\u007F-\u009F\u061C\u200B\u200E-\u200F\u2028-\u2029\u202A-\u202E\u2066-\u2069\uFEFF]/u; + +const utf8 = new TextDecoder("utf-8", { fatal: true }); + +function readableText(bytes: Uint8Array): string | undefined { + if (bytes.length === 0) return undefined; + let text: string; + try { + text = utf8.decode(bytes); + } catch { + return undefined; + } + if (UNSAFE_TEXT_RE.test(text)) return undefined; + return text; +} + +/** Read the pushes in `payload`, or undefined if it is not a push sequence. */ +function readPushes(payload: Uint8Array): DataPush[] | undefined { + const pushes: DataPush[] = []; + let i = 0; + + while (i < payload.length) { + if (pushes.length >= MAX_PUSHES) return undefined; + + const opcode = payload[i]; + if (opcode === undefined) return undefined; + i += 1; + + let length: number; + if (opcode >= 0x01 && opcode <= MAX_DIRECT_PUSH) { + length = opcode; + } else if (opcode === OP_PUSHDATA1) { + if (i + 1 > payload.length) return undefined; + length = payload[i]!; + i += 1; + } else if (opcode === OP_PUSHDATA2) { + if (i + 2 > payload.length) return undefined; + length = payload[i]! | (payload[i + 1]! << 8); + i += 2; + } else if (opcode === OP_PUSHDATA4) { + if (i + 4 > payload.length) return undefined; + length = + payload[i]! + + payload[i + 1]! * 0x100 + + payload[i + 2]! * 0x10000 + + payload[i + 3]! * 0x1000000; + i += 4; + } else { + // A non-push opcode. Legal in a data output, but not something this + // module claims to describe - the caller shows raw hex instead. + return undefined; + } + + if (i + length > payload.length) return undefined; + const bytes = payload.subarray(i, i + length); + i += length; + + const text = readableText(bytes); + pushes.push({ + hex: bytesToHex(bytes), + ...(text === undefined ? {} : { text }), + }); + } + + return pushes; +} + +/** + * Describe a locking script if it is a data output, or undefined if it is not. + * + * Never throws: this runs on a script an app supplied, on the screen where the + * user decides whether to trust that app. + */ +export function readDataOutput(scriptHex: string): DataOutput | undefined { + const script = hexToBytes(scriptHex); + if (!script || script.length === 0 || script[0] !== OP_RETURN) { + return undefined; + } + + const payload = script.subarray(1); + const pushes = readPushes(payload); + + return { + size: script.length, + payloadHex: bytesToHex(payload), + ...(pushes === undefined ? {} : { pushes }), + }; +} diff --git a/packages/lib/src/index.ts b/packages/lib/src/index.ts index ca8ea1e..4d87843 100644 --- a/packages/lib/src/index.ts +++ b/packages/lib/src/index.ts @@ -43,6 +43,8 @@ export { } from "./wave"; export * from "./crypto"; export * from "./sign"; +export * from "./displayText"; +export * from "./dataOutput"; export * from "./encryption"; export * from "./timelock"; export * from "./reveal"; diff --git a/packages/lib/src/psbt/analyze.ts b/packages/lib/src/psbt/analyze.ts index 7241698..bbdbbea 100644 --- a/packages/lib/src/psbt/analyze.ts +++ b/packages/lib/src/psbt/analyze.ts @@ -7,6 +7,7 @@ */ import rjs from "@radiant-core/radiantjs"; import { Buffer } from "buffer"; +import { readDataOutput, type DataOutput } from "../dataOutput"; import { MAX_REASONABLE_FEE_RATE } from "../feePolicy"; import { transactionFromHex } from "../rjsCompat"; import { @@ -51,6 +52,14 @@ export type PsbtOutputSummary = { address?: string; mine: boolean; tokenBearing: boolean; + /** + * Present when this output is an `OP_RETURN` data carrier. + * + * A data output pays nobody, so its value says nothing; the payload is the + * whole of what it does, and it is permanent. Described rather than + * interpreted - see ../dataOutput. + */ + data?: DataOutput; }; export type PsbtAnalysis = { @@ -95,6 +104,7 @@ export function analyzePsbt(psbt: Psbt, ctx?: AnalyzeContext): PsbtAnalysis { const outputs: PsbtOutputSummary[] = tx.outputs.map((o) => { const script = o.script.toHex(); + const data = readDataOutput(script); return { script, // BN → string → bigint keeps values beyond 2^53 exact. @@ -102,6 +112,7 @@ export function analyzePsbt(psbt: Psbt, ctx?: AnalyzeContext): PsbtAnalysis { address: scriptToAddress(script, net), mine: ownScripts.has(script), tokenBearing: isTokenBearing(script), + ...(data === undefined ? {} : { data }), }; }); const totalOut = outputs.reduce((sum, o) => sum + o.value, 0n);