Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify with documentation and tests where our limitations are #161

Open
mozfreddyb opened this issue Apr 9, 2021 · 0 comments
Open

Clarify with documentation and tests where our limitations are #161

mozfreddyb opened this issue Apr 9, 2021 · 0 comments

Comments

@mozfreddyb
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant