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/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/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__/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/__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/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/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/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); 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";