This was an unwanted interaction between two unrelated tests, so I
switched to a different built-in in the second test. I also added a test
case that shows an unfortunate side effect of this more restricted
handling of built-ins.
This commit does a lot of stuff all at once, so here are the main
highlights:
In `TypeTracker.qll`, we change `StepSummary::step` to step only between
source nodes. Because reads and writes of global variables happen in two
different (jump) steps, this requires the intermediate
`ModuleVariableNode` to _also_ be a `LocalSourceNode`, and we therefore
modify the charpred for that class accordingly. (This also means
changing a few of the tests to account for these new source nodes.)
In addition, we change `TypeTracker::step` to likewise step between
local source nodes.
Next, to enable the use of the `track` convenience method on nodes, we
add some pragmas to `TypeTracker::step` that prevent bad joins from
occurring. With this, we can eliminate all of the manual type tracker
join predicates.
Next, we observe that because `StepSummary::step` now uses `flowsTo`, it
automatically encapsulates all local-flow steps. In particular this
means we do not have to use `typePreservingStep` in `smallstep`, but can
use `jumpStep` directly. A similar observation applies to
`TypeTracker::smallstep`.
Having done this, we no longer need `typePreservingStep`, so we get rid
of it.
The meat of this PR is described in the new python/ql/test/experimental/meta/InlineTaintTest.qll file:
> Defines a InlineExpectationsTest for checking whether any arguments in
> `ensure_tainted` and `ensure_not_tainted` calls are tainted.
>
> Also defines query predicates to ensure that:
> - if any arguments to `ensure_not_tainted` are tainted, their annotation is marked with `SPURIOUS`.
> - if any arguments to `ensure_tainted` are not tainted, their annotation is marked with `MISSING`.
>
> The functionality of this module is tested in `ql/test/experimental/meta/inline-taint-test-demo`.
Since it's exposed nicely in the version that doesn't have a
`DataFlow::TypeTracker` parameter, these should be private.
Also found one instance where I had accidentially used DataFlow::Node instead of
LocalSourceNode
Although it is becoming non-trivial to get an overview of what tests we have and
don't have, I didn't find any that highlighted this one
I used all 3 variants of parameters, just to be sure :)
One minor change to the tests results needed: there is no longer local
flow going into the `ModuleVariableNode` for `attr_ref` in the
`moduleattr.ql` test, but I think this is reasonable.
In general, if there is _some_ decorator on a function, it might not be safe to
track content out of it (since the decorator could do anything), but in this
case, we can see what the decorator does, so we should be able to handle it (but
we don't right now).
By my understanding of how type-tracking works, if we track content through
`my_decorator`, then we would also track content to the result of
`unrelated_func()`, which I wanted to make sure our tests would catch.
I found out the core of the problem seems to come from our lack of being able to
track to the inner scope, and added an explicit test for that.
This is slightly dubious, and should really be in the currently
unimplemented "def" counterpart to the "use" bits we already have.
However, it seems to work correctly, and in the spirit of moving
things along, this seemed like the easier solution. We can always
replace the implementation with the "proper" approach at a later point.
In lieu of removing the offending flow (which would likely have
consequences for a lot of other tests), I opted to simply _include_
the relevant nodes directly.
There is now a bit of redundancy in the tests, but I thought it useful
to actually include some of the cases called out explicitly in the
documentation, so as to make it easy to see that the code actually
does what we expect (in these cases, anyway).
This turned out to be fairly simple. Given an import such as
```python
from foo.bar.baz import quux
```
we create an API-graph node for each valid dotted prefix of
`foo.bar.baz`, i.e. `foo`, `foo.bar`, and `foo.bar.baz`. For these, we
then insert nodes in the API graph, such that `foo` steps to `foo.bar`
along an edge labeled `bar`, etc.
Finally, we only allow undotted names to hang off of the API-graph
root. Thus, `foo` will have a `moduleImport` edge off of the root, and
a `getMember` edge for `bar` (which in turn has a `getMember` edge for
`baz`).
Relative imports are explicitly ignored.
Finally, this commit also adds inline tests for a variety of ways of
importing modules, including a copy of the "import-helper" tests (with
a few modifications to allow a single annotation per line, as these
get rather long quickly!).
