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 👍
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.
Not that they actually had a huge problem right now, just that using the old
pattern HAS lead to bad performance in the past. See
https://github.com/github/codeql/pull/4361
This improves runtime for command injection query on
https://lgtm.com/projects/g/alibaba/funcraft from +200 seconds (I did not care
to wait more) down to ~55 seconds on my machine.
This type of tracking predicate with string as additional argument apparently
causes trouble :|
It "kinda" works now, but it really is not a pretty solution. Adding all these
"tracked" objects is SUPER annoying... it _would_ be possible to skip them, but
that seems like it will give the wrong edges for dataflow/taintflow queries :|
A good chunk of it should be able to be removed with access-paths like C# does
for library modeling. Some of it could be solved by better type-tracking API
like API Graphs... but it seems like we generally are just lacking the
nice-to-have features like `.getAMemberCall` and the like. See
https://github.com/github/codeql/pull/4082/files#diff-9aa94c4d713ef9d8da73918ff53db774L33