From 54f43d95dbb2d4c12ff787f81db7e85ff1b0d130 Mon Sep 17 00:00:00 2001 From: LukeWood Date: Tue, 8 Sep 2020 11:54:20 -0700 Subject: [PATCH 1/2] Introduces XSS sink detecting eslint rules These eslint rules attempt to surface XSS sinks from eslint. This commit is a first pass on https://github.com/eclipse-theia/theia/issues/8398 Change-Id: I142bbba9785c4567dfbea1380f5a980560cf7413 Signed-off-by: LukeWood --- .eslintrc.js | 3 +- configs/xss.eslintrc.json | 29 +++++++++ package.json | 2 + yarn.lock | 129 +++++++++++++++++++++++++++++++++++++- 4 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 configs/xss.eslintrc.json diff --git a/.eslintrc.js b/.eslintrc.js index bbfe5b0e31ac4..a76d59ddd0eb7 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -4,7 +4,8 @@ module.exports = { extends: [ './configs/base.eslintrc.json', './configs/warnings.eslintrc.json', - './configs/errors.eslintrc.json' + './configs/errors.eslintrc.json', + './configs/xss.eslintrc.json' ], ignorePatterns: [ '**/{node_modules,lib}', diff --git a/configs/xss.eslintrc.json b/configs/xss.eslintrc.json new file mode 100644 index 0000000000000..11745db462ccb --- /dev/null +++ b/configs/xss.eslintrc.json @@ -0,0 +1,29 @@ +{ + "extends": ["plugin:no-unsanitized/DOM"], + "plugins": ["no-unsanitized", "react"], + "parserOptions": { + "ecmaFeatures": { + "jsx": true + } + }, + "rules": { + "no-unsanitized/method": [ + "warn", { + "escape": { + "methods": ["DOMPurify.sanitize"] + } + } + ], + "no-unsanitized/property": [ + "warn", { + "escape": { + "methods": ["DOMPurify.sanitize"] + } + } + ], + "no-eval": "warn", + "no-implied-eval": "warn", + "react/no-danger-with-children": "warn", + "react/no-danger": "warn" + } +} diff --git a/package.json b/package.json index b58e841ce7a8e..4df21f03e2f23 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,8 @@ "eslint-plugin-deprecation": "^1.1.0", "eslint-plugin-import": "^2.20.0", "eslint-plugin-no-null": "^1.0.2", + "eslint-plugin-no-unsanitized": "^3.1.2", + "eslint-plugin-react": "^7.20.6", "ignore-styles": "^5.0.1", "jsdom": "^11.5.1", "lerna": "^2.2.0", diff --git a/yarn.lock b/yarn.lock index a460db9f0e78a..5a46c3023a035 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2144,6 +2144,15 @@ array.prototype.flat@^1.2.3: define-properties "^1.1.3" es-abstract "^1.17.0-next.1" +array.prototype.flatmap@^1.2.3: + version "1.2.3" + resolved "https://registry.yarnpkg.com/array.prototype.flatmap/-/array.prototype.flatmap-1.2.3.tgz#1c13f84a178566042dd63de4414440db9222e443" + integrity sha512-OOEk+lkePcg+ODXIpvuU9PAryCikCJyo7GlDG1upleEpQRx6mzL9puEBkozQ5iAx20KV0l3DbyQwqciJtqe5Pg== + dependencies: + define-properties "^1.1.3" + es-abstract "^1.17.0-next.1" + function-bind "^1.1.1" + arrify@^1.0.0, arrify@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/arrify/-/arrify-1.0.1.tgz#898508da2226f380df904728456849c1501a4b0d" @@ -5009,6 +5018,13 @@ doctrine@1.5.0: esutils "^2.0.2" isarray "^1.0.0" +doctrine@^2.1.0: + version "2.1.0" + resolved "https://registry.yarnpkg.com/doctrine/-/doctrine-2.1.0.tgz#5cd01fc101621b42c4cd7f5d1a66243716d3f39d" + integrity sha512-35mSku4ZXK0vfCuHEDAwt55dg2jNajHZ1odvF+8SSr82EsZY4QmXfuWso8oEd8zRhVObSN18aM0CjSdoBX7zIw== + dependencies: + esutils "^2.0.2" + doctrine@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/doctrine/-/doctrine-3.0.0.tgz#addebead72a6574db783639dc87a121773973961" @@ -5385,6 +5401,24 @@ es-abstract@^1.17.0, es-abstract@^1.17.0-next.1, es-abstract@^1.17.5: string.prototype.trimend "^1.0.1" string.prototype.trimstart "^1.0.1" +es-abstract@^1.18.0-next.0: + version "1.18.0-next.0" + resolved "https://registry.yarnpkg.com/es-abstract/-/es-abstract-1.18.0-next.0.tgz#b302834927e624d8e5837ed48224291f2c66e6fc" + integrity sha512-elZXTZXKn51hUBdJjSZGYRujuzilgXo8vSPQzjGYXLvSlGiCo8VO8ZGV3kjo9a0WNJJ57hENagwbtlRuHuzkcQ== + dependencies: + es-to-primitive "^1.2.1" + function-bind "^1.1.1" + has "^1.0.3" + has-symbols "^1.0.1" + is-callable "^1.2.0" + is-negative-zero "^2.0.0" + is-regex "^1.1.1" + object-inspect "^1.8.0" + object-keys "^1.1.1" + object.assign "^4.1.0" + string.prototype.trimend "^1.0.1" + string.prototype.trimstart "^1.0.1" + es-to-primitive@^1.2.1: version "1.2.1" resolved "https://registry.yarnpkg.com/es-to-primitive/-/es-to-primitive-1.2.1.tgz#e55cd4c9cdc188bcefb03b366c736323fc5c898a" @@ -5485,6 +5519,28 @@ eslint-plugin-no-null@^1.0.2: resolved "https://registry.yarnpkg.com/eslint-plugin-no-null/-/eslint-plugin-no-null-1.0.2.tgz#1236a812391390a1877ad4007c26e745341c951f" integrity sha1-EjaoEjkTkKGHetQAfCbnRTQclR8= +eslint-plugin-no-unsanitized@^3.1.2: + version "3.1.2" + resolved "https://registry.yarnpkg.com/eslint-plugin-no-unsanitized/-/eslint-plugin-no-unsanitized-3.1.2.tgz#a54724e0b81d43279bb1f8f5e1d82c97da78c858" + integrity sha512-KPShfliA3Uy9qqwQx35P1fwIOeJjZkb0FbMMUFztRYRposzaynsM8JCEb952fqkidROl1kpqY80uSvn+TcWkQQ== + +eslint-plugin-react@^7.20.6: + version "7.20.6" + resolved "https://registry.yarnpkg.com/eslint-plugin-react/-/eslint-plugin-react-7.20.6.tgz#4d7845311a93c463493ccfa0a19c9c5d0fd69f60" + integrity sha512-kidMTE5HAEBSLu23CUDvj8dc3LdBU0ri1scwHBZjI41oDv4tjsWZKU7MQccFzH1QYPYhsnTF2ovh7JlcIcmxgg== + dependencies: + array-includes "^3.1.1" + array.prototype.flatmap "^1.2.3" + doctrine "^2.1.0" + has "^1.0.3" + jsx-ast-utils "^2.4.1" + object.entries "^1.1.2" + object.fromentries "^2.0.2" + object.values "^1.1.1" + prop-types "^15.7.2" + resolve "^1.17.0" + string.prototype.matchall "^4.0.2" + eslint-scope@^4.0.3: version "4.0.3" resolved "https://registry.yarnpkg.com/eslint-scope/-/eslint-scope-4.0.3.tgz#ca03833310f6889a3264781aa82e63eb9cfe7848" @@ -7341,6 +7397,15 @@ inquirer@^7.0.0, inquirer@^7.1.0: strip-ansi "^6.0.0" through "^2.3.6" +internal-slot@^1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/internal-slot/-/internal-slot-1.0.2.tgz#9c2e9fb3cd8e5e4256c6f45fe310067fcfa378a3" + integrity sha512-2cQNfwhAfJIkU4KZPkDI+Gj5yNNnbqi40W9Gge6dfnk4TocEVm00B3bdiL+JINrbGJil2TeHvM4rETGzk/f/0g== + dependencies: + es-abstract "^1.17.0-next.1" + has "^1.0.3" + side-channel "^1.0.2" + interpret@^1.0.0, interpret@^1.0.4: version "1.4.0" resolved "https://registry.yarnpkg.com/interpret/-/interpret-1.4.0.tgz#665ab8bc4da27a774a40584e812e3e0fa45b1a1e" @@ -7575,6 +7640,11 @@ is-natural-number@^4.0.1: resolved "https://registry.yarnpkg.com/is-natural-number/-/is-natural-number-4.0.1.tgz#ab9d76e1db4ced51e35de0c72ebecf09f734cde8" integrity sha1-q5124dtM7VHjXeDHLr7PCfc0zeg= +is-negative-zero@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/is-negative-zero/-/is-negative-zero-2.0.0.tgz#9553b121b0fac28869da9ed459e20c7543788461" + integrity sha1-lVOxIbD6wohp2p7UWeIMdUN4hGE= + is-number@^2.1.0: version "2.1.0" resolved "https://registry.yarnpkg.com/is-number/-/is-number-2.1.0.tgz#01fcbbb393463a548f2f466cce16dece49db908f" @@ -7658,7 +7728,7 @@ is-redirect@^1.0.0: resolved "https://registry.yarnpkg.com/is-redirect/-/is-redirect-1.0.0.tgz#1d03dded53bd8db0f30c26e4f95d36fc7c87dc24" integrity sha1-HQPd7VO9jbDzDCbk+V02/HyH3CQ= -is-regex@^1.1.0: +is-regex@^1.1.0, is-regex@^1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/is-regex/-/is-regex-1.1.1.tgz#c6f98aacc546f6cec5468a07b7b153ab564a57b9" integrity sha512-1+QkEcxiLlB7VEyFtyBg94e08OAsvq7FUBgApTq/w2ymCLyKJgDPsybBENVtA7XCQEgEXxKPonG+mvYRxh/LIg== @@ -8113,6 +8183,14 @@ jsprim@^1.2.2: json-schema "0.2.3" verror "1.10.0" +jsx-ast-utils@^2.4.1: + version "2.4.1" + resolved "https://registry.yarnpkg.com/jsx-ast-utils/-/jsx-ast-utils-2.4.1.tgz#1114a4c1209481db06c690c2b4f488cc665f657e" + integrity sha512-z1xSldJ6imESSzOjd3NNkieVJKRlKYSOtMG8SFyCj2FIrvSaSuli/WjpBkEzCBoR9bYYYFgqJw61Xhu7Lcgk+w== + dependencies: + array-includes "^3.1.1" + object.assign "^4.1.0" + just-extend@^4.0.2: version "4.1.0" resolved "https://registry.yarnpkg.com/just-extend/-/just-extend-4.1.0.tgz#7278a4027d889601640ee0ce0e5a00b992467da4" @@ -9606,7 +9684,7 @@ object-copy@^0.1.0: define-property "^0.2.5" kind-of "^3.0.3" -object-inspect@^1.7.0: +object-inspect@^1.7.0, object-inspect@^1.8.0: version "1.8.0" resolved "https://registry.yarnpkg.com/object-inspect/-/object-inspect-1.8.0.tgz#df807e5ecf53a609cc6bfe93eac3cc7be5b3a9d0" integrity sha512-jLdtEOB112fORuypAyl/50VRVIBIdVQOSUUGQHzJ4xBSbit81zRarz7GThkEFZy1RceYrWYcPcBFPQwHyAc1gA== @@ -9638,6 +9716,25 @@ object.assign@4.1.0, object.assign@^4.1.0: has-symbols "^1.0.0" object-keys "^1.0.11" +object.entries@^1.1.2: + version "1.1.2" + resolved "https://registry.yarnpkg.com/object.entries/-/object.entries-1.1.2.tgz#bc73f00acb6b6bb16c203434b10f9a7e797d3add" + integrity sha512-BQdB9qKmb/HyNdMNWVr7O3+z5MUIx3aiegEIJqjMBbBf0YT9RRxTJSim4mzFqtyr7PDAHigq0N9dO0m0tRakQA== + dependencies: + define-properties "^1.1.3" + es-abstract "^1.17.5" + has "^1.0.3" + +object.fromentries@^2.0.2: + version "2.0.2" + resolved "https://registry.yarnpkg.com/object.fromentries/-/object.fromentries-2.0.2.tgz#4a09c9b9bb3843dd0f89acdb517a794d4f355ac9" + integrity sha512-r3ZiBH7MQppDJVLx6fhD618GKNG40CZYH9wgwdhKxBDDbQgjeWGGd4AtkZad84d291YxvWe7bJGuE65Anh0dxQ== + dependencies: + define-properties "^1.1.3" + es-abstract "^1.17.0-next.1" + function-bind "^1.1.1" + has "^1.0.3" + object.getownpropertydescriptors@^2.0.3: version "2.1.0" resolved "https://registry.yarnpkg.com/object.getownpropertydescriptors/-/object.getownpropertydescriptors-2.1.0.tgz#369bf1f9592d8ab89d712dced5cb81c7c5352649" @@ -11255,6 +11352,14 @@ regex-not@^1.0.0, regex-not@^1.0.2: extend-shallow "^3.0.2" safe-regex "^1.1.0" +regexp.prototype.flags@^1.3.0: + version "1.3.0" + resolved "https://registry.yarnpkg.com/regexp.prototype.flags/-/regexp.prototype.flags-1.3.0.tgz#7aba89b3c13a64509dabcf3ca8d9fbb9bdf5cb75" + integrity sha512-2+Q0C5g951OlYlJz6yu5/M33IcsESLlLfsyIaLJaG4FA2r4yP8MvVMJUUP/fVBkSpbbbZlS5gynbEWLipiiXiQ== + dependencies: + define-properties "^1.1.3" + es-abstract "^1.17.0-next.1" + regexpp@^2.0.1: version "2.0.1" resolved "https://registry.yarnpkg.com/regexpp/-/regexpp-2.0.1.tgz#8d19d31cf632482b589049f8281f93dbcba4d07f" @@ -11863,6 +11968,14 @@ showdown@^1.9.1: dependencies: yargs "^14.2" +side-channel@^1.0.2: + version "1.0.3" + resolved "https://registry.yarnpkg.com/side-channel/-/side-channel-1.0.3.tgz#cdc46b057550bbab63706210838df5d4c19519c3" + integrity sha512-A6+ByhlLkksFoUepsGxfj5x1gTSrs+OydsRptUxeNCabQpCFUvcwIczgOigI8vhY/OJCnPnyE9rGiwgvr9cS1g== + dependencies: + es-abstract "^1.18.0-next.0" + object-inspect "^1.8.0" + signal-exit@^3.0.0, signal-exit@^3.0.2: version "3.0.3" resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.3.tgz#a1410c2edd8f077b08b4e253c8eacfcaf057461c" @@ -12293,6 +12406,18 @@ string-width@^4.1.0, string-width@^4.2.0: is-fullwidth-code-point "^3.0.0" strip-ansi "^6.0.0" +string.prototype.matchall@^4.0.2: + version "4.0.2" + resolved "https://registry.yarnpkg.com/string.prototype.matchall/-/string.prototype.matchall-4.0.2.tgz#48bb510326fb9fdeb6a33ceaa81a6ea04ef7648e" + integrity sha512-N/jp6O5fMf9os0JU3E72Qhf590RSRZU/ungsL/qJUYVTNv7hTG0P/dbPjxINVN9jpscu3nzYwKESU3P3RY5tOg== + dependencies: + define-properties "^1.1.3" + es-abstract "^1.17.0" + has-symbols "^1.0.1" + internal-slot "^1.0.2" + regexp.prototype.flags "^1.3.0" + side-channel "^1.0.2" + string.prototype.trimend@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/string.prototype.trimend/-/string.prototype.trimend-1.0.1.tgz#85812a6b847ac002270f5808146064c995fb6913" From 9cf33e3b35303f77f42d37388cfe34d0c58d3b4f Mon Sep 17 00:00:00 2001 From: Luke Wood Date: Fri, 14 Aug 2020 11:23:41 -0700 Subject: [PATCH 2/2] Applies DOMSanitize.sanitize to innerHTML write This commit sanitizes the text applied to innerHTML of the linkNode attribute on OpenUriCommandHandler. This is an attempt to limit the xss sink surface in Theia. If there is no reason to do so or this breaks some intended behavior please leave a comment indicating so. Change-Id: Ibee9829b3a44f87d15c9d46b9bc342332e5ccf08 Signed-off-by: Luke Wood --- packages/plugin-ext/package.json | 2 ++ packages/plugin-ext/src/main/browser/commands.ts | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/plugin-ext/package.json b/packages/plugin-ext/package.json index d51507c124e79..6df81932f4f40 100644 --- a/packages/plugin-ext/package.json +++ b/packages/plugin-ext/package.json @@ -25,10 +25,12 @@ "@theia/timeline": "^1.6.0", "@theia/workspace": "^1.6.0", "@types/connect": "^3.4.32", + "@types/dompurify": "^2.0.2", "@types/mime": "^2.0.1", "@types/serve-static": "^1.13.3", "connect": "^3.7.0", "decompress": "^4.2.1", + "dompurify": "^2.0.11", "escape-html": "^1.0.3", "filenamify": "^4.1.0", "jsonc-parser": "^2.2.0", diff --git a/packages/plugin-ext/src/main/browser/commands.ts b/packages/plugin-ext/src/main/browser/commands.ts index 84b76ad973bcf..8d171b402608d 100644 --- a/packages/plugin-ext/src/main/browser/commands.ts +++ b/packages/plugin-ext/src/main/browser/commands.ts @@ -19,6 +19,7 @@ import URI from '@theia/core/lib/common/uri'; import { Command, CommandService } from '@theia/core/lib/common/command'; import { AbstractDialog } from '@theia/core/lib/browser'; import { WindowService } from '@theia/core/lib/browser/window/window-service'; +import * as DOMPurify from 'dompurify'; @injectable() export class OpenUriCommandHandler { @@ -90,7 +91,7 @@ class OpenNewTabDialog extends AbstractDialog { showOpenNewTabDialog(uri: string): void { this.value = uri; - this.linkNode.innerHTML = uri; + this.linkNode.innerHTML = DOMPurify.sanitize(uri); this.linkNode.href = uri; this.openButton.onclick = () => { this.windowService.openNewWindow(uri);