Just like our support for the `PyYAML` PyPI package that you import with
`import yaml` is in `Yaml.qll`.
Since this file does not provide any public predicates/modules, it
should be safe to rename it.
So it better matches what is in `py/code-injection`. I had my doubts
about CWE-95, but after reading
https://owasp.org/www-community/attacks/Direct_Dynamic_Code_Evaluation_Eval%20Injection
I think it's fine to add CWE-95 as well 👍
Definitions are:
CWE-78: Improper Neutralization of Special Elements used in an OS
Command ('OS Command Injection')
CWE-94: Improper Control of Generation of Code ('Code Injection')
CWE-95: Improper Neutralization of Directives in Dynamically Evaluated
Code ('Eval Injection')
This was slightly messier than anticipated, as I hadn't accounted for
the dozen uses of `startInAttr` in our codebase. To circumvent this,
I decided to put the type tracking implementation in the `internal`
directory, and wrap it with a file that ensures the old interface still
works.
Now there is a path from the _imports_ of the functions that would
return sensitive data, so we produce more alerts.
I'm not entirely happy about this "double reporting", but I'm not sure
how to get around it without either:
1. disabling the extra taint-step for calls. Not ideal since we would
loose good sources.
2. disabling the extra sources based on function name. Not ideal since
we would loose good sources.
3. disabling the extra sources based on function name, for those calls
that would be handled with the extra taint-step for calls. Not ideal
since that would require running the data-flow query initially to
prune these out :|
So for now, I think the best approach is to accept some risk on this,
and ship to learn :)
This solution was the best I could come up with, but it _is_ a bit
brittle since you need to remember to add this additional taint step
to any configuration that relies on sensitive data sources... I don't
see an easy way around this though :|
The comment about imports was placed wrong. I also realized we didn't
even have a single test-case for
`this.(DataFlow::AttrRead).getAttributeNameExpr() = sensitiveLookupStringConst(classification)`
so I added that (notice that this is only `getattr(foo, x)` and not
`getattr(foo, "password")`)
