In these cases the `return` might end up creating a new HTTP response, so they
need to be modeled as such.
Initially I created a very naive solution that didn't handle either
tricky_return1 or tricky_return2.
The interaction in tricky_return2/helper highlighted for me that to handle this
properly, due to the fact that the flow is across functions, we either need to
use a global dataflow/taint-tracking configuration, or some clever use of
type-trackers.
In the end, this extra effort for not modeling all returns in a flask route
handler as a creation of a HTTP response doesn't really seem to be worth it (at
least not right now). Sicne we use it with taint-tracking for the Reflected XSS
query, and use a HTTP response _creation_ as the sink (without propagating taint
to the HTTP response), we won't get into trouble where we report a path to BOTH
`make_response(...)` and the `return`
```
resp = make_response(...)
return resp
```
If we change this setup in the future, we will probably need to do something to
avoid this double-path reporting.
When dealing with
```
resp = make_response(...)
return resp
```
ideally we don't want to mark the return as a creation of a HTTP response. I'll
deal with this in a second commit, to show off how annoying it looks in the
tests right now :D
I'm not even trying to model it properly right now, and don't have a specific
use-case for it RIGHT NOW. I think we could want this in the future, but I think
it's probably better to model it when we know what we want to use it for.
We might need to rework this a bit when we also start to handle redirects. I
could see a world where we simply allow http redirects to be subclasses of http
responses, and need to manually exclude them from queries (or create
HttpContentResponse to model the HttpResponses that will contain a body). Let us
see where the wind will take us.
I looked through JS and Go libraries, but I didn't feel their modeling would map
very well to Python.
This improves the performance of the printAst.ql query by excluding a lot of string concatenations that happen in files unrelated to the one the user is interested in printing.
This is supposed to help the performance of the AST Viewer on bigger databases.
Two interesting things happened while doing this:
1. I found out that you can't use the same name to define a submodule as any
parent module. So we need give unique names to the top-level module, and the
module for modeling the `flask.Flask` class. I randomly choose a new name for
the top-level module to get things moving (and not be stuck in bikeshedding
forever).
2. With this new setup, I wanted to expose the `route` and `add_url_rule`
methods on instances of `flask.Flask`. It wasn't quite obvious how to do so. I
simply lumped them next to `classRef()` and `instance()`, without too much
care. I did consider putting them inside a `instance` module, which would allow
you to access them by `flask::Flask::instance::route()`, but I wasn't quite
sure, and just did something easy to get moving.
Clause timing report had this suspicious entry
```
CommandInjection.ql-12:DataFlowPublic::Node::getCallableScope#bbf .................. 7.2s
(4 evaluations with max 6.4s in DataFlowPublic::Node::getCallableScope#bbf/3@i3#119d7b)
```
which indeed was a bad join:
```
Tuple counts for DataFlowPublic::Node::getCallableScope#bbf:
293509 ~2% {3} r1 = JOIN DataFlowPublic::Node::getCallableScope#bbf#prev_delta AS L WITH DataFlowPublic::TNode#f AS R ON FIRST 1 OUTPUT L.<1>, L.<0>, L.<2>
22337162 ~0% {3} r2 = JOIN r1 WITH Scope::Scope::getEnclosingScope_dispred#ff_10#join_rhs AS R ON FIRST 1 OUTPUT r1.<1>, r1.<2>, R.<1>
22337162 ~0% {3} r3 = r2 AND NOT DataFlowPublic::Node::getCallableScope#bbf#prev AS R(r2.<0>, r2.<2>, r2.<1>)
22337162 ~0% {3} r4 = SCAN r3 OUTPUT r3.<0>, r3.<2>, r3.<1>
722 ~1% {3} r5 = JOIN r4 WITH m#DataFlowPublic::Node::getCallableScope#bbf AS R ON FIRST 2 OUTPUT r4.<0>, r4.<1>, r4.<2>
722 ~1% {3} r6 = JOIN r5 WITH m#DataFlowPublic::Node::getCallableScope#bbf AS R ON FIRST 2 OUTPUT r5.<0>, r5.<2>, r5.<1>
722 ~1% {3} r7 = r6 AND NOT project#DataFlowPrivate::DataFlowCallable::getScope_dispred#ff AS R(r6.<2>)
722 ~1% {3} r8 = SCAN r7 OUTPUT r7.<0>, r7.<2>, r7.<1>
return r8
```
In this case, the join went away by simply moving the helper predicate
out of the class it was situated in (and since it doesn't mention
`this`, it didn't really belong there in the first place).
Result:
```
DataFlowPublic.qll-8:DataFlowPublic::getCallableScope#ff ........................... 26ms
(4 evaluations with max 15ms in DataFlowPublic::getCallableScope#ff/2@i3#709a9e)
```
