From dbb0489639107f7f58f3b3b96100e2a246240837 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Mon, 15 Jun 2026 22:41:26 -0400 Subject: [PATCH] fix(desktop): harden reminder decode against spec-valid and malformed input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RemindersPanel dereffed content.target unconditionally and decryptReminder trusted JSON.parse output as ReminderContent with no validation. A note-only reminder (no target — explicitly valid per NIP-ER) or malformed self-decrypted plaintext crashed the 60s due-check loop or rendered fake reminders. Make target optional, validate decrypted content fail-closed (object shape, known status, target/note structure), and guard every target deref with a note fallback. Mirror the relay's strict not_before parser so the client ignores values the relay rejects, and use 16 random bytes for the d-tag to meet the spec's 128-bit entropy MUST (randomUUID is only 122 bits). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../reminders/lib/reminderService.test.mjs | 141 ++++++++++++++++++ .../features/reminders/lib/reminderService.ts | 117 +++++++++++++-- .../features/reminders/lib/reminderTypes.ts | 3 +- .../features/reminders/ui/RemindersPanel.tsx | 11 +- 4 files changed, 255 insertions(+), 17 deletions(-) create mode 100644 desktop/src/features/reminders/lib/reminderService.test.mjs diff --git a/desktop/src/features/reminders/lib/reminderService.test.mjs b/desktop/src/features/reminders/lib/reminderService.test.mjs new file mode 100644 index 000000000..2a6f19ed1 --- /dev/null +++ b/desktop/src/features/reminders/lib/reminderService.test.mjs @@ -0,0 +1,141 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { parseNotBefore, parseReminderContent } from "./reminderService.ts"; + +const VALID_TARGET = { + eventId: "abc123", + channelId: "chan1", + preview: "hello world", + authorPubkey: "pk1", +}; + +function content(overrides = {}) { + return JSON.stringify({ + target: VALID_TARGET, + status: "pending", + ...overrides, + }); +} + +test("parseReminderContent_valid_target_reminder_returns_content", () => { + const result = parseReminderContent(content()); + assert.deepEqual(result, { + status: "pending", + target: VALID_TARGET, + note: undefined, + }); +}); + +test("parseReminderContent_note_only_reminder_returns_content", () => { + const result = parseReminderContent( + JSON.stringify({ status: "pending", note: "buy milk" }), + ); + assert.deepEqual(result, { + status: "pending", + target: undefined, + note: "buy milk", + }); +}); + +test("parseReminderContent_done_and_cancelled_statuses_accepted", () => { + assert.equal( + parseReminderContent(content({ status: "done" }))?.status, + "done", + ); + assert.equal( + parseReminderContent(content({ status: "cancelled" }))?.status, + "cancelled", + ); +}); + +test("parseReminderContent_invalid_json_returns_null", () => { + assert.equal(parseReminderContent("not json {"), null); +}); + +test("parseReminderContent_non_object_returns_null", () => { + assert.equal(parseReminderContent("42"), null); + assert.equal(parseReminderContent('"a string"'), null); + assert.equal(parseReminderContent("[1,2,3]"), null); + assert.equal(parseReminderContent("null"), null); +}); + +test("parseReminderContent_unknown_status_returns_null", () => { + assert.equal(parseReminderContent(content({ status: "bogus" })), null); + assert.equal( + parseReminderContent(JSON.stringify({ target: VALID_TARGET })), + null, + ); +}); + +test("parseReminderContent_neither_target_nor_note_returns_null", () => { + assert.equal( + parseReminderContent(JSON.stringify({ status: "pending" })), + null, + ); +}); + +test("parseReminderContent_empty_note_without_target_returns_null", () => { + assert.equal( + parseReminderContent(JSON.stringify({ status: "pending", note: "" })), + null, + ); +}); + +test("parseReminderContent_non_string_note_returns_null", () => { + assert.equal(parseReminderContent(content({ note: 5 })), null); +}); + +test("parseReminderContent_malformed_target_returns_null", () => { + assert.equal( + parseReminderContent(JSON.stringify({ status: "pending", target: {} })), + null, + ); + assert.equal( + parseReminderContent( + JSON.stringify({ status: "pending", target: "string" }), + ), + null, + ); + assert.equal( + parseReminderContent( + JSON.stringify({ + status: "pending", + target: { ...VALID_TARGET, preview: 7 }, + }), + ), + null, + ); +}); + +test("parseReminderContent_unknown_fields_are_ignored", () => { + const result = parseReminderContent(content({ extra: "ignored" })); + assert.deepEqual(result, { + status: "pending", + target: VALID_TARGET, + note: undefined, + }); +}); + +test("parseNotBefore_valid_digits_returns_number", () => { + assert.equal(parseNotBefore("0"), 0); + assert.equal(parseNotBefore("1700000000"), 1_700_000_000); +}); + +test("parseNotBefore_leading_zero_returns_undefined", () => { + assert.equal(parseNotBefore("007"), undefined); + assert.equal(parseNotBefore("01"), undefined); +}); + +test("parseNotBefore_non_digit_returns_undefined", () => { + assert.equal(parseNotBefore("123abc"), undefined); + assert.equal(parseNotBefore("12.5"), undefined); + assert.equal(parseNotBefore("-5"), undefined); + assert.equal(parseNotBefore(" 5"), undefined); + assert.equal(parseNotBefore(""), undefined); +}); + +test("parseNotBefore_above_max_safe_integer_returns_undefined", () => { + assert.equal(parseNotBefore("9007199254740991"), 9_007_199_254_740_991); + assert.equal(parseNotBefore("9007199254740992"), undefined); +}); diff --git a/desktop/src/features/reminders/lib/reminderService.ts b/desktop/src/features/reminders/lib/reminderService.ts index 155ade65a..1c5a5bc6f 100644 --- a/desktop/src/features/reminders/lib/reminderService.ts +++ b/desktop/src/features/reminders/lib/reminderService.ts @@ -23,31 +23,122 @@ function extractDTag(event: RelayEvent): string | null { return tag?.[1] ?? null; } +/** + * Generate a reminder `d`-tag with 128 bits of entropy (NIP-ER line 58 MUST). + * `crypto.randomUUID()` is UUIDv4 = only 122 random bits, so use 16 raw bytes. + */ +function randomDTag(): string { + const bytes = crypto.getRandomValues(new Uint8Array(16)); + return Array.from(bytes, (b) => b.toString(16).padStart(2, "0")).join(""); +} + function extractNotBefore(event: RelayEvent): number | undefined { const tag = event.tags.find((t) => t[0] === "not_before"); - if (!tag?.[1]) return undefined; - const val = Number.parseInt(tag[1], 10); - return Number.isNaN(val) ? undefined : val; + return tag?.[1] ? parseNotBefore(tag[1]) : undefined; +} + +/** + * Parse a NIP-ER `not_before` tag value, mirroring the relay's strict + * validator (NIP-ER line 60): ASCII digits only, no leading zero except "0", + * and within `Number.MAX_SAFE_INTEGER`. Returns undefined for any value the + * relay would reject, so the client ignores reminders the relay calls malformed. + */ +export function parseNotBefore(raw: string): number | undefined { + if (!/^(0|[1-9][0-9]*)$/.test(raw)) return undefined; + const val = Number(raw); + return val <= Number.MAX_SAFE_INTEGER ? val : undefined; +} + +/** + * Validate decrypted reminder plaintext against the shape this client writes, + * returning a typed content object or null. NIP-ER (Content section) requires + * clients to ignore plaintext that is not a JSON object, has an unknown + * `status`, or has a malformed target/note — so anything off-shape fails closed. + */ +export function parseReminderContent( + plaintext: string, +): ReminderContent | null { + let parsed: unknown; + try { + parsed = JSON.parse(plaintext); + } catch { + return null; + } + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + return null; + } + + const obj = parsed as Record; + if ( + obj.status !== "pending" && + obj.status !== "done" && + obj.status !== "cancelled" + ) { + return null; + } + if (obj.note !== undefined && typeof obj.note !== "string") return null; + + let target: ReminderTarget | undefined; + if (obj.target !== undefined) { + const parsedTarget = parseTarget(obj.target); + if (!parsedTarget) return null; + target = parsedTarget; + } + + // A reminder must reference a target or carry a non-empty note. + if (!target && !(typeof obj.note === "string" && obj.note.length > 0)) { + return null; + } + + return { status: obj.status, target, note: obj.note as string | undefined }; +} + +function parseTarget(value: unknown): ReminderTarget | null { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return null; + } + const t = value as Record; + if ( + typeof t.eventId !== "string" || + typeof t.channelId !== "string" || + typeof t.preview !== "string" || + typeof t.authorPubkey !== "string" + ) { + return null; + } + return { + eventId: t.eventId, + channelId: t.channelId, + preview: t.preview, + authorPubkey: t.authorPubkey, + }; } async function decryptReminder(event: RelayEvent): Promise { const dTag = extractDTag(event); if (!dTag) return null; + let plaintext: string; try { - const plaintext = await nip44DecryptFromSelf(event.content); - const content = JSON.parse(plaintext) as ReminderContent; - return { - id: dTag, - notBefore: extractNotBefore(event), - content, - createdAt: event.created_at, - eventId: event.id, - }; + plaintext = await nip44DecryptFromSelf(event.content); } catch { console.warn("[reminderService] failed to decrypt reminder:", event.id); return null; } + + const content = parseReminderContent(plaintext); + if (!content) { + console.warn("[reminderService] ignoring malformed reminder:", event.id); + return null; + } + + return { + id: dTag, + notBefore: extractNotBefore(event), + content, + createdAt: event.created_at, + eventId: event.id, + }; } export async function fetchReminders(pubkey: string): Promise { @@ -66,7 +157,7 @@ export async function createReminder( notBefore: number, note?: string, ): Promise { - const dTag = crypto.randomUUID(); + const dTag = randomDTag(); const content: ReminderContent = { target, note, diff --git a/desktop/src/features/reminders/lib/reminderTypes.ts b/desktop/src/features/reminders/lib/reminderTypes.ts index acb56913d..ea727b7d1 100644 --- a/desktop/src/features/reminders/lib/reminderTypes.ts +++ b/desktop/src/features/reminders/lib/reminderTypes.ts @@ -12,7 +12,8 @@ export type ReminderTarget = { }; export type ReminderContent = { - target: ReminderTarget; + /** Target message. Absent for note-only reminders (NIP-ER allows either). */ + target?: ReminderTarget; /** Optional user-provided note. */ note?: string; status: ReminderStatus; diff --git a/desktop/src/features/reminders/ui/RemindersPanel.tsx b/desktop/src/features/reminders/ui/RemindersPanel.tsx index 9c49b64ff..f36caa855 100644 --- a/desktop/src/features/reminders/ui/RemindersPanel.tsx +++ b/desktop/src/features/reminders/ui/RemindersPanel.tsx @@ -123,9 +123,11 @@ function ReminderRow({

- {reminder.content.target.preview || "Message"} + {reminder.content.target?.preview || + reminder.content.note || + "Reminder"}

- {reminder.content.note ? ( + {reminder.content.target && reminder.content.note ? (

{reminder.content.note}

@@ -207,7 +209,10 @@ export function RemindersPanel({ pubkey }: { pubkey: string }) { if (!r.notBefore) continue; if (r.notBefore > lastCheck && r.notBefore <= now) { toast("Reminder due", { - description: r.content.target.preview || "A reminder is waiting", + description: + r.content.target?.preview || + r.content.note || + "A reminder is waiting", icon: , }); }