Skip to content

Commit

Permalink
Switch to dompurify for sanitizing markdown content (microsoft#131950)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mjbvz authored Sep 3, 2021
1 parent 82a3d26 commit 474d495
Show file tree
Hide file tree
Showing 20 changed files with 2,040 additions and 667 deletions.
2 changes: 1 addition & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
**/vs/css.build.js
**/vs/css.js
**/vs/loader.js
**/insane/**
**/dompurify/**
**/marked/**
**/semver/**
**/test/**/*.js
Expand Down
4 changes: 2 additions & 2 deletions build/filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ module.exports.indentationFilter = [
'!src/vs/css.js',
'!src/vs/css.build.js',
'!src/vs/loader.js',
'!src/vs/base/common/insane/insane.js',
'!src/vs/base/browser/dompurify/*',
'!src/vs/base/common/marked/marked.js',
'!src/vs/base/common/semver/semver.js',
'!src/vs/base/node/terminateProcess.sh',
Expand Down Expand Up @@ -134,7 +134,7 @@ module.exports.jsHygieneFilter = [
'!src/vs/nls.js',
'!src/vs/css.build.js',
'!src/vs/nls.build.js',
'!src/**/insane.js',
'!src/**/dompurify.js',
'!src/**/marked.js',
'!src/**/semver.js',
'!**/test/**',
Expand Down
3 changes: 3 additions & 0 deletions src/tsec.exemptions.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,8 @@
"ban-worker-calls": [
"vs/base/worker/defaultWorkerFactory.ts",
"vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts"
],
"ban-domparser-parsefromstring": [
"vs/base/test/browser/markdownRenderer.test.ts"
]
}
73 changes: 31 additions & 42 deletions src/vs/base/browser/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { IMouseEvent, StandardMouseEvent } from 'vs/base/browser/mouseEvent';
import { TimeoutTimer } from 'vs/base/common/async';
import { onUnexpectedError } from 'vs/base/common/errors';
import { Emitter, Event } from 'vs/base/common/event';
import { insane, InsaneOptions } from 'vs/base/common/insane/insane';
import * as dompurify from 'vs/base/browser/dompurify/dompurify';
import { KeyCode } from 'vs/base/common/keyCodes';
import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { FileAccess, RemoteAuthorities } from 'vs/base/common/network';
import { FileAccess, RemoteAuthorities, Schemas } from 'vs/base/common/network';
import * as platform from 'vs/base/common/platform';
import { withNullAsUndefined } from 'vs/base/common/types';
import { URI } from 'vs/base/common/uri';
Expand Down Expand Up @@ -1361,52 +1361,41 @@ export function detectFullscreen(): IDetectedFullscreen | null {

// -- sanitize and trusted html

function _extInsaneOptions(opts: InsaneOptions, allowedAttributesForAll: string[]): InsaneOptions {
/**
* Sanitizes the given `value` and reset the given `node` with it.
*/
export function safeInnerHtml(node: HTMLElement, value: string): void {
const options: dompurify.Config = {
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'],
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'],
RETURN_DOM: false,
RETURN_DOM_FRAGMENT: false,
};

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

if (opts.allowedTags) {
for (let tag of opts.allowedTags) {
let array = allowedAttributes[tag];
if (!array) {
array = allowedAttributesForAll;
} else {
array = array.concat(allowedAttributesForAll);
// https://github.com/cure53/DOMPurify/blob/main/demos/hooks-scheme-allowlist.html
dompurify.addHook('afterSanitizeAttributes', (node) => {
// build an anchor to map URLs to
const anchor = document.createElement('a');

// check all href/src attributes for validity
for (const attr in ['href', 'src']) {
if (node.hasAttribute(attr)) {
anchor.href = node.getAttribute(attr) as string;
if (!allowedProtocols.includes(anchor.protocol)) {
node.removeAttribute(attr);
}
}
allowedAttributes[tag] = array;
}
}

return { ...opts, allowedAttributes };
}
});

const _ttpSafeInnerHtml = window.trustedTypes?.createPolicy('safeInnerHtml', {
createHTML(value, options: InsaneOptions) {
return insane(value, options);
try {
const html = dompurify.sanitize(value, { ...options, RETURN_TRUSTED_TYPE: true });
node.innerHTML = html as unknown as string;
} finally {
dompurify.removeHook('afterSanitizeAttributes');
}
});

/**
* Sanitizes the given `value` and reset the given `node` with it.
*/
export function safeInnerHtml(node: HTMLElement, value: string): void {

const options = _extInsaneOptions({
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'],
allowedAttributes: {
'a': ['href', 'x-dispatch'],
'button': ['data-href', 'x-dispatch'],
'input': ['type', 'placeholder', 'checked', 'required'],
'label': ['for'],
'select': ['required'],
'span': ['data-command', 'role'],
'textarea': ['name', 'placeholder', 'required'],
},
allowedSchemes: ['http', 'https', 'command']
}, ['class', 'id', 'role', 'tabindex']);

const html = _ttpSafeInnerHtml?.createHTML(value, options) ?? insane(value, options);
node.innerHTML = html as string;
}

/**
Expand Down
17 changes: 17 additions & 0 deletions src/vs/base/browser/dompurify/cgmanifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"registrations": [
{
"component": {
"type": "git",
"git": {
"name": "dompurify",
"repositoryUrl": "https://github.com/cure53/DOMPurify",
"commitHash": "6cfcdf56269b892550af80baa7c1fa5b680e5db7"
}
},
"license": "Apache 2.0",
"version": "2.3.1"
}
],
"version": 1
}
104 changes: 104 additions & 0 deletions src/vs/base/browser/dompurify/dompurify.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// Type definitions for DOM Purify 2.2
// Project: https://github.com/cure53/DOMPurify
// Definitions by: Dave Taylor https://github.com/davetayls
// Samira Bazuzi <https://github.com/bazuzi>
// FlowCrypt <https://github.com/FlowCrypt>
// Exigerr <https://github.com/Exigerr>
// Piotr Błażejewicz <https://github.com/peterblazejewicz>
// Nicholas Ellul <https://github.com/NicholasEllul>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped

export as namespace DOMPurify;
export = DOMPurify;

declare const DOMPurify: createDOMPurifyI;

interface createDOMPurifyI extends DOMPurify.DOMPurifyI {
(window?: Window): DOMPurify.DOMPurifyI;
}

declare namespace DOMPurify {
interface DOMPurifyI {
sanitize(source: string | Node): string;
sanitize(source: string | Node, config: Config & { RETURN_TRUSTED_TYPE: true }): TrustedHTML;
sanitize(source: string | Node, config: Config & { RETURN_DOM_FRAGMENT?: false | undefined; RETURN_DOM?: false | undefined }): string;
sanitize(source: string | Node, config: Config & { RETURN_DOM_FRAGMENT: true }): DocumentFragment;
sanitize(source: string | Node, config: Config & { RETURN_DOM: true }): HTMLElement;
sanitize(source: string | Node, config: Config): string | HTMLElement | DocumentFragment;

addHook(hook: 'uponSanitizeElement', cb: (currentNode: Element, data: SanitizeElementHookEvent, config: Config) => void): void;
addHook(hook: 'uponSanitizeAttribute', cb: (currentNode: Element, data: SanitizeAttributeHookEvent, config: Config) => void): void;
addHook(hook: HookName, cb: (currentNode: Element, data: HookEvent, config: Config) => void): void;

setConfig(cfg: Config): void;
clearConfig(): void;
isValidAttribute(tag: string, attr: string, value: string): boolean;

removeHook(entryPoint: HookName): void;
removeHooks(entryPoint: HookName): void;
removeAllHooks(): void;

version: string;
removed: any[];
isSupported: boolean;
}

interface Config {
ADD_ATTR?: string[] | undefined;
ADD_DATA_URI_TAGS?: string[] | undefined;
ADD_TAGS?: string[] | undefined;
ALLOW_DATA_ATTR?: boolean | undefined;
ALLOWED_ATTR?: string[] | undefined;
ALLOWED_TAGS?: string[] | undefined;
FORBID_ATTR?: string[] | undefined;
FORBID_TAGS?: string[] | undefined;
FORCE_BODY?: boolean | undefined;
KEEP_CONTENT?: boolean | undefined;
/**
* change the default namespace from HTML to something different
*/
NAMESPACE?: string | undefined;
RETURN_DOM?: boolean | undefined;
RETURN_DOM_FRAGMENT?: boolean | undefined;
/**
* This defaults to `true` starting DOMPurify 2.2.0. Note that setting it to `false`
* might cause XSS from attacks hidden in closed shadowroots in case the browser
* supports Declarative Shadow: DOM https://web.dev/declarative-shadow-dom/
*/
RETURN_DOM_IMPORT?: boolean | undefined;
RETURN_TRUSTED_TYPE?: boolean | undefined;
SANITIZE_DOM?: boolean | undefined;
WHOLE_DOCUMENT?: boolean | undefined;
ALLOWED_URI_REGEXP?: RegExp | undefined;
SAFE_FOR_TEMPLATES?: boolean | undefined;
ALLOW_UNKNOWN_PROTOCOLS?: boolean | undefined;
USE_PROFILES?: false | { mathMl?: boolean | undefined; svg?: boolean | undefined; svgFilters?: boolean | undefined; html?: boolean | undefined } | undefined;
IN_PLACE?: boolean | undefined;
}

type HookName =
| 'beforeSanitizeElements'
| 'uponSanitizeElement'
| 'afterSanitizeElements'
| 'beforeSanitizeAttributes'
| 'uponSanitizeAttribute'
| 'afterSanitizeAttributes'
| 'beforeSanitizeShadowDOM'
| 'uponSanitizeShadowNode'
| 'afterSanitizeShadowDOM';

type HookEvent = SanitizeElementHookEvent | SanitizeAttributeHookEvent | null;

interface SanitizeElementHookEvent {
tagName: string;
allowedTags: { [key: string]: boolean };
}

interface SanitizeAttributeHookEvent {
attrName: string;
attrValue: string;
keepAttr: boolean;
allowedAttributes: { [key: string]: boolean };
forceKeepAttr?: boolean | undefined;
}
}
Loading

0 comments on commit 474d495

Please sign in to comment.