From 2599d2a3e7e63cc513700a0b6299bc7883108012 Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Thu, 9 Sep 2021 14:45:14 -0500 Subject: [PATCH 1/3] Add no innerHTML rule --- docs/rules/no-innter-html.md | 19 +++++++++++++++++++ lib/configs/browser.js | 1 + lib/configs/typescript.js | 2 +- lib/index.js | 1 + lib/rules/no-inner-html.js | 33 +++++++++++++++++++++++++++++++++ tests/no-inner-html.js | 34 ++++++++++++++++++++++++++++++++++ 6 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 docs/rules/no-innter-html.md create mode 100644 lib/rules/no-inner-html.js create mode 100644 tests/no-inner-html.js diff --git a/docs/rules/no-innter-html.md b/docs/rules/no-innter-html.md new file mode 100644 index 00000000..035c960e --- /dev/null +++ b/docs/rules/no-innter-html.md @@ -0,0 +1,19 @@ +# No `innerHTML` + +Using `innerHTML` poses a potential security risk. It should only be used when clearing content. + +```js +// bad +function setContent(element, content) { + element.innerHTML = content +} + +// good +function clearContent(element) { + element.innerHTML = '' +} +``` + +## See Also + +https://github.com/github/paste-markdown/security/advisories/GHSA-gpfj-4j6g-c4w9 diff --git a/lib/configs/browser.js b/lib/configs/browser.js index acebee6d..791e92be 100644 --- a/lib/configs/browser.js +++ b/lib/configs/browser.js @@ -10,6 +10,7 @@ module.exports = { 'github/no-blur': 'error', 'github/no-dataset': 'error', 'github/no-innerText': 'error', + 'github/no-inner-html': 'error', 'github/unescaped-html-literal': 'error', 'github/no-useless-passive': 'error', 'github/require-passive-events': 'error', diff --git a/lib/configs/typescript.js b/lib/configs/typescript.js index 6e82ce7e..47bb1044 100644 --- a/lib/configs/typescript.js +++ b/lib/configs/typescript.js @@ -6,7 +6,7 @@ module.exports = { camelcase: 'off', 'no-unused-vars': 'off', 'no-shadow': 'off', - 'no-invalid-this': "off", + 'no-invalid-this': 'off', '@typescript-eslint/no-invalid-this': ['error'], '@typescript-eslint/no-shadow': ['error'], '@typescript-eslint/interface-name-prefix': 'off', diff --git a/lib/index.js b/lib/index.js index 63f9847b..c98a8121 100644 --- a/lib/index.js +++ b/lib/index.js @@ -11,6 +11,7 @@ module.exports = { 'no-dataset': require('./rules/no-dataset'), 'no-implicit-buggy-globals': require('./rules/no-implicit-buggy-globals'), 'no-innerText': require('./rules/no-innerText'), + 'no-inner-html': require('./rules/no-inner-html'), 'no-then': require('./rules/no-then'), 'unescaped-html-literal': require('./rules/unescaped-html-literal'), 'no-useless-passive': require('./rules/no-useless-passive'), diff --git a/lib/rules/no-inner-html.js b/lib/rules/no-inner-html.js new file mode 100644 index 00000000..5cac1e28 --- /dev/null +++ b/lib/rules/no-inner-html.js @@ -0,0 +1,33 @@ +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'disallow `Element.prototype.innerHTML``', + url: require('../url')(module) + }, + schema: [] + }, + + create(context) { + return { + AssignmentExpression(node) { + if (node.operator === "=") { + const leftNode = node.left; + const rightNode = node.right; + + if (leftNode.property && leftNode.property.name === 'innerHTML') { + if (rightNode.type === 'Literal' && rightNode.value === '') { + return + } + + context.report({ + node: node, + message: + 'Using innerHTML poses a potential security risk and should not be used other than cleaning content.' + }) + } + } + } + } + } +} diff --git a/tests/no-inner-html.js b/tests/no-inner-html.js new file mode 100644 index 00000000..7f32a19d --- /dev/null +++ b/tests/no-inner-html.js @@ -0,0 +1,34 @@ +const rule = require('../lib/rules/no-inner-html') +const RuleTester = require('eslint').RuleTester + +const ruleTester = new RuleTester() + +ruleTester.run('no-innter-html', rule, { + valid: [ + { + code: 'document.createElement("js-flash-text").innerHTML = ""' + } + ], + invalid: [ + { + code: 'document.createElement("js-flash-text").innerHTML = "foo"', + errors: [ + { + message: + 'Using innerHTML poses a potential security risk and should not be used other than cleaning content.', + type: 'AssignmentExpression' + } + ] + }, + { + code: 'document.querySelector("js-flash-text").innerHTML = "
code
"', + errors: [ + { + message: + 'Using innerHTML poses a potential security risk and should not be used other than cleaning content.', + type: 'AssignmentExpression' + } + ] + } + ] +}) From 4ba1cbad61a11c8d84091b8e29de56594ff3e399 Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Thu, 9 Sep 2021 14:46:58 -0500 Subject: [PATCH 2/3] lint --- lib/rules/no-inner-html.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/rules/no-inner-html.js b/lib/rules/no-inner-html.js index 5cac1e28..a13e190a 100644 --- a/lib/rules/no-inner-html.js +++ b/lib/rules/no-inner-html.js @@ -11,9 +11,9 @@ module.exports = { create(context) { return { AssignmentExpression(node) { - if (node.operator === "=") { - const leftNode = node.left; - const rightNode = node.right; + if (node.operator === '=') { + const leftNode = node.left + const rightNode = node.right if (leftNode.property && leftNode.property.name === 'innerHTML') { if (rightNode.type === 'Literal' && rightNode.value === '') { @@ -21,7 +21,7 @@ module.exports = { } context.report({ - node: node, + node, message: 'Using innerHTML poses a potential security risk and should not be used other than cleaning content.' }) From 76900cfc39054d6c740dc8b2a07202f6ef5de538 Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Thu, 9 Sep 2021 16:24:00 -0500 Subject: [PATCH 3/3] clearing instead of cleaning --- lib/rules/no-inner-html.js | 2 +- tests/no-inner-html.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/no-inner-html.js b/lib/rules/no-inner-html.js index a13e190a..3845731a 100644 --- a/lib/rules/no-inner-html.js +++ b/lib/rules/no-inner-html.js @@ -23,7 +23,7 @@ module.exports = { context.report({ node, message: - 'Using innerHTML poses a potential security risk and should not be used other than cleaning content.' + 'Using innerHTML poses a potential security risk and should not be used other than clearing content.' }) } } diff --git a/tests/no-inner-html.js b/tests/no-inner-html.js index 7f32a19d..386b1785 100644 --- a/tests/no-inner-html.js +++ b/tests/no-inner-html.js @@ -15,7 +15,7 @@ ruleTester.run('no-innter-html', rule, { errors: [ { message: - 'Using innerHTML poses a potential security risk and should not be used other than cleaning content.', + 'Using innerHTML poses a potential security risk and should not be used other than clearing content.', type: 'AssignmentExpression' } ] @@ -25,7 +25,7 @@ ruleTester.run('no-innter-html', rule, { errors: [ { message: - 'Using innerHTML poses a potential security risk and should not be used other than cleaning content.', + 'Using innerHTML poses a potential security risk and should not be used other than clearing content.', type: 'AssignmentExpression' } ]