mozilla / eslint-plugin-no-unsanitized

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

Clarify with documentation and tests where our limitations are #161

Open mozfreddyb opened 3 years ago

mozfreddyb commented 3 years ago

Some limitations might be obvious, but we should state them nevertheless.

As a follow-up from #157, but also in relation to https://bugzilla.mozilla.org/show_bug.cgi?id=1701169, I think this project needs a good explanation of where we stop looking for what's being called when looking at a CallExpression.

Especially in contrast to looking at function parameters andassigment epxressions that we analyze to find safe/unsafe values when they flow into a sink.

Example 1:

const evil = '<img src=x onerror=alert("hi");>'
function getwrite() { return document.write.bind(document) };
getwrite()(evil);

E.g., this can't and won't be detected as we are unable to analyze what getwrite is and what it returns and that's probably OK. We know that we can't ensure getwrite() isn't returning something potentially dangerous, but we shouldn't block it. We might however throw a low-severity finding that can be disabled? In the end, we need to acknowledge that we can't protect against malicious code authors and allow folks to customize checked/disallowed functions. For a codebase that uses wrappers on top of typical DOM APIs, it's reasonable to add these wrappers to the plugin configuration.

Example 2:

function lol() {
  return (() => '<img src=x onerror=alert("hi");>');
}
foo.innerHTML = lol()();

This should be disallowed. We can't really say what's being called here, but we do know it isn't a static literal or directly sanitized.