From 4f348d0a9fd56daa63cdcb998aa4ac7f73dfc98d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 26 Sep 2026 17:52:52 -0700 Subject: [PATCH] fix(routes): harden POST /me/inbox/send A recipient carrying CR, LF or NUL is a 400 before anything is appended or handed to deliver, so a header-injection payload never reaches the envelope cache or the host transport. The Message-ID is minted from the addr-spec of the sender (@intx/mime extractAddrSpec), so a senderAddressFor that returns "Name " no longer 500s, and a sender with no addr-spec falls back to hub.invalid. A RangeError from building the frame, including the frame-byte cap the route now enforces with assertMailboxFrameBytes, is a 400. --- e2e/mount-send.test.ts | 61 +++++++++++++++++++++++++++++++++++++-- e2e/write-failure.test.ts | 5 ++-- src/frame.test.ts | 12 ++++++++ src/frame.ts | 13 ++++++--- src/mount.ts | 27 +++++++++++++++-- 5 files changed, 107 insertions(+), 11 deletions(-) diff --git a/e2e/mount-send.test.ts b/e2e/mount-send.test.ts index 86c6450..98ee8b0 100644 --- a/e2e/mount-send.test.ts +++ b/e2e/mount-send.test.ts @@ -8,6 +8,7 @@ import { } from "../src/mount.js"; import { createInMemoryMailboxEventBus } from "../src/bus.js"; import { openNativeMailboxStore } from "../src/native-store.js"; +import { MAX_MAILBOX_FRAME_BYTES } from "../src/write.js"; import { allowAllGrants, mountAs, withTestDb, seedScope } from "./helpers.js"; import type { MailboxDb } from "../src/db.js"; @@ -20,14 +21,14 @@ beforeEach(async () => { await seedScope(db, SCOPE.tenantId, SCOPE.principalId); }); -function buildApp(deliveries: OutgoingMailboxMessage[]) { +function buildApp(deliveries: OutgoingMailboxMessage[], sender = FROM) { const app = mountAs( SCOPE, createMailboxRoutes({ db, requireGrant: allowAllGrants, bus: createInMemoryMailboxEventBus(), - senderAddressFor: () => FROM, + senderAddressFor: () => sender, deliver: (message) => { deliveries.push(message); }, @@ -124,6 +125,62 @@ describe("POST /me/inbox/send", () => { expect(deliveries).toHaveLength(0); }); + function send(app: ReturnType, body: object) { + return app.request("/me/inbox/send", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); + } + + test("400s on a recipient carrying CR, LF or NUL, before any append or deliver", async () => { + const deliveries: OutgoingMailboxMessage[] = []; + const app = buildApp(deliveries); + for (const to of [ + "bob@example.com\r\nBcc: evil@example.com", + "bob@example.com\nX: 1", + "bob\u0000@example.com", + ]) { + const res = await send(app, { to: [to], body: "Hi" }); + expect(res.status).toBe(400); + } + expect(deliveries).toHaveLength(0); + const sent = await openNativeMailboxStore(db, { ...SCOPE, folder: "Sent" }); + expect(sent.messages).toHaveLength(0); + }); + + test("mints the Message-ID from the addr-spec of a display-name sender", async () => { + const deliveries: OutgoingMailboxMessage[] = []; + const app = buildApp(deliveries, "Pat "); + const res = await send(app, { to: ["bob@example.com"], body: "Hi" }); + expect(res.status).toBe(200); + const { messageId } = (await res.json()) as { messageId: string }; + expect(messageId).toMatch(/^<[^<>]+@t1\.example>$/); + expect(deliveries[0]!.from).toBe("Pat "); + }); + + test("mints under hub.invalid when the host's sender has no addr-spec", async () => { + const deliveries: OutgoingMailboxMessage[] = []; + const app = buildApp(deliveries, "Pat"); + const res = await send(app, { to: ["bob@example.com"], body: "Hi" }); + expect(res.status).toBe(200); + const { messageId } = (await res.json()) as { messageId: string }; + expect(messageId).toMatch(/^<[^<>]+@hub\.invalid>$/); + }); + + test("400s on a frame over the size cap without calling deliver", async () => { + const deliveries: OutgoingMailboxMessage[] = []; + const app = buildApp(deliveries); + const res = await send(app, { + to: ["bob@example.com"], + body: "x".repeat(MAX_MAILBOX_FRAME_BYTES), + }); + expect(res.status).toBe(400); + expect(deliveries).toHaveLength(0); + const sent = await openNativeMailboxStore(db, { ...SCOPE, folder: "Sent" }); + expect(sent.messages).toHaveLength(0); + }); + test("403s with no resolvable principal", async () => { const app = mountAs( null, diff --git a/e2e/write-failure.test.ts b/e2e/write-failure.test.ts index 242f6bb..df9f655 100644 --- a/e2e/write-failure.test.ts +++ b/e2e/write-failure.test.ts @@ -166,13 +166,13 @@ describe("a refused append", () => { }); describe("NUL in header and envelope fields", () => { - test("send stores the message with NUL dropped from subject and recipient", async () => { + test("send stores the message with NUL dropped from the subject", async () => { const app = sendApp([]); const res = await app.request("/me/inbox/send", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ - to: ["bob\u0000@example.com"], + to: ["bob@example.com"], subject: "hi\u0000", body: "b", }), @@ -183,7 +183,6 @@ describe("NUL in header and envelope fields", () => { folder: "Sent", }); expect(store.messages[0]?.envelope.subject).toBe("hi"); - expect(store.messages[0]?.envelope.to).toEqual(["bob@example.com"]); }); test("writeMailboxMessage stores a NUL subject without it", async () => { diff --git a/src/frame.test.ts b/src/frame.test.ts index cf09c3f..2a7784c 100644 --- a/src/frame.test.ts +++ b/src/frame.test.ts @@ -173,6 +173,18 @@ describe("Message-ID domain fallback", () => { expect(generateMessageId("no-at-sign").endsWith("@local>")).toBe(true); }); + test("uses the addr-spec's domain when the sender carries a display form", () => { + const id = generateMailboxMessageId(""); + expect(id).toMatch(MESSAGE_ID); + expect(id.endsWith("@t.example>")).toBe(true); + }); + + test("never copies a trailing comment into the id", () => { + const id = generateMailboxMessageId("p@t.example (c)"); + expect(id).toMatch(MESSAGE_ID); + expect(id).not.toContain("("); + }); + test("mints a distinct id per call", () => { expect(generateMailboxMessageId("bot@example.com")).not.toBe( generateMailboxMessageId("bot@example.com"), diff --git a/src/frame.ts b/src/frame.ts index b57d15d..740c991 100644 --- a/src/frame.ts +++ b/src/frame.ts @@ -1,4 +1,5 @@ import { + extractAddrSpec, extractPartByPath, formatRFC2822Date, parseHeaderSection, @@ -30,12 +31,16 @@ export const MESSAGE_ID_FALLBACK_DOMAIN = "hub.invalid"; /** * Mint a Message-ID for a frame this package authors: ``, where - * the domain is the sender's, or `hub.invalid` when the sender address has no - * parseable domain. + * the domain is that of the sender's addr-spec (`Name ` included), or + * `hub.invalid` when the sender address has no parseable addr-spec. */ export function generateMailboxMessageId(fromAddress: string): string { - const at = fromAddress.lastIndexOf("@"); - const domain = at === -1 ? "" : fromAddress.slice(at + 1).trim(); + let addrSpec = ""; + try { + addrSpec = extractAddrSpec(fromAddress); + } catch {} + const at = addrSpec.lastIndexOf("@"); + const domain = at === -1 ? "" : addrSpec.slice(at + 1).trim(); return `<${crypto.randomUUID()}@${domain === "" ? MESSAGE_ID_FALLBACK_DOMAIN : domain}>`; } diff --git a/src/mount.ts b/src/mount.ts index 27b4c7b..c7a2bb7 100644 --- a/src/mount.ts +++ b/src/mount.ts @@ -24,6 +24,7 @@ import { generateMailboxMessageId, headerValue, } from "./frame.js"; +import { assertMailboxFrameBytes } from "./write.js"; const logger = getLogger(["corbits-mailbox", "mount"]); @@ -444,7 +445,10 @@ export function createMailboxRoutes( "`deliver` — this package owns no transport.", responses: { 200: { description: "The Sent copy's messageId and uid" }, - 400: { description: "Malformed body" }, + 400: { + description: + "Malformed body, a recipient with CR, LF or NUL, or a frame over the size cap", + }, 403: { description: "No resolvable principalId" }, }, }), @@ -463,6 +467,16 @@ export function createMailboxRoutes( return c.json({ error: parsed.summary }, 400); } + // A recipient is a header value and an envelope address: a CR or LF + // would inject headers into the frame, and none of them belongs in + // either. + if (parsed.to.some((address) => /[\r\n\0]/.test(address))) { + return c.json( + { error: "recipients must not contain CR, LF or NUL" }, + 400, + ); + } + const fromAddress = headerValue(await senderAddressFor(resolved)); const messageId = generateMailboxMessageId(fromAddress); const subject = parsed.subject ?? ""; @@ -507,7 +521,16 @@ export function createMailboxRoutes( }; if (inReplyTo !== undefined) frameArgs.inReplyTo = inReplyTo; if (references !== undefined) frameArgs.references = references; - const raw = buildMailFrame(frameArgs); + let raw: Uint8Array; + try { + raw = buildMailFrame(frameArgs); + assertMailboxFrameBytes(raw); + } catch (err) { + if (err instanceof RangeError) { + return c.json({ error: err.message }, 400); + } + throw err; + } const sentStore = await openNativeMailboxStore( db,