mozilla / eslint-plugin-no-unsanitized

Custom ESLint rule to disallows unsafe innerHTML, outerHTML, insertAdjacentHTML and alike
Mozilla Public License 2.0
223 stars 34 forks source link

Allow ThisExpressions as allowed callables (fixes #71) #72

Closed mozfreddyb closed 6 years ago

mozfreddyb commented 6 years ago

Hey @jonathanKingston can you take a look? I think ThisExpressions pose no risk, but I'd like a second pair of eyes on this.

jonathanKingston commented 6 years ago

I don't see any obvious concerns, can we ensure that the following fails:

(() => {this.eval("alert(1)");})();

Especially as use of a top level arrow can be common:

<script>
document.body.addEventListener("click", () => {
  this.eval("alert(1)");
});
</script>

I know we took the stance to avoid obfuscation however I think this could be a risk, especially for the more generic cases that we allow customisation for.

The other concern is window callbacks:

window.addEventListener("focus", function () {
  this.eval("alert(1)");
});
mozfreddyb commented 6 years ago

We don't disallow eval, so assuming you're thinking of another bad function like insertAdjacentHTML. We do disallow this.insertAdjacentHTML() and this().insertAdjacentHTML.

mozfreddyb commented 6 years ago

Added some tests for the review comment :)

jonathanKingston commented 6 years ago

This is great, will merge when Travis is happy :).