You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2023/12/07 21:38:00 UTC
(superset) branch reduce-innerHTML-use created (now 1a20a9812a)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a change to branch reduce-innerHTML-use
in repository https://gitbox.apache.org/repos/asf/superset.git
at 1a20a9812a Wave 1
This branch includes the following new commits:
new 1a20a9812a Wave 1
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: Wave 1
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch reduce-innerHTML-use
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 1a20a9812ac5dc7e9b9c13a28d6d3bb2d3938940
Author: Evan Rusackas <ev...@rusackas.com>
AuthorDate: Thu Dec 7 14:37:48 2023 -0700
Wave 1
---
superset-frontend/.eslintrc.js | 1 +
superset-frontend/package-lock.json | 17 +++++++++++
superset-frontend/package.json | 1 +
.../packages/superset-ui-core/src/utils/index.ts | 1 +
.../src/utils/{index.ts => sanitizers.ts} | 35 ++++++++++++----------
.../test/chart/components/reactify.test.tsx | 2 ++
.../src/components/DynamicEditableTitle/index.tsx | 2 +-
.../src/components/DynamicPlugins/index.tsx | 6 +++-
superset-frontend/src/components/Icons/Icon.tsx | 13 ++++++--
.../src/dashboard/util/injectCustomCss.ts | 2 ++
superset-frontend/src/embedded/index.tsx | 2 +-
.../utils/client-ws-app/public/javascripts/app.js | 6 ++--
12 files changed, 64 insertions(+), 24 deletions(-)
diff --git a/superset-frontend/.eslintrc.js b/superset-frontend/.eslintrc.js
index 6eebb0d2df..048f0f1213 100644
--- a/superset-frontend/.eslintrc.js
+++ b/superset-frontend/.eslintrc.js
@@ -42,6 +42,7 @@ module.exports = {
'prettier',
'prettier/react',
'plugin:react-hooks/recommended',
+ 'plugin:no-unsanitized/DOM',
],
parser: '@babel/eslint-parser',
parserOptions: {
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
index c4e771b259..ff4f18189d 100644
--- a/superset-frontend/package-lock.json
+++ b/superset-frontend/package-lock.json
@@ -238,6 +238,7 @@
"eslint-plugin-jest-dom": "^3.6.5",
"eslint-plugin-jsx-a11y": "^6.4.1",
"eslint-plugin-no-only-tests": "^2.4.0",
+ "eslint-plugin-no-unsanitized": "^4.0.2",
"eslint-plugin-prettier": "^4.0.0",
"eslint-plugin-react": "^7.22.0",
"eslint-plugin-react-hooks": "^4.2.0",
@@ -30944,6 +30945,15 @@
"node": ">=4.0.0"
}
},
+ "node_modules/eslint-plugin-no-unsanitized": {
+ "version": "4.0.2",
+ "resolved": "https://registry.npmjs.org/eslint-plugin-no-unsanitized/-/eslint-plugin-no-unsanitized-4.0.2.tgz",
+ "integrity": "sha512-Pry0S9YmHoz8NCEMRQh7N0Yexh2MYCNPIlrV52hTmS7qXnTghWsjXouF08bgsrrZqaW9tt1ZiK3j5NEmPE+EjQ==",
+ "dev": true,
+ "peerDependencies": {
+ "eslint": "^6 || ^7 || ^8"
+ }
+ },
"node_modules/eslint-plugin-prettier": {
"version": "4.0.0",
"resolved": "https://registry.npmjs.org/eslint-plugin-prettier/-/eslint-plugin-prettier-4.0.0.tgz",
@@ -88668,6 +88678,13 @@
"integrity": "sha512-azP9PwQYfGtXJjW273nIxQH9Ygr+5/UyeW2wEjYoDtVYPI+WPKwbj0+qcAKYUXFZLRumq4HKkFaoDBAwBoXImQ==",
"dev": true
},
+ "eslint-plugin-no-unsanitized": {
+ "version": "4.0.2",
+ "resolved": "https://registry.npmjs.org/eslint-plugin-no-unsanitized/-/eslint-plugin-no-unsanitized-4.0.2.tgz",
+ "integrity": "sha512-Pry0S9YmHoz8NCEMRQh7N0Yexh2MYCNPIlrV52hTmS7qXnTghWsjXouF08bgsrrZqaW9tt1ZiK3j5NEmPE+EjQ==",
+ "dev": true,
+ "requires": {}
+ },
"eslint-plugin-prettier": {
"version": "4.0.0",
"resolved": "https://registry.npmjs.org/eslint-plugin-prettier/-/eslint-plugin-prettier-4.0.0.tgz",
diff --git a/superset-frontend/package.json b/superset-frontend/package.json
index 3b6310fa71..0fd65b8608 100644
--- a/superset-frontend/package.json
+++ b/superset-frontend/package.json
@@ -303,6 +303,7 @@
"eslint-plugin-jest-dom": "^3.6.5",
"eslint-plugin-jsx-a11y": "^6.4.1",
"eslint-plugin-no-only-tests": "^2.4.0",
+ "eslint-plugin-no-unsanitized": "^4.0.2",
"eslint-plugin-prettier": "^4.0.0",
"eslint-plugin-react": "^7.22.0",
"eslint-plugin-react-hooks": "^4.2.0",
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/index.ts b/superset-frontend/packages/superset-ui-core/src/utils/index.ts
index 32fa88251e..1ac83b863a 100644
--- a/superset-frontend/packages/superset-ui-core/src/utils/index.ts
+++ b/superset-frontend/packages/superset-ui-core/src/utils/index.ts
@@ -32,3 +32,4 @@ export * from './featureFlags';
export * from './random';
export * from './typedMemo';
export * from './html';
+export * from './sanitizers';
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/index.ts b/superset-frontend/packages/superset-ui-core/src/utils/sanitizers.ts
similarity index 50%
copy from superset-frontend/packages/superset-ui-core/src/utils/index.ts
copy to superset-frontend/packages/superset-ui-core/src/utils/sanitizers.ts
index 32fa88251e..ad73ee8056 100644
--- a/superset-frontend/packages/superset-ui-core/src/utils/index.ts
+++ b/superset-frontend/packages/superset-ui-core/src/utils/sanitizers.ts
@@ -16,19 +16,22 @@
* specific language governing permissions and limitations
* under the License.
*/
-export { default as convertKeysToCamelCase } from './convertKeysToCamelCase';
-export { default as ensureIsArray } from './ensureIsArray';
-export { default as ensureIsInt } from './ensureIsInt';
-export { default as isDefined } from './isDefined';
-export { default as isRequired } from './isRequired';
-export { default as isEqualArray } from './isEqualArray';
-export { default as makeSingleton } from './makeSingleton';
-export { default as promiseTimeout } from './promiseTimeout';
-export { default as logging } from './logging';
-export { default as removeDuplicates } from './removeDuplicates';
-export { lruCache } from './lruCache';
-export { getSelectedText } from './getSelectedText';
-export * from './featureFlags';
-export * from './random';
-export * from './typedMemo';
-export * from './html';
+export function sanitizeFileName(fileName: string) {
+ // Allow only alphanumeric characters, hyphens, and underscores
+ return fileName.replace(/[^a-zA-Z0-9-_]/g, '');
+}
+
+export function sanitizeUrl(url: string) {
+ try {
+ // Attempt to create a URL object. If this fails, the URL is invalid
+ // eslint-disable-next-line no-new
+ new URL(url);
+
+ // The URL is valid if no error was thrown.
+ // Proceed with removing JavaScript: and data: protocols to prevent XSS attacks
+ return url.replace(/(javascript:|data:)/gi, '');
+ } catch (e) {
+ // If URL is invalid, return an empty string or handle as needed
+ return '';
+ }
+}
diff --git a/superset-frontend/packages/superset-ui-core/test/chart/components/reactify.test.tsx b/superset-frontend/packages/superset-ui-core/test/chart/components/reactify.test.tsx
index 3277a6fece..a1086c14f9 100644
--- a/superset-frontend/packages/superset-ui-core/test/chart/components/reactify.test.tsx
+++ b/superset-frontend/packages/superset-ui-core/test/chart/components/reactify.test.tsx
@@ -29,6 +29,8 @@ describe('reactify(renderFn)', () => {
const container = element;
container.innerHTML = '';
const child = document.createElement('b');
+ // TODO: make this safer
+ // eslint-disable-next-line no-unsanitized/property
child.innerHTML = props.content ?? '';
container.append(child);
},
diff --git a/superset-frontend/src/components/DynamicEditableTitle/index.tsx b/superset-frontend/src/components/DynamicEditableTitle/index.tsx
index 86205bebc2..f2810f615d 100644
--- a/superset-frontend/src/components/DynamicEditableTitle/index.tsx
+++ b/superset-frontend/src/components/DynamicEditableTitle/index.tsx
@@ -113,7 +113,7 @@ export const DynamicEditableTitle = ({
// then we can measure the width of that span to resize the input element
useLayoutEffect(() => {
if (sizerRef?.current) {
- sizerRef.current.innerHTML = (currentTitle || placeholder).replace(
+ sizerRef.current.textContent = (currentTitle || placeholder).replace(
/\s/g,
' ',
);
diff --git a/superset-frontend/src/components/DynamicPlugins/index.tsx b/superset-frontend/src/components/DynamicPlugins/index.tsx
index 9134e73fda..3e2c65fa99 100644
--- a/superset-frontend/src/components/DynamicPlugins/index.tsx
+++ b/superset-frontend/src/components/DynamicPlugins/index.tsx
@@ -25,6 +25,7 @@ import {
getChartMetadataRegistry,
logging,
makeApi,
+ sanitizeUrl,
} from '@superset-ui/core';
import { omitBy } from 'lodash';
@@ -189,7 +190,10 @@ export const DynamicPluginProvider: React.FC = ({ children }) => {
plugins.map(async plugin => {
let error: Error | null = null;
try {
- await import(/* webpackIgnore: true */ plugin.bundle_url);
+ const plugin_bundle_url = sanitizeUrl(plugin.bundle_url);
+ // This is sanitized via a custom util function
+ // eslint-disable-next-line no-unsanitized/method
+ await import(/* webpackIgnore: true */ plugin_bundle_url);
} catch (err) {
logging.error(
`Failed to load plugin ${plugin.key} with the following error:`,
diff --git a/superset-frontend/src/components/Icons/Icon.tsx b/superset-frontend/src/components/Icons/Icon.tsx
index 283fd194ca..2bd951e16a 100644
--- a/superset-frontend/src/components/Icons/Icon.tsx
+++ b/superset-frontend/src/components/Icons/Icon.tsx
@@ -19,7 +19,7 @@
import React, { useEffect, useRef, useState } from 'react';
import AntdIcon from '@ant-design/icons';
-import { styled } from '@superset-ui/core';
+import { styled, sanitizeFileName } from '@superset-ui/core';
import TransparentIcon from 'src/assets/images/icons/transparent.svg';
import IconType from './IconType';
@@ -55,9 +55,18 @@ export const Icon = (props: IconProps) => {
useEffect(() => {
let cancelled = false;
async function importIcon(): Promise<void> {
+ // This is sanitized via a custom util function
+ /* eslint-disable no-unsanitized/method */
+
ImportedSVG.current = (
- await import(`!!@svgr/webpack!src/assets/images/icons/${fileName}.svg`)
+ await import(
+ `!!@svgr/webpack!src/assets/images/icons/${sanitizeFileName(
+ fileName,
+ )}.svg`
+ )
).default;
+ /* eslint-enable no-unsanitized/method */
+
if (!cancelled) {
setLoaded(true);
}
diff --git a/superset-frontend/src/dashboard/util/injectCustomCss.ts b/superset-frontend/src/dashboard/util/injectCustomCss.ts
index 43cb66f7d9..8df8daec7d 100644
--- a/superset-frontend/src/dashboard/util/injectCustomCss.ts
+++ b/superset-frontend/src/dashboard/util/injectCustomCss.ts
@@ -42,6 +42,8 @@ export default function injectCustomCss(css: string) {
if ('styleSheet' in style) {
(style as HTMLStyleElement & MysteryStyleElement).styleSheet.cssText = css;
} else {
+ // TODO: find a way to avoid innerHTML here, or make this less scary
+ // eslint-disable-next-line no-unsanitized/property
style.innerHTML = css;
}
diff --git a/superset-frontend/src/embedded/index.tsx b/superset-frontend/src/embedded/index.tsx
index 50c026fba8..084ef90fa4 100644
--- a/superset-frontend/src/embedded/index.tsx
+++ b/superset-frontend/src/embedded/index.tsx
@@ -72,7 +72,7 @@ const appMountPoint = document.getElementById('app')!;
const MESSAGE_TYPE = '__embedded_comms__';
function showFailureMessage(message: string) {
- appMountPoint.innerHTML = message;
+ appMountPoint.textContent = message;
}
if (!window.parent || window.parent === window) {
diff --git a/superset-websocket/utils/client-ws-app/public/javascripts/app.js b/superset-websocket/utils/client-ws-app/public/javascripts/app.js
index 5afc847a18..d248287934 100644
--- a/superset-websocket/utils/client-ws-app/public/javascripts/app.js
+++ b/superset-websocket/utils/client-ws-app/public/javascripts/app.js
@@ -42,7 +42,7 @@ function connect() {
// Connection opened
socket.addEventListener('open', function () {
socketCount++;
- document.getElementById('socket-count').innerHTML = socketCount;
+ document.getElementById('socket-count').textContent = socketCount;
connect();
socket.send('Hello Server!');
@@ -58,6 +58,6 @@ function connect() {
connect();
setInterval(() => {
- document.getElementById('message-count').innerHTML = messageCount;
- document.getElementById('message-debug').innerHTML = lastMessage;
+ document.getElementById('message-count').textContent = messageCount;
+ document.getElementById('message-debug').textContent = lastMessage;
}, 250);