Skip to content

Commit 474d495

Browse files
authored
Switch to dompurify for sanitizing markdown content (microsoft#131950)
* Switch to dompurify for sanitizing markdown content Switches us from using `insane` to instead use `dompurify`, which seems to be better maintained and also has some nice features, such as built-in trusted types support I've tried to port over our existing sanitizer settings as best as possible, but there's not always a 1:1 mapping between how insane works and how dompurify does. I'd like to get this change in early in the iteration to catch potential regressions * Remove logging and renaming param * Move dompurify to browser layer * Fixing tests and how we check valid attributes * Allow innerhtml in specific files * Use isEqualNode instead of checking innerHTML directly innerHTML can return different results on different browsers. Use `isEqualNode` instead * Reapply fix for trusted types * Enable ALLOW_UNKNOWN_PROTOCOLS I beleive this is required since we allow links to commands and loading images over remote * in -> of * Fix check of protocol * Enable two more safe tags
1 parent 82a3d26 commit 474d495

File tree

20 files changed

+2040
-667
lines changed

20 files changed

+2040
-667
lines changed

.eslintignore

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
**/vs/css.build.js
44
**/vs/css.js
55
**/vs/loader.js
6-
**/insane/**
6+
**/dompurify/**
77
**/marked/**
88
**/semver/**
99
**/test/**/*.js

build/filters.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ module.exports.indentationFilter = [
3737
'!src/vs/css.js',
3838
'!src/vs/css.build.js',
3939
'!src/vs/loader.js',
40-
'!src/vs/base/common/insane/insane.js',
40+
'!src/vs/base/browser/dompurify/*',
4141
'!src/vs/base/common/marked/marked.js',
4242
'!src/vs/base/common/semver/semver.js',
4343
'!src/vs/base/node/terminateProcess.sh',
@@ -134,7 +134,7 @@ module.exports.jsHygieneFilter = [
134134
'!src/vs/nls.js',
135135
'!src/vs/css.build.js',
136136
'!src/vs/nls.build.js',
137-
'!src/**/insane.js',
137+
'!src/**/dompurify.js',
138138
'!src/**/marked.js',
139139
'!src/**/semver.js',
140140
'!**/test/**',

src/tsec.exemptions.json

+3
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,8 @@
2929
"ban-worker-calls": [
3030
"vs/base/worker/defaultWorkerFactory.ts",
3131
"vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts"
32+
],
33+
"ban-domparser-parsefromstring": [
34+
"vs/base/test/browser/markdownRenderer.test.ts"
3235
]
3336
}

src/vs/base/browser/dom.ts

+31-42
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ import { IMouseEvent, StandardMouseEvent } from 'vs/base/browser/mouseEvent';
1010
import { TimeoutTimer } from 'vs/base/common/async';
1111
import { onUnexpectedError } from 'vs/base/common/errors';
1212
import { Emitter, Event } from 'vs/base/common/event';
13-
import { insane, InsaneOptions } from 'vs/base/common/insane/insane';
13+
import * as dompurify from 'vs/base/browser/dompurify/dompurify';
1414
import { KeyCode } from 'vs/base/common/keyCodes';
1515
import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
16-
import { FileAccess, RemoteAuthorities } from 'vs/base/common/network';
16+
import { FileAccess, RemoteAuthorities, Schemas } from 'vs/base/common/network';
1717
import * as platform from 'vs/base/common/platform';
1818
import { withNullAsUndefined } from 'vs/base/common/types';
1919
import { URI } from 'vs/base/common/uri';
@@ -1361,52 +1361,41 @@ export function detectFullscreen(): IDetectedFullscreen | null {
13611361

13621362
// -- sanitize and trusted html
13631363

1364-
function _extInsaneOptions(opts: InsaneOptions, allowedAttributesForAll: string[]): InsaneOptions {
1364+
/**
1365+
* Sanitizes the given `value` and reset the given `node` with it.
1366+
*/
1367+
export function safeInnerHtml(node: HTMLElement, value: string): void {
1368+
const options: dompurify.Config = {
1369+
ALLOWED_TAGS: ['a', 'button', 'blockquote', 'code', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'hr', 'input', 'label', 'li', 'p', 'pre', 'select', 'small', 'span', 'strong', 'textarea', 'ul', 'ol'],
1370+
ALLOWED_ATTR: ['href', 'data-href', 'data-command', 'target', 'title', 'name', 'src', 'alt', 'class', 'id', 'role', 'tabindex', 'style', 'data-code', 'width', 'height', 'align', 'x-dispatch', 'required', 'checked', 'placeholder'],
1371+
RETURN_DOM: false,
1372+
RETURN_DOM_FRAGMENT: false,
1373+
};
13651374

1366-
let allowedAttributes: Record<string, string[]> = opts.allowedAttributes ?? {};
1375+
const allowedProtocols = [Schemas.http, Schemas.https, Schemas.command];
13671376

1368-
if (opts.allowedTags) {
1369-
for (let tag of opts.allowedTags) {
1370-
let array = allowedAttributes[tag];
1371-
if (!array) {
1372-
array = allowedAttributesForAll;
1373-
} else {
1374-
array = array.concat(allowedAttributesForAll);
1377+
// https://github.com/cure53/DOMPurify/blob/main/demos/hooks-scheme-allowlist.html
1378+
dompurify.addHook('afterSanitizeAttributes', (node) => {
1379+
// build an anchor to map URLs to
1380+
const anchor = document.createElement('a');
1381+
1382+
// check all href/src attributes for validity
1383+
for (const attr in ['href', 'src']) {
1384+
if (node.hasAttribute(attr)) {
1385+
anchor.href = node.getAttribute(attr) as string;
1386+
if (!allowedProtocols.includes(anchor.protocol)) {
1387+
node.removeAttribute(attr);
1388+
}
13751389
}
1376-
allowedAttributes[tag] = array;
13771390
}
1378-
}
1379-
1380-
return { ...opts, allowedAttributes };
1381-
}
1391+
});
13821392

1383-
const _ttpSafeInnerHtml = window.trustedTypes?.createPolicy('safeInnerHtml', {
1384-
createHTML(value, options: InsaneOptions) {
1385-
return insane(value, options);
1393+
try {
1394+
const html = dompurify.sanitize(value, { ...options, RETURN_TRUSTED_TYPE: true });
1395+
node.innerHTML = html as unknown as string;
1396+
} finally {
1397+
dompurify.removeHook('afterSanitizeAttributes');
13861398
}
1387-
});
1388-
1389-
/**
1390-
* Sanitizes the given `value` and reset the given `node` with it.
1391-
*/
1392-
export function safeInnerHtml(node: HTMLElement, value: string): void {
1393-
1394-
const options = _extInsaneOptions({
1395-
allowedTags: ['a', 'button', 'blockquote', 'code', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'hr', 'input', 'label', 'li', 'p', 'pre', 'select', 'small', 'span', 'strong', 'textarea', 'ul', 'ol'],
1396-
allowedAttributes: {
1397-
'a': ['href', 'x-dispatch'],
1398-
'button': ['data-href', 'x-dispatch'],
1399-
'input': ['type', 'placeholder', 'checked', 'required'],
1400-
'label': ['for'],
1401-
'select': ['required'],
1402-
'span': ['data-command', 'role'],
1403-
'textarea': ['name', 'placeholder', 'required'],
1404-
},
1405-
allowedSchemes: ['http', 'https', 'command']
1406-
}, ['class', 'id', 'role', 'tabindex']);
1407-
1408-
const html = _ttpSafeInnerHtml?.createHTML(value, options) ?? insane(value, options);
1409-
node.innerHTML = html as string;
14101399
}
14111400

14121401
/**
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"registrations": [
3+
{
4+
"component": {
5+
"type": "git",
6+
"git": {
7+
"name": "dompurify",
8+
"repositoryUrl": "https://github.com/cure53/DOMPurify",
9+
"commitHash": "6cfcdf56269b892550af80baa7c1fa5b680e5db7"
10+
}
11+
},
12+
"license": "Apache 2.0",
13+
"version": "2.3.1"
14+
}
15+
],
16+
"version": 1
17+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
// Type definitions for DOM Purify 2.2
2+
// Project: https://github.com/cure53/DOMPurify
3+
// Definitions by: Dave Taylor https://github.com/davetayls
4+
// Samira Bazuzi <https://github.com/bazuzi>
5+
// FlowCrypt <https://github.com/FlowCrypt>
6+
// Exigerr <https://github.com/Exigerr>
7+
// Piotr Błażejewicz <https://github.com/peterblazejewicz>
8+
// Nicholas Ellul <https://github.com/NicholasEllul>
9+
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
10+
11+
export as namespace DOMPurify;
12+
export = DOMPurify;
13+
14+
declare const DOMPurify: createDOMPurifyI;
15+
16+
interface createDOMPurifyI extends DOMPurify.DOMPurifyI {
17+
(window?: Window): DOMPurify.DOMPurifyI;
18+
}
19+
20+
declare namespace DOMPurify {
21+
interface DOMPurifyI {
22+
sanitize(source: string | Node): string;
23+
sanitize(source: string | Node, config: Config & { RETURN_TRUSTED_TYPE: true }): TrustedHTML;
24+
sanitize(source: string | Node, config: Config & { RETURN_DOM_FRAGMENT?: false | undefined; RETURN_DOM?: false | undefined }): string;
25+
sanitize(source: string | Node, config: Config & { RETURN_DOM_FRAGMENT: true }): DocumentFragment;
26+
sanitize(source: string | Node, config: Config & { RETURN_DOM: true }): HTMLElement;
27+
sanitize(source: string | Node, config: Config): string | HTMLElement | DocumentFragment;
28+
29+
addHook(hook: 'uponSanitizeElement', cb: (currentNode: Element, data: SanitizeElementHookEvent, config: Config) => void): void;
30+
addHook(hook: 'uponSanitizeAttribute', cb: (currentNode: Element, data: SanitizeAttributeHookEvent, config: Config) => void): void;
31+
addHook(hook: HookName, cb: (currentNode: Element, data: HookEvent, config: Config) => void): void;
32+
33+
setConfig(cfg: Config): void;
34+
clearConfig(): void;
35+
isValidAttribute(tag: string, attr: string, value: string): boolean;
36+
37+
removeHook(entryPoint: HookName): void;
38+
removeHooks(entryPoint: HookName): void;
39+
removeAllHooks(): void;
40+
41+
version: string;
42+
removed: any[];
43+
isSupported: boolean;
44+
}
45+
46+
interface Config {
47+
ADD_ATTR?: string[] | undefined;
48+
ADD_DATA_URI_TAGS?: string[] | undefined;
49+
ADD_TAGS?: string[] | undefined;
50+
ALLOW_DATA_ATTR?: boolean | undefined;
51+
ALLOWED_ATTR?: string[] | undefined;
52+
ALLOWED_TAGS?: string[] | undefined;
53+
FORBID_ATTR?: string[] | undefined;
54+
FORBID_TAGS?: string[] | undefined;
55+
FORCE_BODY?: boolean | undefined;
56+
KEEP_CONTENT?: boolean | undefined;
57+
/**
58+
* change the default namespace from HTML to something different
59+
*/
60+
NAMESPACE?: string | undefined;
61+
RETURN_DOM?: boolean | undefined;
62+
RETURN_DOM_FRAGMENT?: boolean | undefined;
63+
/**
64+
* This defaults to `true` starting DOMPurify 2.2.0. Note that setting it to `false`
65+
* might cause XSS from attacks hidden in closed shadowroots in case the browser
66+
* supports Declarative Shadow: DOM https://web.dev/declarative-shadow-dom/
67+
*/
68+
RETURN_DOM_IMPORT?: boolean | undefined;
69+
RETURN_TRUSTED_TYPE?: boolean | undefined;
70+
SANITIZE_DOM?: boolean | undefined;
71+
WHOLE_DOCUMENT?: boolean | undefined;
72+
ALLOWED_URI_REGEXP?: RegExp | undefined;
73+
SAFE_FOR_TEMPLATES?: boolean | undefined;
74+
ALLOW_UNKNOWN_PROTOCOLS?: boolean | undefined;
75+
USE_PROFILES?: false | { mathMl?: boolean | undefined; svg?: boolean | undefined; svgFilters?: boolean | undefined; html?: boolean | undefined } | undefined;
76+
IN_PLACE?: boolean | undefined;
77+
}
78+
79+
type HookName =
80+
| 'beforeSanitizeElements'
81+
| 'uponSanitizeElement'
82+
| 'afterSanitizeElements'
83+
| 'beforeSanitizeAttributes'
84+
| 'uponSanitizeAttribute'
85+
| 'afterSanitizeAttributes'
86+
| 'beforeSanitizeShadowDOM'
87+
| 'uponSanitizeShadowNode'
88+
| 'afterSanitizeShadowDOM';
89+
90+
type HookEvent = SanitizeElementHookEvent | SanitizeAttributeHookEvent | null;
91+
92+
interface SanitizeElementHookEvent {
93+
tagName: string;
94+
allowedTags: { [key: string]: boolean };
95+
}
96+
97+
interface SanitizeAttributeHookEvent {
98+
attrName: string;
99+
attrValue: string;
100+
keepAttr: boolean;
101+
allowedAttributes: { [key: string]: boolean };
102+
forceKeepAttr?: boolean | undefined;
103+
}
104+
}

0 commit comments

Comments
 (0)