Authorize dynamic function calls

[glacambre]

This commit fixes an error that would be thrown when analyzing statements of the form x.innerHTML = y()(). The error was thrown from normalizeMethodCall, which didn't expect to receive a CallExpression.

When this happened, normalizeMethodCall was called by getCodeName, which was itself called by isAllowedCallExpression.

This is fixed by making isAllowedCallExpression return true if the callee is a CallExpression and using the previous logic if it isn't.

Closes #138.

[glacambre]

Instead, can you please add a case for CallExpression in normalizeMethodCall and return an empty methodName, like we do for ArrowFunctionExpressions?

This was my first approach but it doesn't work as isAllowedCallExpression uses funcName && … on the returned name and empty strings are falsy. The reason I didn't change the if-expression is that I didn't know if it would break assumptions made in other places or not. Let me know if you'd still prefer to move the check to normalizeMethodCall :).

[mozfreddyb]

First of all, thank you for the quick response. I'm afraid, I have been misreading the code you want to allow. I don't think we can allow foo.innerHTML = y()() for the same reason we can't allow foo.innerHTML = something.

We only want to allow stuff that is either obviously safe (e.g., foo.innerHTML = "some static string") or using a allowed escaper function (e.g., foo.innerHTML = DOMPurify.purify(whatever);

[glacambre]

Sorry, the fault is mine, I should have guessed that you preferred false positives to false negatives :). I'll push an updated version in ~6 hours.

[mozfreddyb]

Err, to clarify: I don't think we can find a way to allow foo.innerHTML = y()() at all Not sure how you intend to follow-up, but take your time. I'm here to help :)

[glacambre]

I don't think we can find a way to allow foo.innerHTML = y()() at all

Well it really depends on the philosophy of the tool. Either you want zero false positives and you're ok with getting false negatives or you want zero false negatives and you're ok with getting false positives :).

[glacambre]

@mozfreddyb In case you missed it, I implemented the change you requested :)

[glacambre]

This PR has been open for a year and now has conflicts, so let's close it. @mozfreddyb feel free to salvage anything you want from this PR if you think it'd still be useful :)

[mozfreddyb]

I think is #177 now. Closing. Thanks @glacambre!