From 052f8a43fb967b019cdec19a1307d23c6a15f76a Mon Sep 17 00:00:00 2001 From: Kristian Wind Date: Thu, 20 Aug 2026 06:39:19 +0200 Subject: [PATCH] fix(notes): en note kunne smugle et script ind gennem et billedes alt-tekst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Indført i aften, sammen med billedvisningen, og fundet ved at læse den igen. `escapeHtml` tager &, < og > — ikke anførselstegn, og det er rigtigt for tekst mellem to tags, hvor et anførselstegn ikke betyder noget. `alt` er ikke tekst mellem to tags. Det står inde i en attribut, og der lukker det første anførselstegn den, hvorefter resten står som markup: ![" onerror="alert(1)](/api/v1/attachments/aaaa) blev til et med en onerror på. Adressen var i forvejen bundet til vores eget bilagsmønster, så den var ikke vejen ind — teksten ved siden af var. Det er ikke en teoretisk vej. En note deles ved at ligge i et projekt, så den læses af alle, der kan læse projektet, og scriptet ville køre på vores eget domæne med sessionscookien, hos den der åbnede noten. En værdi i en attribut har brug for sin egen undslipning, og har nu en. Prøven skriver noten, åbner den og ser efter, at hændelsen står som tekst i alt og ikke som en attribut på elementet — og at billedets 404, som er selve vejen en onerror ville blive kaldt ad, ikke kalder noget. Co-Authored-By: Claude Opus 5 --- .claude/worktrees/agent-a7f869c634e650110 | 1 + web/e2e/smoke.spec.js | 38 +++++++++++++++++++++++ web/src/lib/richtext.js | 20 +++++++++++- 3 files changed, 58 insertions(+), 1 deletion(-) create mode 160000 .claude/worktrees/agent-a7f869c634e650110 diff --git a/.claude/worktrees/agent-a7f869c634e650110 b/.claude/worktrees/agent-a7f869c634e650110 new file mode 160000 index 0000000..dee2d5e --- /dev/null +++ b/.claude/worktrees/agent-a7f869c634e650110 @@ -0,0 +1 @@ +Subproject commit dee2d5e30b39bf90238724a347e08dbbd6be32f0 diff --git a/web/e2e/smoke.spec.js b/web/e2e/smoke.spec.js index a968022..56b566b 100644 --- a/web/e2e/smoke.spec.js +++ b/web/e2e/smoke.spec.js @@ -2396,3 +2396,41 @@ test('lister overlever en gemning — også i et citat, og også med numre', asy expect(trouble).toEqual([]); }); + +test('en note kan ikke smugle et script ind gennem et billedes alt-tekst', async ({ page }) => { + const trouble = watchForTrouble(page); + await page.goto('/noter'); + + // Undslap `alt` ikke sine anførselstegn, lukkede den første af dem attributten, + // og resten stod som markup: et img med en onerror på. En note deles gennem et + // projekt, så det ville køre på vores eget domæne, hos den der åbnede noten. + const id = await page.evaluate(async () => { + const r = await fetch('/api/v1/notes', { + method: 'POST', + credentials: 'include', + headers: { 'Content-Type': 'application/json', 'Sec-Fetch-Site': 'same-origin' }, + body: JSON.stringify({ + body: '# Ondsindet\n\n![" onerror="window.__pwned = true](/api/v1/attachments/aaaabbbbccccdddd)\n' + }) + }); + return (await r.json()).id; + }); + + await page.goto('/noter?note=' + id); + const ed = page.getByRole('textbox', { name: 'Notens tekst' }); + await expect(ed.locator('img')).toHaveCount(1); + // Billedet peger ingen steder, så det fejler at indlæse — hvilket er præcis + // den vej, en onerror ville blive kaldt ad. + await page.waitForTimeout(800); + + expect(await page.evaluate(() => window.__pwned)).toBeUndefined(); + // Og hændelsen står som tekst i alt-attributten, hvor den hører hjemme. + const alt = await ed.locator('img').getAttribute('alt'); + expect(alt).toContain('onerror'); + expect(await ed.locator('img').evaluate((el) => el.hasAttribute('onerror'))).toBe(false); + + // Billedets egen 404 hører til prøven: adressen peger med vilje på et bilag, + // der ikke findes, fordi det er den vej en onerror ville blive kaldt ad. Alt + // andet skal stadig være stille. + expect(trouble.filter((t) => !t.includes('/api/v1/attachments/'))).toEqual([]); +}); diff --git a/web/src/lib/richtext.js b/web/src/lib/richtext.js index 6ecd648..a301d41 100644 --- a/web/src/lib/richtext.js +++ b/web/src/lib/richtext.js @@ -37,6 +37,16 @@ const BLOCKS = [ const escapeHtml = (s) => s.replace(/&/g, '&').replace(//g, '>'); +/** + * En værdi, der skal stå inde i en attribut. + * + * escapeHtml er skrevet til tekst mellem to tags, hvor et anførselstegn ikke + * betyder noget, og lader dem derfor stå. Inde i en attribut lukker de den — og + * så står resten af strengen som markup. Det er den ene forskel, og den er hele + * forskellen mellem tekst og et onerror. + */ +const attr = (s) => escapeHtml(s).replace(/"/g, '"').replace(/'/g, '''); + /** Inline marks, innermost first so a bold word inside italics survives both. */ function inlineToHtml(text) { return ( @@ -51,8 +61,16 @@ function inlineToHtml(text) { // ellers hente fra en fremmed vært, når noten åbnes, og fortælle den, at // den er blevet læst — og en note er det sidste sted, man vil have en // sporingspixel. Alt andet står som den tekst, det er. + // + // `alt` skal citat-undslippes her, og det er ikke en formalitet. + // escapeHtml tager &, < og > — ikke anførselstegn, fordi den er skrevet + // til tekst mellem to tags, hvor et anførselstegn ikke betyder noget. Her + // står værdien inde i en attribut, og der lukker et anførselstegn den: + // `![" onerror="alert(1)](/api/v1/attachments/aaaa)` blev til et img med + // en onerror på. En note er delt gennem et projekt, så det ville køre på + // vores eget domæne med sessionscookien, hos den, der åbnede noten. .replace(/!\[([^\]]*)\]\((\/api\/v1\/attachments\/[0-9a-f-]+)\)/g, - (_, alt, src) => `${alt}`) + (_, alt, src) => `${attr(alt)}`) .replace(/<u>(.+?)<\/u>/g, '$1') .replace(/`([^`]+)`/g, '$1') .replace(/\*\*(.+?)\*\*/g, '$1')