From a3285653aebb514c4f31ac46b14dd8854654addb Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 9 Dec 2022 14:20:27 +0000 Subject: [PATCH 1/2] C++: Prevent an expression to stepping to itself. --- .../cpp/ir/dataflow/internal/DataFlowUtil.qll | 46 ++++++++++++++----- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index c20f71b7c03..ff28a44ec36 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -1326,13 +1326,8 @@ predicate localInstructionFlow(Instruction e1, Instruction e2) { localFlow(instructionNode(e1), instructionNode(e2)) } -/** - * Holds if data can flow from `e1` to `e2` in zero or more - * local (intra-procedural) steps. - */ -pragma[inline] -predicate localExprFlow(Expr e1, Expr e2) { localExprFlowStep*(e1, e2) } - +cached +private module ExprFlowCached { /** * Holds if `n1.asExpr()` doesn't have a result and `n1` flows to `n2` in a single * dataflow step. @@ -1352,14 +1347,43 @@ private predicate localStepsToExpr(Node n1, Node n2, Expr e2) { e2 = n2.asExpr() } -/** Holds if data can flow from `e1` to `e2` in one local (intra-procedural) step. */ -cached -predicate localExprFlowStep(Expr e1, Expr e2) { - exists(Node mid, Node n1, Node n2 | + /** + * Holds if `n1.asExpr() = e1` and `n2.asExpr() = e2` and `n2` is the first node + * reacahble from `n1` such that `n2.asExpr()` exists. + */ + private predicate localExprFlowSingleExprStep(Node n1, Expr e1, Node n2, Expr e2) { + exists(Node mid | localFlowStep(n1, mid) and localStepsToExpr(mid, n2, e2) and e1 = n1.asExpr() ) + } + + /** + * Holds if `n1.asExpr() = e1` and `e1 != e2` and `n2` is the first reachable node from + * `n1` such that `n2.asExpr() = e2`. + */ + private predicate localExprFlowStepImpl(Node n1, Expr e1, Node n2, Expr e2) { + exists(Node n, Expr e | localExprFlowSingleExprStep(n1, e1, n, e) | + // If `n.asExpr()` and `n1.asExpr()` both resolve to the same node (which can + // happen if `n2` is the node attached to a conversion of `e1`), then we recursively + // perform another expression step. + if e1 = e + then localExprFlowStepImpl(n, e, n2, e2) + else ( + // If we manage to step to a different expression we're done. + e2 = e and + n2 = n + ) + ) + } + + /** Holds if data can flow from `e1` to `e2` in one local (intra-procedural) step. */ + cached + predicate localExprFlowStep(Expr e1, Expr e2) { localExprFlowStepImpl(_, e1, _, e2) } +} + +import ExprFlowCached } cached From 52bf39bcf93e07ff7d95edb9bdf9543a0955131b Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 9 Dec 2022 14:21:09 +0000 Subject: [PATCH 2/2] C++: Use a 'fastTC' instead of '*' to improve performance. --- .../cpp/ir/dataflow/internal/DataFlowUtil.qll | 59 ++++++++++++------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index ff28a44ec36..43b88964531 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -1328,24 +1328,24 @@ predicate localInstructionFlow(Instruction e1, Instruction e2) { cached private module ExprFlowCached { -/** - * Holds if `n1.asExpr()` doesn't have a result and `n1` flows to `n2` in a single - * dataflow step. - */ -private predicate localStepFromNonExpr(Node n1, Node n2) { - not exists(n1.asExpr()) and - localFlowStep(n1, n2) -} + /** + * Holds if `n1.asExpr()` doesn't have a result and `n1` flows to `n2` in a single + * dataflow step. + */ + private predicate localStepFromNonExpr(Node n1, Node n2) { + not exists(n1.asExpr()) and + localFlowStep(n1, n2) + } -/** - * Holds if `n1.asExpr()` doesn't have a result, `n2.asExpr() = e2` and - * `n2` is the first node reachable from `n1` such that `n2.asExpr()` exists. - */ -pragma[nomagic] -private predicate localStepsToExpr(Node n1, Node n2, Expr e2) { - localStepFromNonExpr*(n1, n2) and - e2 = n2.asExpr() -} + /** + * Holds if `n1.asExpr()` doesn't have a result, `n2.asExpr() = e2` and + * `n2` is the first node reachable from `n1` such that `n2.asExpr()` exists. + */ + pragma[nomagic] + private predicate localStepsToExpr(Node n1, Node n2, Expr e2) { + localStepFromNonExpr*(n1, n2) and + e2 = n2.asExpr() + } /** * Holds if `n1.asExpr() = e1` and `n2.asExpr() = e2` and `n2` is the first node @@ -1353,10 +1353,10 @@ private predicate localStepsToExpr(Node n1, Node n2, Expr e2) { */ private predicate localExprFlowSingleExprStep(Node n1, Expr e1, Node n2, Expr e2) { exists(Node mid | - localFlowStep(n1, mid) and - localStepsToExpr(mid, n2, e2) and - e1 = n1.asExpr() - ) + localFlowStep(n1, mid) and + localStepsToExpr(mid, n2, e2) and + e1 = n1.asExpr() + ) } /** @@ -1384,6 +1384,23 @@ private predicate localStepsToExpr(Node n1, Node n2, Expr e2) { } import ExprFlowCached + +/** + * Holds if data can flow from `e1` to `e2` in one or more + * local (intra-procedural) steps. + */ +pragma[inline] +private predicate localExprFlowPlus(Expr e1, Expr e2) = fastTC(localExprFlowStep/2)(e1, e2) + +/** + * Holds if data can flow from `e1` to `e2` in zero or more + * local (intra-procedural) steps. + */ +pragma[inline] +predicate localExprFlow(Expr e1, Expr e2) { + e1 = e2 + or + localExprFlowPlus(e1, e2) } cached