codeql
codeql copied to clipboard
Java: Add support for data flow through thrown exceptions.
This adds support for exception flow.
A test is included that shows some of what works and what doesn't. Currently we get spurious flow due to imprecisions in type pruning. There's also some missing, since I've ignored MaD-synthesised method bodies for now.
Will this also fix https://github.com/github/codeql/issues/3710?
Will this also fix #3710?
Excellent question. I am indeed introducing dedicated data flow nodes corresponding to variable declarations in catch clauses along with (among others) the outgoing edges you ask for in the issue. However, I was not initially planning to expose those nodes, as I hadn't yet imagined use-cases. Do you think it is useful to be able to e.g. declare such a variable declaration as a source in the way that you do in the issue? With the changes in this PR I'd hoped that you didn't need to.
Will this also fix #3710?
Excellent question. I am indeed introducing dedicated data flow nodes corresponding to variable declarations in catch clauses along with (among others) the outgoing edges you ask for in the issue. However, I was not initially planning to expose those nodes, as I hadn't yet imagined use-cases. Do you think it is useful to be able to e.g. declare such a variable declaration as a source in the way that you do in the issue? With the changes in this PR I'd hoped that you didn't need to.
#3710 was originally created in response to the "Step 3: Errors and Exceptions" part of https://securitylab.github.com/ctf/codeql-and-chill/.
try {
parse(tainted);
} catch (Exception e) {
sink(e.getMessage())
}
The goal was to catch flow from parse
to sink
, but the code of parse
is unavailable.
Will something like this be modelable with this PR?
(I still have to say that this feels like a really rare use case)
#3710 was originally created in response to the "Step 3: Errors and Exceptions" part of https://securitylab.github.com/ctf/codeql-and-chill/.
try { parse(tainted); } catch (Exception e) { sink(e.getMessage()) }
The goal was to catch flow from
parse
tosink
, but the code ofparse
is unavailable. Will something like this be modelable with this PR? (I still have to say that this feels like a really rare use case)
Yes. In the context of this PR, the way to do that would be to make parse
a taint step that transfers taint from its argument to its returned exception. No need to match up the parse call with the enclosing try-catch, that's all something that the feature in this PR will handle (including the case when parse
would be nested inside some wrapper call).