This required a minor change in the type tracker implementation, but
apart from that no other changes appear to be needed. Seems to clean
up the test output quite a bit.
Since predicate name `import` is not allowed, I adopted `importNode` as it sort
of matches what `exprNode` does.
---
Due to only using `importMember` in `os_attr` we previously didn't handle
`import os.path as alias` :|
I did creat a hotfix for this (https://github.com/github/codeql/pull/4446), but
in doing so I realized the core of the problem: We're exposing ourselves to
making these kinds of mistakes by having BOTH importModule and importMember, and
we don't really gain anything from doing this!
We do loose the ability to easily only modeling `from mod import val` and not
`import mod.val`, but I don't think that will ever be relevant.
This change will also make us to recognize some invalid code, for example in
import os.system as runtime_error
we would now model that `runtime_error` is a reference to the `os.system`
function (although the actual import would result in a runtime error).
Overall these are tradeoffs I'm willing to make, as it does makes things simpler
from a QL modeling point of view, and THAT sounds nice 👍
This is not a very good test for showing that we don't handle direct imports,
but it was the best I had available without inventing something new. It's very
fragile, since any of these would propagate taint (due to handling all `join`
calls as if the qualifier was a string):
ospath_alias.join(ts)
ospath_alias.join(ts, "foo", "bar")
But this test DOES serve the purpose of illustrating that my fix works :D
Required a small change in `DataFlow::importModule` to get the desired
behaviour (cf. the type trackers defined in `moduleattr.ql`, but this
should be harmless. The node that is added doesn't have any flow
anywhere.
The extra hist in `test.py` seen in `globalStep.expected`
are due to the removal of manual filtering code.
(That code was from when dataflow had many strange things in it.)
Only have one type of callable, but have an extra type of call.
A constructor call directs to an init callable
(should also handle `call` overrides at some point).
Removes steps from `ModuleVariableNode`s from `essaFlowStep`, and
instead puts them only in `jumpStep`. This cleans up the logic a bit.
This slightly broke the type tracker implementation (as it relied on
`essaFlowStep` being fairly liberal), so I have rewritten it to
explicitly rely on just familiar predicates for local and jump steps.
Additionally, we disallow Essa-to-Essa steps where exactly one of the
two nodes corresponds to a global variable (i.e. only local-local and
global-global steps).
