Skip to content

Commit

Permalink
refactor: [M3-7990] - Replace sanitize-html with DOM friendly alterna…
Browse files Browse the repository at this point in the history
…tive (linode#10378)

* Save work

* Handle all instances and types

* fix tests

* build fixes

* Trusted types policy

* Added changeset: Replace sanitize-html with dompurify
  • Loading branch information
abailly-akamai authored Apr 22, 2024
1 parent a68e2f2 commit 5672faf
Show file tree
Hide file tree
Showing 10 changed files with 116 additions and 145 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tech Stories
---

Replace sanitize-html with dompurify ([#10378](https://github.com/linode/manager/pull/10378))
4 changes: 2 additions & 2 deletions packages/manager/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"chart.js": "~2.9.4",
"copy-to-clipboard": "^3.0.8",
"country-region-data": "^1.4.5",
"dompurify": "^3.1.0",
"flag-icons": "^6.6.5",
"font-logos": "^0.18.0",
"formik": "~2.1.3",
Expand Down Expand Up @@ -71,7 +72,6 @@
"redux": "^4.0.4",
"redux-thunk": "^2.3.0",
"reselect": "^4.0.0",
"sanitize-html": "^2.12.1",
"search-string": "^3.1.0",
"throttle-debounce": "^2.0.0",
"tss-react": "^4.8.2",
Expand Down Expand Up @@ -133,6 +133,7 @@
"@types/chai-string": "^1.4.5",
"@types/chart.js": "^2.9.21",
"@types/css-mediaquery": "^0.1.1",
"@types/dompurify": "^3.0.5",
"@types/he": "^1.1.0",
"@types/highlight.js": "~10.1.0",
"@types/jsdom": "^21.1.4",
Expand All @@ -154,7 +155,6 @@
"@types/react-select": "^3.0.11",
"@types/recompose": "^0.30.0",
"@types/redux-mock-store": "^1.0.1",
"@types/sanitize-html": "^2.9.1",
"@types/throttle-debounce": "^1.0.0",
"@types/uuid": "^3.4.3",
"@types/yup": "^0.29.13",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { unsafe_MarkdownIt } from 'src/utilities/markdown';
import { sanitizeHTML } from 'src/utilities/sanitizeHTML';
import { useColorMode } from 'src/utilities/theme';

import type { IOptions } from 'sanitize-html';
import type { SanitizeOptions } from 'src/utilities/sanitizeHTML';

hljs.registerLanguage('apache', apache);
hljs.registerLanguage('bash', bash);
Expand All @@ -35,7 +35,7 @@ export type SupportedLanguage =
export interface HighlightedMarkdownProps {
className?: string;
language?: SupportedLanguage;
sanitizeOptions?: IOptions;
sanitizeOptions?: SanitizeOptions;
textOrMarkdown: string;
}

Expand Down Expand Up @@ -88,8 +88,7 @@ export const HighlightedMarkdown = (props: HighlightedMarkdownProps) => {
const unsafe_parsedMarkdown = unsafe_MarkdownIt.render(textOrMarkdown);

const sanitizedHtml = sanitizeHTML({
// eslint-disable-next-line xss/no-mixed-html
options: sanitizeOptions,
sanitizeOptions,
sanitizingTier: 'flexible',
text: unsafe_parsedMarkdown,
});
Expand Down
9 changes: 8 additions & 1 deletion packages/manager/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,14 @@ export const allowedHTMLTagsFlexible: string[] = [
'tr',
];

export const allowedHTMLAttr = ['href', 'lang', 'title', 'align'];
export const allowedHTMLAttr = [
'href',
'lang',
'title',
'align',
'class',
'rel',
];

/**
* MBps rate for intra DC migrations (AKA Mutations)
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/src/features/Events/EventRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export const Row = (props: RowProps) => {
<TableCell data-qa-event-message-cell parentColumn="Event">
<HighlightedMarkdown
sanitizeOptions={{
allowedTags: ['a'],
ALLOWED_TAGS: ['a'],
disallowedTagsMode: 'discard',
}}
textOrMarkdown={message}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { useTheme } from '@mui/material';
import { useSnackbar } from 'notistack';
import * as React from 'react';
import { useParams } from 'react-router-dom';
import sanitize from 'sanitize-html';

import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
import { Drawer } from 'src/components/Drawer';
Expand All @@ -19,6 +18,7 @@ import {
import { useGrants, useProfile } from 'src/queries/profile';
import { getAPIErrorOrDefault } from 'src/utilities/errorUtils';
import { getEntityIdsByPermission } from 'src/utilities/grants';
import { sanitizeHTML } from 'src/utilities/sanitizeHTML';

interface Props {
helperText: string;
Expand Down Expand Up @@ -94,10 +94,14 @@ export const AddLinodeDrawer = (props: Props) => {
};

const errorNotice = () => {
let errorMsg = sanitize(localError || '', {
allowedAttributes: {},
allowedTags: [], // Disallow all HTML tags,
});
let errorMsg = sanitizeHTML({
sanitizeOptions: {
ALLOWED_ATTR: [],
ALLOWED_TAGS: [], // Disallow all HTML tags,
},
sanitizingTier: 'strict',
text: localError || '',
}).toString();
// match something like: Linode <linode_label> (ID <linode_id>)

const linode = /Linode (.+?) \(ID ([^\)]+)\)/i.exec(errorMsg);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { NodeBalancer } from '@linode/api-v4';
import { useTheme } from '@mui/material';
import { useQueryClient } from '@tanstack/react-query';
import { useSnackbar } from 'notistack';
import * as React from 'react';
import { useQueryClient } from '@tanstack/react-query';
import { useParams } from 'react-router-dom';
import sanitize from 'sanitize-html';

import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
import { Drawer } from 'src/components/Drawer';
Expand All @@ -22,6 +21,7 @@ import { queryKey } from 'src/queries/nodebalancers';
import { useGrants, useProfile } from 'src/queries/profile';
import { getAPIErrorOrDefault } from 'src/utilities/errorUtils';
import { getEntityIdsByPermission } from 'src/utilities/grants';
import { sanitizeHTML } from 'src/utilities/sanitizeHTML';

interface Props {
helperText: string;
Expand Down Expand Up @@ -103,10 +103,14 @@ export const AddNodebalancerDrawer = (props: Props) => {
};

const errorNotice = () => {
let errorMsg = sanitize(localError || '', {
allowedAttributes: {},
allowedTags: [], // Disallow all HTML tags,
});
let errorMsg = sanitizeHTML({
sanitizeOptions: {
ALLOWED_ATTR: [],
ALLOWED_TAGS: [], // Disallow all HTML tags,
},
sanitizingTier: 'strict',
text: localError || '',
}).toString();
// match something like: NodeBalancer <nodebalancer_label> (ID <nodebalancer_id>)

const nodebalancer = /NodeBalancer (.+?) \(ID ([^\)]+)\)/i.exec(errorMsg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const RenderEvent = React.memo((props: RenderEventProps) => {
<div className={unseenEventClass}>
<HighlightedMarkdown
sanitizeOptions={{
allowedTags: getAllowedHTMLTags('strict'),
ALLOWED_TAGS: getAllowedHTMLTags('strict'),
disallowedTagsMode: 'discard',
}}
textOrMarkdown={message}
Expand Down
114 changes: 59 additions & 55 deletions packages/manager/src/utilities/sanitizeHTML.ts
Original file line number Diff line number Diff line change
@@ -1,77 +1,81 @@
import sanitize from 'sanitize-html';
import DOMPurify from 'dompurify';

import { allowedHTMLAttr } from 'src/constants';

import { getAllowedHTMLTags, isURLValid } from './sanitizeHTML.utils';

import type { AllowedHTMLTagsTier } from './sanitizeHTML.utils';
import type { IOptions } from 'sanitize-html';
import type { Config } from 'dompurify';

type DisallowedTagsMode = 'discard' | 'escape';

export interface SanitizeOptions extends Config {
disallowedTagsMode?: DisallowedTagsMode;
}

interface SanitizeHTMLOptions {
allowMoreTags?: string[];
disallowedTagsMode?: IOptions['disallowedTagsMode'];
options?: IOptions;
disallowedTagsMode?: DisallowedTagsMode;
sanitizeOptions?: Config;
sanitizingTier: AllowedHTMLTagsTier;
text: string;
}

export const sanitizeHTML = ({
allowMoreTags,
disallowedTagsMode = 'escape',
options = {},
sanitizeOptions: options = {},
sanitizingTier,
text,
}: SanitizeHTMLOptions) =>
sanitize(text, {
allowedAttributes: {
'*': allowedHTMLAttr,
// "target" and "rel" are allowed because they are handled in the
// transformTags map below.
a: [...allowedHTMLAttr, 'target', 'rel'],
},
allowedClasses: {
span: ['version'],
},
allowedTags: getAllowedHTMLTags(sanitizingTier, allowMoreTags),
disallowedTagsMode,
transformTags: {
// This transformation function does the following to anchor tags:
// 1. Turns the <a /> into a <span /> if the "href" is invalid
// 2. Adds `rel="noopener noreferrer" if _target is "blank" (for security)
// 3. Removes "target" attribute if it's anything other than "_blank"
// 4. Removes custom "rel" attributes
}: SanitizeHTMLOptions) => {
DOMPurify.setConfig({
ALLOWED_ATTR: allowedHTMLAttr,
ALLOWED_TAGS: getAllowedHTMLTags(sanitizingTier, allowMoreTags),
KEEP_CONTENT: disallowedTagsMode === 'discard' ? false : true,
RETURN_DOM: false,
RETURN_DOM_FRAGMENT: false,
RETURN_TRUSTED_TYPE: false,
...options,
});

a: (tagName, attribs) => {
// If the URL is invalid, transform to a span.
const href = attribs.href ?? '';
if (href && !isURLValid(href)) {
return {
attribs: {},
tagName: 'span',
};
}
// Define transform function for anchor tags
const anchorHandler = (node: HTMLAnchorElement) => {
const href = node.getAttribute('href') ?? '';

// If this link opens a new tab, add "noopener noreferrer" for security.
const target = attribs.target ?? '';
if (target && target === '_blank') {
return {
attribs: {
...attribs,
rel: 'noopener noreferrer',
},
tagName,
};
}
// If the URL is invalid, transform to a span.
if (href && !isURLValid(href)) {
const span = document.createElement('span');
span.setAttribute('class', node.getAttribute('class') || '');

// Otherwise we don't want to allow the "rel" or "target" attributes.
delete attribs.rel;
delete attribs.target;
if (node.parentNode) {
node.parentNode.replaceChild(span, node);
}
} else {
// If this link opens a new tab, add "noopener noreferrer" for security.
const target = node.getAttribute('target') || '';
if (target === '_blank') {
node.setAttribute('rel', 'noopener noreferrer');
} else {
node.removeAttribute('rel');
node.removeAttribute('target');
}
}
};

return {
attribs,
tagName,
};
},
},
...options,
}).trim();
// Register hooks for DOMPurify
DOMPurify.addHook('uponSanitizeElement', (node, data) => {
if (data.tagName === 'a') {
anchorHandler(node as HTMLAnchorElement);
} else if (data.tagName === 'span') {
// Allow class attribute only for span elements
const classAttr = node.getAttribute('class');
if (classAttr && classAttr.trim() !== 'version') {
node.removeAttribute('class');
}
}
});

// Perform sanitization
const output = DOMPurify.sanitize(text);
return output.trim();
};
Loading

0 comments on commit 5672faf

Please sign in to comment.