Skip to content

Commit 7d347ec

Browse files
committed
fix(desktop): bind only mention pairs this community vouches for
Visibility was the paste's whole trust boundary: a record whose label appeared in the inserted content was registered, and registration outlives the paste. Clipboard HTML is attacker-authored, so any copied page could carry `<span data-mention-pubkey="<their key>" data-mention-label="John Smith">@john Smith</span>` — plausible, plainly visible, and accepted. The chip lit, the send carried their key in its `p` tag, and the name stayed bound for the rest of the session. Seeing a label only proves the user saw a name, never that the name belongs to the key beside it; a Buzz provenance marker proves less still, since an attacker writes the same marker. The pair itself is now checked against state the community supplied. Local first — the mention candidates the composer would offer and the profile lookup the surface renders from, both already in hand — then the relay's own profile for the pubkey. Labels are compared against `collectProfileAliases`, the same alias set that turned a `p` tag into the label the copy carries, so a chip rendered off a kind-0 `name` or a NIP-05 handle verifies on the alias it was rendered from. Anything neither source names is dropped: the words paste as readable text and tag nobody. That relay lookup is what keeps the headline case working. A mention of someone who is not a member of the destination channel is exactly what no local directory can speak to, so it is the case the fetch exists for — and it is bounded by the record cap the parser already applies, reads the profile entries `useUsersBatchQuery` maintains before spending a request, and treats an entry those hooks call stale as no answer. Verification can need that round trip, so binding lands after the insertion. The visibility gate therefore runs a second time against what the composer holds when the answer arrives: an in-flight check whose paste the user has since deleted or replaced must not bind a name nothing on screen shows. A composer with no verifier binds nothing, and a failed lookup binds nothing — the pasted words are already in place and simply stay plain, so there is no partial state to retry. `selectBindableMentionIdentities` filters the verifier's answer back down to what it asked about, so the seam cannot widen into "the verifier decides what gets bound". `canonicalMentionLabel` moves out of `matchChipTextToLabel` and is shared, so the copy-side chip classifier and this check agree on what one name spelled two ways means. Tests: eleven cases over the hook the composers actually pass, driving the real `get_users_batch` seam — the forged pair dropped whether the key is unknown or known under another name, each local source and the relay vouching on their own, a mixed paste separated, and the cache freshness rule; four over the bindable selector; and a Playwright case pasting the visible forged shape into a DM, asserting the lookup ran, the chip stayed dark, and the sent event carried no impostor `p` tag. Removing the check lights the chip and fails it; ignoring only its verdict fails it the same way. The full clipboard spec passes. Signed-off-by: Matt Toohey <contact@matttoohey.com>
1 parent fcc9f32 commit 7d347ec

13 files changed

Lines changed: 797 additions & 72 deletions

desktop/src/features/forum/ui/ForumComposer.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,10 @@ export function ForumComposer({
360360
uploadFileRef.current = media.uploadFile;
361361
const registerMentionPubkeyRef = React.useRef(mentions.registerMentionPubkey);
362362
registerMentionPubkeyRef.current = mentions.registerMentionPubkey;
363+
const verifyMentionIdentitiesRef = React.useRef(
364+
mentions.verifyMentionIdentities,
365+
);
366+
verifyMentionIdentitiesRef.current = mentions.verifyMentionIdentities;
363367

364368
React.useEffect(() => {
365369
if (!richText.editor) return;
@@ -387,6 +391,7 @@ export function ForumComposer({
387391
clipboardData,
388392
preventDefault: () => event.preventDefault(),
389393
registerMentionPubkey: registerMentionPubkeyRef.current,
394+
verifyMentionIdentities: verifyMentionIdentitiesRef.current,
390395
view: _view,
391396
});
392397
}

desktop/src/features/messages/lib/mentionClipboard.test.mjs

Lines changed: 89 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
getBuzzCopyKind,
99
matchChipTextToLabel,
1010
parseMentionClipboardRecords,
11-
registerMentionClipboardIdentities,
11+
selectBindableMentionIdentities,
1212
selectVisibleMentionIdentities,
1313
} from "./mentionClipboard.ts";
1414

@@ -288,42 +288,102 @@ test("ignores a mention the extractor would mask as code", () => {
288288
);
289289
});
290290

291-
// ── registerMentionClipboardIdentities ────────────────────────────────
291+
// ── selectBindableMentionIdentities ───────────────────────────────────
292292

293-
test("registers each recovered pair with its agent flag", () => {
294-
const registered = [];
295-
registerMentionClipboardIdentities({
296-
html: buildMentionClipboardHtml({
293+
/** A verifier that vouches for every pair — isolates the other two gates. */
294+
const vouchForAll = async (records) => records;
295+
296+
test("keeps each recovered pair with its agent flag", async () => {
297+
assert.deepEqual(
298+
await selectBindableMentionIdentities({
299+
html: buildMentionClipboardHtml({
300+
text: "@John Smith and @Fizz",
301+
identities: [john, fizz],
302+
}),
297303
text: "@John Smith and @Fizz",
298-
identities: [john, fizz],
304+
verifyMentionIdentities: vouchForAll,
299305
}),
300-
registerMentionPubkey: (displayName, pubkey, options) =>
301-
registered.push([displayName, pubkey, options?.isAgent]),
302-
text: "@John Smith and @Fizz",
303-
});
304-
305-
assert.deepEqual(registered, [
306-
["John Smith", JOHN, false],
307-
["Fizz", FIZZ, true],
308-
]);
306+
[john, fizz],
307+
);
309308
});
310309

311-
test("registers nothing for a record the paste does not show", () => {
312-
const registered = [];
310+
test("keeps nothing for a record the paste does not show", async () => {
313311
// A crafted sidecar: an empty span rebinding a name the content never
314312
// carries, riding alongside one the user can actually see.
315-
registerMentionClipboardIdentities({
316-
html:
317-
`<span data-mention="" data-mention-pubkey="${ALEX}" ` +
318-
`data-mention-label="John Smith"></span>` +
319-
`<span data-mention="" data-mention-pubkey="${FIZZ}" ` +
320-
`data-mention-kind="agent" data-mention-label="Fizz">@Fizz</span>`,
321-
registerMentionPubkey: (displayName, pubkey) =>
322-
registered.push([displayName, pubkey]),
323-
text: "@Fizz take a look",
324-
});
313+
assert.deepEqual(
314+
await selectBindableMentionIdentities({
315+
html:
316+
`<span data-mention="" data-mention-pubkey="${ALEX}" ` +
317+
`data-mention-label="John Smith"></span>` +
318+
`<span data-mention="" data-mention-pubkey="${FIZZ}" ` +
319+
`data-mention-kind="agent" data-mention-label="Fizz">@Fizz</span>`,
320+
text: "@Fizz take a look",
321+
verifyMentionIdentities: vouchForAll,
322+
}),
323+
[fizz],
324+
);
325+
});
325326

326-
assert.deepEqual(registered, [["Fizz", FIZZ]]);
327+
test("keeps nothing for a visible pair trusted state will not vouch for", async () => {
328+
// The shape a hostile page carries: a plausible name against a key of its
329+
// choosing, written where the user *does* see it. Visibility is not the
330+
// question here — the verifier declining it is.
331+
const impostor = { label: "John Smith", pubkey: ALEX, isAgent: false };
332+
const asked = [];
333+
assert.deepEqual(
334+
await selectBindableMentionIdentities({
335+
html: buildMentionClipboardHtml({
336+
text: "@John Smith fixed the bug",
337+
identities: [impostor],
338+
}),
339+
text: "@John Smith fixed the bug",
340+
verifyMentionIdentities: async (records) => {
341+
asked.push(...records);
342+
return [];
343+
},
344+
}),
345+
[],
346+
);
347+
// The visible pair still reached the verifier: it is the trust answer that
348+
// dropped it, not an earlier gate quietly doing the work.
349+
assert.deepEqual(asked, [impostor]);
350+
});
351+
352+
test("binds only what it asked about, whatever the verifier returns", async () => {
353+
// The verifier is a seam, not an authority: a bug or a future implementation
354+
// that answers with a pair nobody copied must not widen the paste.
355+
assert.deepEqual(
356+
await selectBindableMentionIdentities({
357+
html: buildMentionClipboardHtml({
358+
text: "@John Smith fixed the bug",
359+
identities: [john],
360+
}),
361+
text: "@John Smith fixed the bug",
362+
verifyMentionIdentities: async (records) => [
363+
...records,
364+
{ label: "John Smith", pubkey: ALEX, isAgent: false },
365+
],
366+
}),
367+
[john],
368+
);
369+
});
370+
371+
test("does not consult the verifier when nothing is visible", async () => {
372+
let consulted = false;
373+
assert.deepEqual(
374+
await selectBindableMentionIdentities({
375+
html:
376+
`<span data-mention="" data-mention-pubkey="${ALEX}" ` +
377+
`data-mention-label="John Smith"></span>`,
378+
text: "look at this",
379+
verifyMentionIdentities: async () => {
380+
consulted = true;
381+
return [];
382+
},
383+
}),
384+
[],
385+
);
386+
assert.equal(consulted, false, "a hidden record must cost no lookup");
327387
});
328388

329389
// ── matchChipTextToLabel ──────────────────────────────────────────────

desktop/src/features/messages/lib/mentionClipboard.ts

Lines changed: 59 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,22 @@ export const MENTION_LABEL_ATTRIBUTE = "data-mention-label";
2828
/** Full channel-reference label, same partial-selection role as above. */
2929
export const CHANNEL_LABEL_ATTRIBUTE = "data-channel-label";
3030

31+
/**
32+
* Canonical form for comparing two spellings of one mention label.
33+
*
34+
* Tolerates what a label picks up in transit and nothing more: a pasteboard
35+
* round trip swaps spaces for U+00A0, markup gains padding, and mention
36+
* resolution is case-insensitive end to end. Every clipboard comparison of
37+
* two labels goes through here, so "the same name" means one thing across the
38+
* copy-side chip classifier and the paste-side trust check.
39+
*/
40+
export function canonicalMentionLabel(value: string): string {
41+
return value
42+
.replace(/\u00a0/g, " ")
43+
.trim()
44+
.toLowerCase();
45+
}
46+
3147
/** How a chip's copied text relates to the full label it declares. */
3248
export type ChipTextMatch = "full" | "truncated" | "fragment";
3349

@@ -62,18 +78,15 @@ export function matchChipTextToLabel(
6278
label: string,
6379
sigil: "@" | "#",
6480
): ChipTextMatch {
65-
const canonical = (value: string) =>
66-
value
67-
.replace(/\u00a0/g, " ")
68-
.trim()
69-
.toLowerCase();
70-
const body = canonical(text);
81+
const body = canonicalMentionLabel(text);
7182
const matches = (form: string) => body === form || body === `${sigil}${form}`;
72-
if (matches(canonical(label))) return "full";
83+
if (matches(canonicalMentionLabel(label))) return "full";
7384
// Derived from the helper the chips render with, so the tolerated form
7485
// cannot drift from what a fully selected capped chip actually carries.
7586
const truncated = truncateInlineChipLabel(label);
76-
if (truncated !== label && matches(canonical(truncated))) return "truncated";
87+
if (truncated !== label && matches(canonicalMentionLabel(truncated))) {
88+
return "truncated";
89+
}
7790
return "fragment";
7891
}
7992

@@ -333,35 +346,54 @@ export function selectVisibleMentionIdentities(
333346
}
334347

335348
/**
336-
* Teach a composer every identity a Buzz copy carried *and* showed.
349+
* Narrow copied records to the pairs trusted Buzz state vouches for.
337350
*
338-
* Registration is what makes a pasted multi-word name known to the mention
339-
* decorations *and* to the send-time extractor, so the chip re-lights and the
340-
* original pubkey survives the round trip.
351+
* Must resolve to a subset of what it was handed; callers enforce that rather
352+
* than assume it, so the seam cannot widen into "the verifier decides what
353+
* gets bound". See `useVerifyMentionIdentities` for the implementation and
354+
* `mentionIdentityTrust` for why the check exists.
341355
*/
342-
export function registerMentionClipboardIdentities({
356+
export type VerifyMentionIdentities = (
357+
records: readonly MentionIdentity[],
358+
) => Promise<readonly MentionIdentity[]>;
359+
360+
/** Case-insensitive identity of a `label → pubkey` pair. */
361+
function mentionIdentityKey(identity: MentionIdentity): string {
362+
return `${canonicalMentionLabel(identity.label)} ${identity.pubkey.trim().toLowerCase()}`;
363+
}
364+
365+
/**
366+
* The identities a paste is allowed to bind.
367+
*
368+
* Three conditions, all necessary. The clipboard has to *carry* the record;
369+
* the content the paste inserts has to *show* its label, so no binding
370+
* outlives a paste the user could not see; and trusted Buzz state has to
371+
* *vouch* for the pair, because a visible `@John Smith` beside an attacker's
372+
* key is visible either way.
373+
*
374+
* Binding is what makes a pasted multi-word name known to the mention
375+
* decorations *and* to the send-time extractor — so the chip re-lights and
376+
* the original pubkey survives the round trip. Everything dropped here stays
377+
* readable text that tags nobody.
378+
*/
379+
export async function selectBindableMentionIdentities({
343380
html,
344-
registerMentionPubkey,
345381
text,
382+
verifyMentionIdentities,
346383
}: {
347384
/** Clipboard HTML holding the identity records — untrusted. */
348385
html: string;
349-
registerMentionPubkey: (
350-
displayName: string,
351-
pubkey: string,
352-
options?: { isAgent?: boolean },
353-
) => void;
354386
/** The text the paste inserts; a record unmentioned there is discarded. */
355387
text: string;
356-
}): MentionIdentity[] {
357-
const records = selectVisibleMentionIdentities(
388+
verifyMentionIdentities: VerifyMentionIdentities;
389+
}): Promise<MentionIdentity[]> {
390+
const visible = selectVisibleMentionIdentities(
358391
parseMentionClipboardRecords(html),
359392
text,
360393
);
361-
for (const record of records) {
362-
registerMentionPubkey(record.label, record.pubkey, {
363-
isAgent: record.isAgent,
364-
});
365-
}
366-
return records;
394+
if (visible.length === 0) return [];
395+
const vouched = new Set(
396+
(await verifyMentionIdentities(visible)).map(mentionIdentityKey),
397+
);
398+
return visible.filter((record) => vouched.has(mentionIdentityKey(record)));
367399
}

0 commit comments

Comments
 (0)