From 0970ab8bed05c4996e388b0730619f5053ea552f Mon Sep 17 00:00:00 2001
From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com>
Date: Mon, 11 Nov 2024 23:10:30 -0800
Subject: [PATCH] chore: Add the eslint-no-unsanitized rule (#85)
This commit adds the eslint-plugin-no-unsanitized rule, which flags most usages of .innerHTML and document.write as unsafe.
---
.eslintrc.js | 1 +
docs/demo/icons.ts | 15 +-
docs/demo/ui/makeNewImageDialog.ts | 7 +-
docs/demo/ui/newImageDialog.css | 6 +-
package.json | 1 +
packages/js-draw/src/SVGLoader/SVGLoader.ts | 2 +
.../src/rendering/renderers/SVGRenderer.ts | 8 +-
packages/js-draw/src/toolbar/IconProvider.ts | 248 +++++++++---------
packages/js-draw/src/util/createElement.ts | 113 ++++++++
packages/material-icons/src/lib.ts | 14 +-
packages/material-icons/src/types.ts | 11 +
packages/material-icons/tools/prebuild.js | 4 +-
yarn.lock | 10 +
13 files changed, 294 insertions(+), 146 deletions(-)
create mode 100644 packages/js-draw/src/util/createElement.ts
create mode 100644 packages/material-icons/src/types.ts
diff --git a/.eslintrc.js b/.eslintrc.js
index 604409c41..8b6ae3b16 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -8,6 +8,7 @@ module.exports = {
'eslint:recommended',
'plugin:@typescript-eslint/eslint-recommended',
'plugin:@typescript-eslint/recommended',
+ 'plugin:no-unsanitized/recommended-legacy',
'eslint-config-prettier',
],
// See https://typescript-eslint.io/linting/troubleshooting/#i-get-errors-telling-me-eslint-was-configured-to-run--however-that-tsconfig-does-not--none-of-those-tsconfigs-include-this-file
diff --git a/docs/demo/icons.ts b/docs/demo/icons.ts
index becfa9075..e272845fd 100644
--- a/docs/demo/icons.ts
+++ b/docs/demo/icons.ts
@@ -41,11 +41,16 @@ export const makeIconFromText = (text: string) => {
export const makeLocalStorageIcon = () => {
const elem = document.createElementNS(svgNamespace, 'svg');
- const iconFillSStyle = 'style="fill: var(--icon-color);"';
- elem.innerHTML = `
-
-
- `;
+
+ const makePath = (d: string) => {
+ const path = document.createElementNS(svgNamespace, 'path');
+ path.style.fill = 'var(--icon-color)';
+ path.setAttribute('d', d);
+ elem.appendChild(path);
+ };
+
+ makePath('M 50,10 V 60 H 35 L 55,85 75,60 H 60 V 10 Z');
+ makePath('m 15,85 v 10 h 85 V 85 Z');
elem.setAttribute('viewBox', '5 0 100 100');
return elem;
diff --git a/docs/demo/ui/makeNewImageDialog.ts b/docs/demo/ui/makeNewImageDialog.ts
index b701d6a5a..cdfc1947a 100644
--- a/docs/demo/ui/makeNewImageDialog.ts
+++ b/docs/demo/ui/makeNewImageDialog.ts
@@ -87,10 +87,11 @@ const makeNewImageDialog = (
const templateButton = document.createElement('button');
templateButton.classList.add('new-image-template-button');
- const icon = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
- icon.innerHTML = svgData;
+ const icon = new Image();
+ icon.classList.add('icon');
+ icon.src = `data:image/svg+xml;utf8,${encodeURIComponent(svgData)}`;
const label = document.createElement('div');
- label.innerText = title;
+ label.textContent = title;
templateButton.onclick = () => {
closeDialogWithStringResult(svgData);
diff --git a/docs/demo/ui/newImageDialog.css b/docs/demo/ui/newImageDialog.css
index 6aa17544f..19ed56dc2 100644
--- a/docs/demo/ui/newImageDialog.css
+++ b/docs/demo/ui/newImageDialog.css
@@ -14,7 +14,11 @@
}
.new-image-template-button .icon {
- width: 95%;
+ width: 300px;
+ height: 150px;
+
+ object-position: left top;
+ object-fit: none; /* Do not resize to fit container */
}
.new-image-template-button:hover {
diff --git a/package.json b/package.json
index e704a458a..866d6ac21 100644
--- a/package.json
+++ b/package.json
@@ -31,6 +31,7 @@
"@typescript-eslint/parser": "8.4.0",
"eslint": "8.57.0",
"eslint-config-prettier": "9.1.0",
+ "eslint-plugin-no-unsanitized": "4.1.2",
"jest": "29.7.0",
"jest-environment-jsdom": "29.7.0",
"jsdom": "25.0.0",
diff --git a/packages/js-draw/src/SVGLoader/SVGLoader.ts b/packages/js-draw/src/SVGLoader/SVGLoader.ts
index 62b588003..eb9ce8979 100644
--- a/packages/js-draw/src/SVGLoader/SVGLoader.ts
+++ b/packages/js-draw/src/SVGLoader/SVGLoader.ts
@@ -700,6 +700,7 @@ export default class SVGLoader implements ImageLoader {
const { svgElem, cleanUp } = (() => {
// If the user requested an iframe load (the default) try to load with an iframe.
// There are some cases (e.g. in a sandboxed iframe) where this doesn't work.
+ // TODO(v2): Use domParserLoad by default.
if (!domParserLoad) {
try {
const sandbox = document.createElement('iframe');
@@ -744,6 +745,7 @@ export default class SVGLoader implements ImageLoader {
sandboxDoc.close();
const svgElem = sandboxDoc.createElementNS('http://www.w3.org/2000/svg', 'svg');
+ // eslint-disable-next-line no-unsanitized/property -- setting innerHTML in a sandboxed document.
svgElem.innerHTML = text;
sandboxDoc.body.appendChild(svgElem);
diff --git a/packages/js-draw/src/rendering/renderers/SVGRenderer.ts b/packages/js-draw/src/rendering/renderers/SVGRenderer.ts
index 49e584d3b..51286559f 100644
--- a/packages/js-draw/src/rendering/renderers/SVGRenderer.ts
+++ b/packages/js-draw/src/rendering/renderers/SVGRenderer.ts
@@ -71,7 +71,9 @@ export default class SVGRenderer extends AbstractRenderer {
if (!this.elem.querySelector(`#${renderedStylesheetId}`)) {
// Default to rounded strokes.
const styleSheet = document.createElementNS('http://www.w3.org/2000/svg', 'style');
- styleSheet.innerHTML = `
+ styleSheet.appendChild(
+ document.createTextNode(
+ `
path {
stroke-linecap: round;
stroke-linejoin: round;
@@ -80,7 +82,9 @@ export default class SVGRenderer extends AbstractRenderer {
text {
white-space: pre;
}
- `.replace(/\s+/g, '');
+ `.replace(/\s+/g, ''),
+ ),
+ );
styleSheet.setAttribute('id', renderedStylesheetId);
this.elem.appendChild(styleSheet);
}
diff --git a/packages/js-draw/src/toolbar/IconProvider.ts b/packages/js-draw/src/toolbar/IconProvider.ts
index f1e5eb9b5..d2099c1b3 100644
--- a/packages/js-draw/src/toolbar/IconProvider.ts
+++ b/packages/js-draw/src/toolbar/IconProvider.ts
@@ -7,16 +7,11 @@ import Viewport from '../Viewport';
import { makeFreehandLineBuilder } from '../components/builders/FreehandLineBuilder';
import { makePolylineBuilder } from '../components/builders/PolylineBuilder';
import { EraserMode } from '../tools/Eraser';
+import { createSvgElement, createSvgElements, createSvgPaths } from '../util/createElement';
export type IconElemType = HTMLImageElement | SVGElement;
const svgNamespace = 'http://www.w3.org/2000/svg';
-const iconColorFill = `
- style='fill: var(--icon-color);'
-`;
-const iconColorStrokeFill = `
- style='fill: var(--icon-color); stroke: var(--icon-color);'
-`;
let checkerboardIdCounter = 0;
const makeCheckerboardPattern = () => {
@@ -36,7 +31,15 @@ const makeCheckerboardPattern = () => {
`;
const patternRef = `url(#${id})`;
- return { patternDef, patternRef };
+ return {
+ patternDefElement: (): SVGElement => {
+ const element = new DOMParser().parseFromString(patternDef, 'image/svg+xml');
+ return element.firstChild as SVGElement;
+ },
+ // @deprecated use patternDefElement
+ patternDef,
+ patternRef,
+ };
};
const makeRedoIcon = (mirror: boolean) => {
@@ -53,11 +56,16 @@ const makeRedoIcon = (mirror: boolean) => {
transform-origin: center;
}
-
`;
+
+ const path = document.createElementNS(svgNamespace, 'path');
+ path.setAttribute('d', 'M20,20 A15,15 0 0 1 70,80 L80,90 L60,70 L65,90 L87,90 L65,80');
+ path.classList.add('toolbar-svg-undo-redo-icon');
+ if (mirror) {
+ path.style.transform = 'scale(-1, 1)';
+ }
+ icon.appendChild(path);
+
icon.setAttribute('viewBox', '0 0 100 100');
return icon;
};
@@ -105,68 +113,65 @@ export default class IconProvider {
}
public makeDropdownIcon(): IconElemType {
- const icon = document.createElementNS(svgNamespace, 'svg');
- icon.innerHTML = `
-
-
-
- `;
+ const icon = this.makeIconFromPath('M5,10 L50,90 L95,10 Z');
icon.setAttribute('viewBox', '-10 -10 110 110');
return icon;
}
public makeEraserIcon(eraserSize?: number, mode?: EraserMode): IconElemType {
- const icon = document.createElementNS(svgNamespace, 'svg');
eraserSize ??= 10;
const scaledSize = eraserSize / 4;
const eraserColor = '#ff70af';
// Draw an eraser-like shape. Created with Inkscape
- icon.innerHTML = `
-
-
-
-
-
-
-
-
-
-
-
-
- `;
- icon.setAttribute('viewBox', '0 0 120 120');
+ const icon = createSvgElement('svg', {
+ viewBox: '0 0 120 120',
+ children: [
+ createSvgElement('defs', {
+ children: [
+ createSvgElement('linearGradient', {
+ id: 'dash-pattern',
+ children: createSvgElements('stop', [
+ { offset: '80%', 'stop-color': eraserColor },
+ { offset: '85%', 'stop-color': 'white' },
+ { offset: '90%', 'stop-color': eraserColor },
+ ]),
+ }),
+ ],
+ }),
+ createSvgElement('path', {
+ fill: mode === EraserMode.PartialStroke ? 'url(#dash-pattern)' : eraserColor,
+ stroke: 'black',
+ transform: 'rotate(41.35)',
+ d: `
+ M 52.5 27
+ C 50 28.9 48.9 31.7 48.9 34.8
+ L 48.9 39.8
+ C 48.9 45.3 53.4 49.8 58.9 49.8
+ L 103.9 49.8
+ C 105.8 49.8 107.6 49.2 109.1 48.3
+ L 110.2 ${scaledSize + 49.5} L 159.7 ${scaledSize + 5}
+ L 157.7 ${-scaledSize + 5.2} L 112.4 ${49.5 - scaledSize}
+ C 113.4 43.5 113.9 41.7 113.9 39.8
+ L 113.9 34.8
+ C 113.9 29.3 109.4 24.8 103.9 24.8
+ L 58.9 24.8
+ C 56.5 24.8 54.3 25.7 52.5 27
+ z
+ `,
+ }),
+ createSvgElement('rect', {
+ stroke: '#cc8077',
+ fill: 'var(--icon-color)',
+ width: 65,
+ height: 75,
+ x: 48.9,
+ y: -38.7,
+ transform: 'rotate(41.35)',
+ }),
+ ],
+ });
return icon;
}
@@ -176,8 +181,8 @@ export default class IconProvider {
// Draw a cursor-like shape
icon.innerHTML = `
-
-
+
+
`;
icon.setAttribute('viewBox', '0 0 100 100');
@@ -483,8 +488,6 @@ export default class IconProvider {
const color = penStyle.color;
const rounded = this.isRoundedTipPen(penStyle);
- const icon = document.createElementNS(svgNamespace, 'svg');
- icon.setAttribute('viewBox', '0 0 100 100');
const tipThickness = strokeSize / 2;
const inkTipPath = `
@@ -525,74 +528,54 @@ export default class IconProvider {
const checkerboardPattern = makeCheckerboardPattern();
- const ink = `
-
-
-
-
- `;
+ const colorString = color.toHexString();
+ const ink = createSvgPaths(
+ {
+ fill: checkerboardPattern.patternRef,
+ d: inkTipPath,
+ },
+ {
+ fill: checkerboardPattern.patternRef,
+ d: inkTrailPath,
+ },
+ {
+ fill: colorString,
+ d: inkTipPath,
+ },
+ {
+ fill: colorString,
+ d: inkTrailPath,
+ },
+ );
- const penTip = `
-
-
- `;
+ const penTip = createSvgPaths(
+ { fill: checkerboardPattern.patternRef, d: penTipPath },
+ { fill: tipColor, stroke: colorString, d: penTipPath },
+ );
- const grip = `
-
+ const grip = createSvgPaths(
+ { fill: 'var(--icon-color)', stroke: 'var(--icon-color)', d: gripMainPath },
-
-
-
+ // Shadows
+ { fill: 'rgba(150, 150, 150, 0.3)', d: gripShadow1Path },
+ { fill: 'rgba(100, 100, 100, 0.2)', d: gripShadow2Path },
-
-
-
- `;
+ // Color bubble
+ { fill: checkerboardPattern.patternRef, d: colorBubblePath },
+ { fill: colorString, d: colorBubblePath },
+ );
- icon.innerHTML = `
-
- ${checkerboardPattern.patternDef}
-
-
- ${ink}
- ${penTip}
- ${grip}
-
- `;
+ const icon = document.createElementNS(svgNamespace, 'svg');
+ icon.setAttribute('viewBox', '0 0 100 100');
+
+ const iconMainContent = createSvgElement('g', {
+ children: [ink, penTip, grip].flat(),
+ });
+ const defs = createSvgElement('defs', {
+ children: [checkerboardPattern.patternDefElement()],
+ });
+
+ icon.replaceChildren(defs, iconMainContent);
return icon;
}
@@ -631,7 +614,7 @@ export default class IconProvider {
const checkerboardPattern = makeCheckerboardPattern();
const defs = document.createElementNS(svgNamespace, 'defs');
- defs.innerHTML = checkerboardPattern.patternDef;
+ defs.appendChild(checkerboardPattern.patternDefElement());
icon.appendChild(defs);
const background = document.createElementNS(svgNamespace, 'g');
@@ -719,7 +702,7 @@ export default class IconProvider {
const checkerboardPattern = makeCheckerboardPattern();
const defs = document.createElementNS(svgNamespace, 'defs');
- defs.innerHTML = checkerboardPattern.patternDef;
+ defs.appendChild(checkerboardPattern.patternDefElement());
icon.appendChild(defs);
const fluidBackground = document.createElementNS(svgNamespace, 'path');
@@ -979,6 +962,9 @@ export default class IconProvider {
* @returns An object with both the definition of a checkerboard pattern and the syntax to
* reference that pattern. The defs provided by this function should be wrapped within a
* `` element.
+ *
+ * **Note**: This function's return value includes both `patternDefElement` (which returns
+ * an Element) and a (deprecated) `patternDef` string. Avoid using the `patternDef` result.
*/
protected makeCheckerboardPattern() {
return makeCheckerboardPattern();
diff --git a/packages/js-draw/src/util/createElement.ts b/packages/js-draw/src/util/createElement.ts
new file mode 100644
index 000000000..4c278f374
--- /dev/null
+++ b/packages/js-draw/src/util/createElement.ts
@@ -0,0 +1,113 @@
+type ElementTagNames = keyof HTMLElementTagNameMap | keyof SVGElementTagNameMap;
+
+/**
+ * Maps from known elment tag names to options that can be set with .setAttribute.
+ * New elements/properties should be added as necessary.
+ */
+interface ElementToPropertiesMap {
+ path: {
+ d: string;
+ fill: string;
+ stroke: string;
+ transform: string;
+ };
+ rect: {
+ stroke: string;
+ fill: string;
+ x: number;
+ y: number;
+ width: number;
+ height: number;
+ transform: string;
+ };
+ stop: {
+ offset: string;
+ 'stop-color': string;
+ };
+ svg: {
+ viewBox: `${number} ${number} ${number} ${number}`;
+ };
+}
+type EmptyObject = Record;
+type ElementProperties = Tag extends keyof ElementToPropertiesMap
+ ? Partial
+ : EmptyObject;
+
+/** Contains options for creating an element with tag = `Tag`. */
+type ElementConfig = ElementProperties & {
+ id?: string;
+ children?: (HTMLElement | SVGElement)[];
+};
+
+/**
+ * Maps from element tag names (e.g. `Tag='button'`) to the corresponding element type
+ * (e.g. `HTMLButtonElement`).
+ */
+type ElementTagToType = Tag extends keyof HTMLElementTagNameMap
+ ? HTMLElementTagNameMap[Tag]
+ : Tag extends keyof SVGElementTagNameMap
+ ? SVGElementTagNameMap[Tag]
+ : never;
+
+export enum ElementNamespace {
+ Html = 'html',
+ Svg = 'svg',
+}
+
+/**
+ * Shorthand for creating an element with `document.createElement`, then assigning properties.
+ *
+ * Non-HTML elements (e.g. `svg` elements) should use the `elementType` parameter to select
+ * the element namespace.
+ */
+const createElement = (
+ tag: Tag,
+ props: ElementConfig,
+ elementType: ElementNamespace = ElementNamespace.Html,
+): ElementTagToType => {
+ let elem: ElementTagToType;
+ if (elementType === ElementNamespace.Html) {
+ elem = document.createElement(tag) as ElementTagToType;
+ } else if (elementType === ElementNamespace.Svg) {
+ elem = document.createElementNS('http://www.w3.org/2000/svg', tag) as ElementTagToType;
+ } else {
+ throw new Error(`Unknown element type ${elementType}`);
+ }
+
+ for (const [key, value] of Object.entries(props)) {
+ if (key === 'children') continue;
+
+ if (typeof value !== 'string' && typeof value !== 'number') {
+ throw new Error(`Unsupported value type ${typeof value}`);
+ }
+ elem.setAttribute(key, value.toString());
+ }
+
+ if (props.children) {
+ for (const item of props.children) {
+ elem.appendChild(item);
+ }
+ }
+
+ return elem;
+};
+
+export const createSvgElement = (
+ tag: Tag,
+ props: ElementConfig,
+) => {
+ return createElement(tag, props, ElementNamespace.Svg);
+};
+
+export const createSvgElements = (
+ tag: Tag,
+ elements: ElementConfig[],
+) => {
+ return elements.map((props) => createSvgElement(tag, props));
+};
+
+export const createSvgPaths = (...paths: ElementConfig<'path'>[]) => {
+ return createSvgElements('path', paths);
+};
+
+export default createElement;
diff --git a/packages/material-icons/src/lib.ts b/packages/material-icons/src/lib.ts
index bede8f94e..f83c15761 100644
--- a/packages/material-icons/src/lib.ts
+++ b/packages/material-icons/src/lib.ts
@@ -59,10 +59,18 @@ import Shapes from './icons/Shapes.svg';
import Draw from './icons/Draw.svg';
import InkPen from './icons/InkPen.svg';
import ContentPaste from './icons/ContentPaste.svg';
+import { OpaqueIconType } from './types';
-const icon = (data: string) => {
+/**
+ * Converts an icon to an HTML element.
+ *
+ * This function accepts an "opaque" type to avoid unsafely creating icon DOM elements
+ * from untrusted strings.
+ */
+const icon = (data: OpaqueIconType) => {
const icon = document.createElement('div');
- icon.innerHTML = data;
+ // eslint-disable-next-line no-unsanitized/property -- data must not be user-provided (enforced with a custom type).
+ icon.innerHTML = data as unknown as string;
return icon.childNodes[0] as SVGElement;
};
@@ -139,7 +147,7 @@ class MaterialIconProvider extends IconProvider {
if (style.color.a < 1) {
const checkerboard = this.makeCheckerboardPattern();
const defs = document.createElementNS('http://www.w3.org/2000/svg', 'defs');
- defs.innerHTML = checkerboard.patternDef;
+ defs.appendChild(checkerboard.patternDefElement());
svg.appendChild(defs);
const lineBackground = line.cloneNode() as SVGPathElement;
diff --git a/packages/material-icons/src/types.ts b/packages/material-icons/src/types.ts
new file mode 100644
index 000000000..91efe1853
--- /dev/null
+++ b/packages/material-icons/src/types.ts
@@ -0,0 +1,11 @@
+/**
+ * All auto-generated icons should have this type.
+ *
+ * Goal: Ensure that icon processing functions are working with the original
+ * auto-generated Material icons.
+ *
+ * @internal
+ */
+export interface OpaqueIconType {
+ __opaqueType__iconType: unknown;
+}
diff --git a/packages/material-icons/tools/prebuild.js b/packages/material-icons/tools/prebuild.js
index 39c66cdca..e6cb840c0 100644
--- a/packages/material-icons/tools/prebuild.js
+++ b/packages/material-icons/tools/prebuild.js
@@ -35,7 +35,9 @@ const convertIcons = async () => {
// modified to set the fill of the icon.
// The icon was downloaded from https://fonts.google.com/icons
- export const ${iconName} = ${JSON.stringify(updatedIcon)};
+ import { OpaqueIconType } from '../types';
+
+ export const ${iconName} = ${JSON.stringify(updatedIcon)} as unknown as OpaqueIconType;
export default ${iconName};
`,
);
diff --git a/yarn.lock b/yarn.lock
index 122ff21e8..6e9d453f5 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1108,6 +1108,7 @@ __metadata:
"@typescript-eslint/parser": "npm:8.4.0"
eslint: "npm:8.57.0"
eslint-config-prettier: "npm:9.1.0"
+ eslint-plugin-no-unsanitized: "npm:4.1.2"
jest: "npm:29.7.0"
jest-environment-jsdom: "npm:29.7.0"
jsdom: "npm:25.0.0"
@@ -4097,6 +4098,15 @@ __metadata:
languageName: node
linkType: hard
+"eslint-plugin-no-unsanitized@npm:4.1.2":
+ version: 4.1.2
+ resolution: "eslint-plugin-no-unsanitized@npm:4.1.2"
+ peerDependencies:
+ eslint: ^8 || ^9
+ checksum: 10c0/c62bbfc02e8a73adc7ff9fdbbf8131a2a843eaebc6e8eb5f311c4db8a641bb46535bd066052857c7daea6c6d2a6c38bec66e06ad5704319083a1e2c467e71a4a
+ languageName: node
+ linkType: hard
+
"eslint-scope@npm:5.1.1":
version: 5.1.1
resolution: "eslint-scope@npm:5.1.1"