refactor(api): 📦 Change sanitizer from DOMPurify to xss

This commit is contained in:
Jesse Wierzbinski 2024-05-02 17:20:24 -10:00
parent a430db5c30
commit 154f17ab12
No known key found for this signature in database
7 changed files with 99 additions and 52 deletions

BIN
bun.lockb

Binary file not shown.

View file

@ -555,14 +555,14 @@ export const contentToHtml = async (
let htmlContent: string; let htmlContent: string;
if (content["text/html"]) { if (content["text/html"]) {
htmlContent = content["text/html"].content; htmlContent = await sanitizeHtml(content["text/html"].content);
} else if (content["text/markdown"]) { } else if (content["text/markdown"]) {
htmlContent = await sanitizeHtml( htmlContent = await sanitizeHtml(
await markdownParse(content["text/markdown"].content), await markdownParse(content["text/markdown"].content),
); );
} else if (content["text/plain"]?.content) { } else if (content["text/plain"]?.content) {
// Split by newline and add <p> tags // Split by newline and add <p> tags
htmlContent = content["text/plain"].content htmlContent = (await sanitizeHtml(content["text/plain"].content))
.split("\n") .split("\n")
.map((line) => `<p>${line}</p>`) .map((line) => `<p>${line}</p>`)
.join("\n"); .join("\n");

View file

@ -77,10 +77,8 @@
"cli-parser": "workspace:*", "cli-parser": "workspace:*",
"cli-table": "^0.3.11", "cli-table": "^0.3.11",
"config-manager": "workspace:*", "config-manager": "workspace:*",
"dompurify": "^3.1.2",
"drizzle-orm": "^0.30.7", "drizzle-orm": "^0.30.7",
"extract-zip": "^2.0.1", "extract-zip": "^2.0.1",
"happy-dom": "14.7.1",
"html-to-text": "^9.0.5", "html-to-text": "^9.0.5",
"ioredis": "^5.3.2", "ioredis": "^5.3.2",
"ip-matching": "^2.1.2", "ip-matching": "^2.1.2",
@ -103,6 +101,8 @@
"request-parser": "workspace:*", "request-parser": "workspace:*",
"sharp": "^0.33.3", "sharp": "^0.33.3",
"string-comparison": "^1.3.0", "string-comparison": "^1.3.0",
"stringify-entities": "^4.0.4",
"xss": "^1.0.15",
"zod": "^3.22.4", "zod": "^3.22.4",
"zod-validation-error": "^3.2.0" "zod-validation-error": "^3.2.0"
} }

View file

@ -45,6 +45,7 @@ import { config } from "~packages/config-manager";
import type { Attachment as APIAttachment } from "~types/mastodon/attachment"; import type { Attachment as APIAttachment } from "~types/mastodon/attachment";
import type { Status as APIStatus } from "~types/mastodon/status"; import type { Status as APIStatus } from "~types/mastodon/status";
import { User } from "./user"; import { User } from "./user";
import { sanitizedHtmlStrip } from "@sanitization";
/** /**
* Gives helpers to fetch notes from database in a nice format * Gives helpers to fetch notes from database in a nice format
@ -208,7 +209,7 @@ export class Note {
contentType: "text/html", contentType: "text/html",
visibility, visibility,
sensitive: is_sensitive, sensitive: is_sensitive,
spoilerText: spoiler_text, spoilerText: await sanitizedHtmlStrip(spoiler_text),
uri: uri || null, uri: uri || null,
replyId: replyId ?? null, replyId: replyId ?? null,
quotingId: quoteId ?? null, quotingId: quoteId ?? null,

View file

@ -1,6 +1,6 @@
import { apiRoute, applyConfig } from "@api"; import { apiRoute, applyConfig } from "@api";
import { errorResponse, jsonResponse } from "@response"; import { errorResponse, jsonResponse } from "@response";
import { sanitizeHtml } from "@sanitization"; import { sanitizeHtml, sanitizedHtmlStrip } from "@sanitization";
import { config } from "config-manager"; import { config } from "config-manager";
import { and, eq } from "drizzle-orm"; import { and, eq } from "drizzle-orm";
import ISO6391 from "iso-639-1"; import ISO6391 from "iso-639-1";
@ -97,10 +97,9 @@ export default apiRoute<typeof meta, typeof schema>(
const sanitizedNote = await sanitizeHtml(note ?? ""); const sanitizedNote = await sanitizeHtml(note ?? "");
const sanitizedDisplayName = await sanitizeHtml(display_name ?? "", { const sanitizedDisplayName = await sanitizedHtmlStrip(
ALLOWED_TAGS: [], display_name ?? "",
ALLOWED_ATTR: [], );
});
let mediaManager: MediaBackend; let mediaManager: MediaBackend;

View file

@ -361,4 +361,62 @@ describe(meta.route, () => {
}); });
}); });
}); });
describe("HTML injection testing", () => {
test("should not allow HTML injection", async () => {
const response = await sendTestRequest(
new Request(new URL(meta.route, config.http.base_url), {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bearer ${tokens[0].accessToken}`,
},
body: JSON.stringify({
status: "Hi! <script>alert('Hello, world!');</script>",
federate: false,
}),
}),
);
expect(response.status).toBe(200);
expect(response.headers.get("content-type")).toBe(
"application/json",
);
const object = (await response.json()) as APIStatus;
expect(object.content).toBe(
"<p>Hi! &lt;script&gt;alert('Hello, world!');&lt;/script&gt;</p>",
);
});
test("should not allow HTML injection in spoiler_text", async () => {
const response = await sendTestRequest(
new Request(new URL(meta.route, config.http.base_url), {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bearer ${tokens[0].accessToken}`,
},
body: JSON.stringify({
status: "Hello, world!",
spoiler_text:
"uwu <script>alert('Hello, world!');</script>",
federate: false,
}),
}),
);
expect(response.status).toBe(200);
expect(response.headers.get("content-type")).toBe(
"application/json",
);
const object = (await response.json()) as APIStatus;
expect(object.spoiler_text).toBe(
"uwu &#x3C;script&#x3E;alert(&#x27;Hello, world!&#x27;);&#x3C;/script&#x3E;",
);
});
});
}); });

View file

@ -1,52 +1,41 @@
import { config } from "config-manager"; import xss, { type IFilterXSSOptions } from "xss";
import DOMPurify from "dompurify"; import { stringifyEntitiesLight } from "stringify-entities";
import { GlobalWindow } from "happy-dom";
const window = new GlobalWindow(); export const sanitizedHtmlStrip = (html: string) => {
return sanitizeHtml(html, {
whiteList: {},
});
};
export const sanitizeHtml = async ( export const sanitizeHtml = async (
html: string, html: string,
extraConfig?: DOMPurify.Config, extraConfig?: IFilterXSSOptions,
) => { ) => {
// @ts-expect-error Types clash but it works i swear const sanitizedHtml = xss(html, {
const sanitizedHtml = DOMPurify(window).sanitize(html, { whiteList: {
ALLOWED_TAGS: [ a: ["href", "title", "target", "rel", "class"],
"a", p: ["class"],
"p", br: ["class"],
"br", b: ["class"],
"b", i: ["class"],
"i", em: ["class"],
"em", strong: ["class"],
"strong", del: ["class"],
"del", code: ["class"],
"code", u: ["class"],
"u", pre: ["class"],
"pre", ul: ["class"],
"ul", ol: ["class"],
"ol", li: ["class"],
"li", blockquote: ["class"],
"blockquote",
],
ALLOWED_ATTR: [
"href",
"target",
"title",
"rel",
"class",
"start",
"reversed",
"value",
],
ALLOWED_URI_REGEXP: new RegExp(
`/^(?:(?:${config.validation.url_scheme_whitelist.join(
"|",
)}):|[^a-z]|[a-z+.-]+(?:[^a-z+.-:]|$))/i`,
),
USE_PROFILES: {
mathMl: true,
}, },
stripIgnoreTag: false,
escapeHtml: (unsafeHtml) =>
stringifyEntitiesLight(unsafeHtml, {
escapeOnly: true,
}),
...extraConfig, ...extraConfig,
}) as string; });
// Check text to only allow h-*, p-*, u-*, dt-*, e-*, mention, hashtag, ellipsis, invisible classes // Check text to only allow h-*, p-*, u-*, dt-*, e-*, mention, hashtag, ellipsis, invisible classes
const allowedClasses = [ const allowedClasses = [