CPP: Fix some use after free FPs. #14275
Conversation
|
I'm wondering if this fix will still work after the frontend upgrade, because the IR (and the expressions yielded) will be different for these kinds of assignments these. Let me check that. |
The fix still woks, but no longer seems needed after the frontend upgrade. |
| void test_free_malloc() { | ||
| void *a = malloc(10); | ||
| void *b; | ||
| free(b = a); |
There was a problem hiding this comment.
I think this needs some kind of // GOOD or // BAD comment. However, it's not clear to me to which query being tested here the comment applies.
cpp/ql/src/Critical/UseAfterFree.ql
Outdated
| @@ -31,7 +31,7 @@ private predicate externalCallNeverDereferences(FormattingFunctionCall call, int | |||
|
|
|||
| predicate isUse0(DataFlow::Node n, Expr e) { | |||
| e = n.asExpr() and | |||
| not isFree(_, e, _) and | |||
| not isFree(n, _, _) and | |||
There was a problem hiding this comment.
This makes me a bit nervous. because isFree looks a (un)converted expressions explicitly. Was the change just made to bind n, or is there a deeper reason? If the former, can't we just drop the node argument of isUse0 completely?
There was a problem hiding this comment.
I think the change was leftover from an earlier attempt which I accidentality left in. I have now changed it to not have a DataFlow::Node argumebnt at all and call asExpr at call sites.
alexet
force-pushed
the
fix-use-after-free-fp
branch
from
d51c757
to
6b0ae0f
Compare
|
Thanks for the fixes. Your latest DCA experiment seemed to have failed hard. Kicked off a new DCA experiment myself. |
We try and exclude source expressions that are frees themselves however if there are more than one expresssion for a node we may use one expression to detect it is a use but not eliminate it due to the other expression.
The dataflow node for
a = bhas two expressions (a = bandb).a = bis a function argument to an unknown function so we assume that it is used.bis not an argument tofree. By using different expression we incorrectly work out that the node is a non-free use.