diff --git a/README.md b/README.md index 44eb00c..6bb3f5b 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ We also implemented several [custom rules](./lib/rules) where we did not find su | [@microsoft/sdl/no-postmessage-star-origin](./docs/rules/no-postmessage-star-origin.md) | Always provide specific target origin, not \* when sending data to other windows using [`postMessage`](https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage#Security_concerns) to avoid data leakage outside of trust boundary. | | [@microsoft/sdl/no-unsafe-alloc](./docs/rules/no-unsafe-alloc.md) | When calling [`Buffer.allocUnsafe`](https://nodejs.org/api/buffer.html#buffer_static_method_buffer_allocunsafe_size) and [`Buffer.allocUnsafeSlow`](https://nodejs.org/api/buffer.html#buffer_static_method_buffer_allocunsafeslow_size), the allocated memory is not wiped-out and can contain old, potentially sensitive data. | | [@microsoft/sdl/no-winjs-html-unsafe](./docs/rules/no-winjs-html-unsafe.md) | Calls to [`WinJS.Utilities.setInnerHTMLUnsafe()`]() and similar methods do not perform any input validation and should be avoided. Use [`WinJS.Utilities.setInnerHTML()`]() instead. | -| [@microsoft/sdl/react-iframe-missing-sandbox](./docs/rules/react-iframe-missing-sandbox.md) | The [sandbox](https://www.w3schools.com/tags/att_iframe_sandbox.asp) attribute enables an extra set of restrictions for the content in the iframe and should always be specified. | +| [react/iframe-missing-sandbox](https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/iframe-missing-sandbox.md) | The [sandbox](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#sandbox) attribute enables an extra set of restrictions for the content in the iframe and should always be specified. | | [react/no-danger](https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-danger.md) | Bans usage of `dangerouslySetInnerHTML` property in React as it allows passing unsanitized HTML in DOM. | | [@typescript-eslint/no-implied-eval](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-implied-eval.md) | Similar to built-in ESLint rule `no-implied-eval`. Bans usage of `setTimeout()`, `setInterval()`, `setImmediate()`, `execScript()` or `new Function()` as they are similar to `eval()` and allow code execution from string arguments. | diff --git a/config/react.js b/config/react.js index 6260cb1..323d857 100644 --- a/config/react.js +++ b/config/react.js @@ -29,15 +29,13 @@ module.exports = (pluginSdl) => { enforceDynamicLinks: "always", warnOnSpreadAttributes: true } - ] + ], + "react/react-iframe-missing-sandbox": "error" } }, { plugins: { "@microsoft/sdl": pluginSdl - }, - rules: { - "@microsoft/sdl/react-iframe-missing-sandbox": "error" } } ]; diff --git a/docs/rules/react-iframe-missing-sandbox.md b/docs/rules/react-iframe-missing-sandbox.md deleted file mode 100644 index 9be6f53..0000000 --- a/docs/rules/react-iframe-missing-sandbox.md +++ /dev/null @@ -1,17 +0,0 @@ -# An iframe element is missing a sandbox attribute (react-iframe-missing-sandbox) - -The [sandbox](https://www.w3schools.com/tags/att_iframe_sandbox.asp) attribute enables an extra set of restrictions for the content in the iframe and should always be specified. - -Additional functionality such as the ability to run scripts should be enabled only in justifiable cases after thorough security review. - -- [Rule Source](../../lib/rules/react-iframe-missing-sandbox.js) -- [Rule Test](../../tests/lib/rules/react-iframe-missing-sandbox.js) - -## Related Rules - -- [tslint-microsoft-contrib/react-iframe-missing-sandbox](https://github.com/microsoft/tslint-microsoft-contrib/blob/master/src/reactIframeMissingSandboxRule.ts) - -## More Reading - -- [How to safely inject HTML in React using an iframe](https://medium.com/the-thinkmill/how-to-safely-inject-html-in-react-using-an-iframe-adc775d458bc) -- [Play safely in sandboxed IFrames](https://www.html5rocks.com/en/tutorials/security/sandboxed-iframes/) diff --git a/lib/index.js b/lib/index.js index d918dd5..07a16bc 100644 --- a/lib/index.js +++ b/lib/index.js @@ -27,8 +27,7 @@ const plugin = { "no-msapp-exec-unsafe": require("./rules/no-msapp-exec-unsafe"), "no-postmessage-star-origin": require("./rules/no-postmessage-star-origin"), "no-unsafe-alloc": require("./rules/no-unsafe-alloc"), - "no-winjs-html-unsafe": require("./rules/no-winjs-html-unsafe"), - "react-iframe-missing-sandbox": require("./rules/react-iframe-missing-sandbox") + "no-winjs-html-unsafe": require("./rules/no-winjs-html-unsafe") }, // Filled in later in order to reference plugin itself. configs: {} diff --git a/lib/rules/react-iframe-missing-sandbox.js b/lib/rules/react-iframe-missing-sandbox.js deleted file mode 100644 index fe33095..0000000 --- a/lib/rules/react-iframe-missing-sandbox.js +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -/** - * @fileoverview Rule to enforce sandbox attribute on iframe elements - */ - -"use strict"; - -// TODO: Follow-up on https://github.com/yannickcr/eslint-plugin-react/issues/2754 and try to merge rule into eslint-plugin-react - -module.exports = { - meta: { - type: "suggestion", - fixable: "code", - schema: [], - docs: { - category: "Security", - description: - "The [sandbox](https://www.w3schools.com/tags/att_iframe_sandbox.asp) attribute enables an extra set of restrictions for the content in the iframe and should always be specified.", - url: "https://github.com/microsoft/eslint-plugin-sdl/blob/master/docs/rules/react-iframe-missing-sandbox.md" - }, - messages: { - attributeMissing: "An iframe element is missing a sandbox attribute", - invalidValue: - 'An iframe element defines a sandbox attribute with invalid value "{{ value }}"', - invalidCombination: - "An iframe element defines a sandbox attribute with both allow-scripts and allow-same-origin which is invalid" - } - }, - - create(context) { - const ALLOWED_VALUES = [ - // From https://www.w3schools.com/tags/att_iframe_sandbox.asp - "", - "allow-forms", - "allow-modals", - "allow-orientation-lock", - "allow-pointer-lock", - "allow-popups", - "allow-popups-to-escape-sandbox", - "allow-presentation", - "allow-same-origin", - "allow-scripts", - "allow-top-navigation", - "allow-top-navigation-by-user-activation" - ]; - - function validateSandboxAttribute(node, attribute) { - const values = attribute.value.value.split(" "); - let allowScripts = false; - let allowSameOrigin = false; - values.forEach((attributeValue) => { - const trimmedAttributeValue = attributeValue.trim(); - if (ALLOWED_VALUES.indexOf(trimmedAttributeValue) === -1) { - context.report({ - node, - messageId: "invalidValue", - data: { - value: trimmedAttributeValue - } - }); - } - if (trimmedAttributeValue === "allow-scripts") { - allowScripts = true; - } - if (trimmedAttributeValue === "allow-same-origin") { - allowSameOrigin = true; - } - }); - if (allowScripts && allowSameOrigin) { - context.report({ - node, - messageId: "invalidCombination" - }); - } - } - - return { - 'JSXOpeningElement[name.name="iframe"]'(node) { - let sandboxAttributeFound = false; - node.attributes.forEach((attribute) => { - if ( - attribute.type === "JSXAttribute" && - attribute.name && - attribute.name.type === "JSXIdentifier" && - attribute.name.name === "sandbox" - ) { - sandboxAttributeFound = true; - if (attribute.value && attribute.value.type === "Literal" && attribute.value.value) { - // Only string literals are supported for now - validateSandboxAttribute(node, attribute); - } - } - }); - if (!sandboxAttributeFound) { - context.report({ - node, - messageId: "attributeMissing" - }); - } - } - }; - } -}; diff --git a/tests/lib/rules/react-iframe-missing-sandbox.js b/tests/lib/rules/react-iframe-missing-sandbox.js deleted file mode 100644 index 1844682..0000000 --- a/tests/lib/rules/react-iframe-missing-sandbox.js +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -"use strict"; - -const path = require("path"); -const ruleId = path.parse(__filename).name; -const rule = require(path.join("../../../lib/rules/", ruleId)); -const RuleTester = require("eslint").RuleTester; - -var ruleTester = new RuleTester({ - languageOptions: { - parserOptions: { - ecmaVersion: 2018, - sourceType: "module", - ecmaFeatures: { - jsx: true - } - } - } -}); - -ruleTester.run(ruleId, rule, { - valid: [ - { code: '
;' }, - { code: '' }, - { code: '' }, - { code: '' }, - { code: '' }, - { code: '' }, - { code: '' }, - { code: '' }, - { code: '' }, - { code: '' }, - { code: '' }, - { code: '' }, - { code: '' }, - { - code: '' - }, - { code: '' }, - { - code: '' - } - ], - invalid: [ - { - code: ";", - errors: [{ messageId: "attributeMissing" }] - }, - { - code: "', - errors: [{ messageId: "invalidValue", data: { value: "__unknown__" } }] - }, - { - code: ';', - errors: [{ messageId: "invalidCombination" }] - }, - { - code: '