From 35d3302c9e707c717c6617176fea133d79f7a3e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karla=20Valc=C3=A1rcel=20Mart=C3=ADnez?= <99458559+karla-vm@users.noreply.github.com> Date: Thu, 9 Jan 2025 13:20:24 -0500 Subject: [PATCH] CMDCT-4228: Address Pen Test Findings (#12000) Co-authored-by: Jon Holman <9025884+JonHolman@users.noreply.github.com> --- .../app-api/utils/sanitize/sanitize.test.ts | 45 +++---------------- services/app-api/utils/sanitize/sanitize.ts | 16 +++++++ .../ui-src/src/components/alerts/Alert.tsx | 2 +- 3 files changed, 24 insertions(+), 39 deletions(-) diff --git a/services/app-api/utils/sanitize/sanitize.test.ts b/services/app-api/utils/sanitize/sanitize.test.ts index 9130540e1..2a3cc9999 100644 --- a/services/app-api/utils/sanitize/sanitize.test.ts +++ b/services/app-api/utils/sanitize/sanitize.test.ts @@ -12,35 +12,13 @@ const safeUndefined = undefined; const cleanString = "test"; -const dirtyImgString = ''; -const cleanImgString = ''; - -const dirtyLinkString = ""; +const dirtyLinkString = ""; const cleanLinkString = ''; -const dirtyScriptString = - ""; -const cleanScriptString = ""; - -const dirtySvgString = ""; -const cleanSvgString = ""; - // ARRAYS -const dirtyStringArray = [ - cleanString, - dirtyImgString, - dirtyLinkString, - dirtySvgString, - dirtyScriptString, -]; -const cleanStringArray = [ - cleanString, - cleanImgString, - cleanLinkString, - cleanSvgString, - cleanScriptString, -]; +const dirtyStringArray = [cleanString, dirtyLinkString]; +const cleanStringArray = [cleanString, cleanLinkString]; const dirtyNestedStringArray = [dirtyStringArray, dirtyStringArray]; const cleanNestedStringArray = [cleanStringArray, cleanStringArray]; @@ -48,11 +26,11 @@ const cleanNestedStringArray = [cleanStringArray, cleanStringArray]; // OBJECTS const dirtyObject = { - string: dirtyImgString, + string: dirtyLinkString, array: dirtyStringArray, }; const cleanObject = { - string: cleanImgString, + string: cleanLinkString, array: cleanStringArray, }; @@ -61,10 +39,7 @@ const cleanObjectArray = [cleanObject, cleanObject]; const dirtyComplexObject = { string1: cleanString, - string2: dirtyImgString, - string3: dirtyLinkString, - string4: dirtySvgString, - string5: dirtyScriptString, + string2: dirtyLinkString, array: dirtyStringArray, nestedStringArray: dirtyNestedStringArray, nestedObjectArray: dirtyObjectArray, @@ -74,10 +49,7 @@ const dirtyComplexObject = { }; const cleanComplexObject = { string1: cleanString, - string2: cleanImgString, - string3: cleanLinkString, - string4: cleanSvgString, - string5: cleanScriptString, + string2: cleanLinkString, array: cleanStringArray, nestedStringArray: cleanNestedStringArray, nestedObjectArray: cleanObjectArray, @@ -93,10 +65,7 @@ describe("Test sanitizeString", () => { }); test("Test sanitizeString cleans dirty strings", () => { - expect(sanitizeString(dirtyImgString)).toEqual(cleanImgString); expect(sanitizeString(dirtyLinkString)).toEqual(cleanLinkString); - expect(sanitizeString(dirtySvgString)).toEqual(cleanSvgString); - expect(sanitizeString(dirtyScriptString)).toEqual(cleanScriptString); }); }); diff --git a/services/app-api/utils/sanitize/sanitize.ts b/services/app-api/utils/sanitize/sanitize.ts index f6cdb7a50..c75cd290a 100644 --- a/services/app-api/utils/sanitize/sanitize.ts +++ b/services/app-api/utils/sanitize/sanitize.ts @@ -4,6 +4,22 @@ import { JSDOM } from "jsdom"; const windowEmulator: any = new JSDOM("").window; const DOMPurify = createDOMPurify(windowEmulator); +/* + * DOMPurify prevents all XSS attacks by default. With these settings, it also + * prevents "deception" attacks. If an attacker could put
+ * into the site's admin banner, they could make give the banner any appearance, + * overlaid anywhere on the page. For example, a fake "session expired" modal + * with a malicious link. Thus, this very strict DOMPurify config. + */ +DOMPurify.setConfig({ + // Only these tags will be allowed through + ALLOWED_TAGS: ["ul", "ol", "li", "a", "#text"], + // On those tags, only these attributes are allowed + ALLOWED_ATTR: ["href", "alt"], + // If a tag is removed, so will all its child elements & text + KEEP_CONTENT: false, +}); + // sanitize string export const sanitizeString = (string: string) => { if (DOMPurify.isSupported) { diff --git a/services/ui-src/src/components/alerts/Alert.tsx b/services/ui-src/src/components/alerts/Alert.tsx index 5cdc3084c..b65fe4462 100644 --- a/services/ui-src/src/components/alerts/Alert.tsx +++ b/services/ui-src/src/components/alerts/Alert.tsx @@ -45,7 +45,7 @@ export const Alert = ({ : alertIcon } sx={sx.icon} - alt={status} + alt={status || "alert"} /> )}