JS: add RemoteFlowSource.isThirdPartyControllable()

[asger-semmle]

Adds isThirdPartyControllable() predicate to RemoteFlowSource for flow sources that can be controlled by the attacker during CSRF.

ReflectedXSS and ServerSideUrlRedirect have been updated to use this. This eliminates some false positives.

The CorsMisconfiguration query is a bit of a conundrum. It is certainly in the CSRF category, but is different from the others in that the tracks values that may refer to a foreign URL. Most importantly, this includes the Origin header, which is may refer to a malicious web site, but is not otherwise directly controlled by the attacker. I see three options for how to approach this:

1. Change CORS to only use third-party controllable flow sources, with the special addition of the Origin header, and maybe the Referer header.
2. Change isThirdPartyControllable() to include Origin and Referer headers. Technically you could have a reflected XSS through the referer header, and a server-side URL redirect to the referer could cause information leak if a secret session/auth token is appended in the URL.
3. Leave the CORS query as it is.

So far this PR just uses option 3, but I'd like to hear your opinion on this. I haven't seen any concrete results that could motivate one choice over another.

[xiemaisi]

I agree with the overall goal of this PR, but I'm wondering whether we should perhaps go about it slightly differently as suggested in the comments.

[xiemaisi]

I don't understand this explanation, particularly the bit beginning with "and in doing so". Why is it important what the web site controls? Aren't we more interested in what data the remote user/attacker controls?

[asger-semmle]

The web site is the attacker here. Would the following change clarify things?

A malicious web site can redirect the visitor's browser to any other domain [...]

[asger-semmle]

The goal of the paragraph is to explain why a user might sent a request with data he doesn't control, and why it's not okay to just blame for user for sending stupid requests that compromise his account.

[xiemaisi]

Yes, I think that would be clearer.

[xiemaisi]

This sounds too special-case to put it on a fairly general class like RemoteFlowSource. Is there a more general way to describe this scenario independent of the specifics of HTTP requests? Or should we perhaps only have this predicate on RequestInputAccess and specialise the sources of ReflectedXSS and ServerSideUrlRedirect to be request input accesses? (I don't think any of the other remote flow sources are relevant for those two queries, but it's of course worth checking.)

[asger-semmle]

Originally I wanted to describe it more generally as something like:

Holds if data from this source may be controlled by an actor other than the one who sent it.

However, now we have to decide how to classify browser-side sources like document.location. Who sent this data? By what definition is this even a "remote" flow source?

On one hand, one could argue that they were "sent" to the user by another web server from where the user was redirected (most CSRF attacks essentially start that way). From that viewpoint, they are indeed controlled by the sender of the data (the malicious web site), thus is not third-party controlled.

On the other hand, one could argue that any flow source of interest on the browser-side must come from a third party because the client trusts both himself and the server. Thus one would expect to use isThirdPartyControllable() in DomBasedXss and ClientSideUrlRedirect.

In any case, it was sufficiently non-obvious how to classify document.location, and that's why I opted for a definition that's less ambiguous.

[asger-semmle]

I like you suggestion on moving it into RequestInputAccess, as that's essentially the same as saying it must be an "incoming request" (albeit restricted to HTTP requests).

[asger-semmle]

I've moved it into RequestInputAccess and also added the Referer header as a source in ReflectedXss. The documentation has been rewritten.

[xiemaisi]

LGTM