From f2352d82728710f82263c13a57416be83eecb99c Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 31 Jan 2022 14:28:45 +0100 Subject: [PATCH] Data flow: Inline `local(Expr|Instruction)?(Flow|Taint)` Computing a full transitive closure is often bad; by inlining all calls we are providing more context to the QL optimizer. --- cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll | 2 ++ .../semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll | 2 ++ .../lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll | 3 +++ .../semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll | 3 +++ .../semmle/code/csharp/dataflow/internal/DataFlowPublic.qll | 1 + .../code/csharp/dataflow/internal/TaintTrackingPublic.qll | 2 ++ docs/ql-libraries/dataflow/dataflow.md | 2 +- .../ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll | 2 ++ .../semmle/code/java/dataflow/internal/TaintTrackingUtil.qll | 2 ++ .../lib/semmle/python/dataflow/new/internal/DataFlowUtil.qll | 1 + .../python/dataflow/new/internal/TaintTrackingPublic.qll | 2 ++ ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll | 2 ++ .../lib/codeql/ruby/dataflow/internal/TaintTrackingPublic.qll | 2 ++ 13 files changed, 25 insertions(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll index c67374c3db9..ed3eadca08f 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll @@ -592,12 +592,14 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) { * Holds if data flows from `source` to `sink` in zero or more local * (intra-procedural) steps. */ +pragma[inline] predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) } /** * 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) { localFlow(exprNode(e1), exprNode(e2)) } /** diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll index 5bc4d88c658..6fb13455286 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll @@ -124,12 +124,14 @@ predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeT * Holds if taint may propagate from `source` to `sink` in zero or more local * (intra-procedural) steps. */ +pragma[inline] predicate localTaint(DataFlow::Node source, DataFlow::Node sink) { localTaintStep*(source, sink) } /** * Holds if taint can flow from `e1` to `e2` in zero or more * local (intra-procedural) steps. */ +pragma[inline] predicate localExprTaint(Expr e1, Expr e2) { localTaint(DataFlow::exprNode(e1), DataFlow::exprNode(e2)) } 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 582d397fead..75b530e937d 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 @@ -1032,12 +1032,14 @@ SideEffectInstruction getSideEffectFor(CallInstruction call, int argument) { * Holds if data flows from `source` to `sink` in zero or more local * (intra-procedural) steps. */ +pragma[inline] predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) } /** * Holds if data can flow from `i1` to `i2` in zero or more * local (intra-procedural) steps. */ +pragma[inline] predicate localInstructionFlow(Instruction e1, Instruction e2) { localFlow(instructionNode(e1), instructionNode(e2)) } @@ -1046,6 +1048,7 @@ predicate localInstructionFlow(Instruction e1, Instruction 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) { localFlow(exprNode(e1), exprNode(e2)) } private newtype TContent = diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll index b6b9a7243b2..1086701a97f 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll @@ -121,12 +121,14 @@ private predicate operandToInstructionTaintStep(Operand opFrom, Instruction inst * Holds if taint may propagate from `source` to `sink` in zero or more local * (intra-procedural) steps. */ +pragma[inline] predicate localTaint(DataFlow::Node source, DataFlow::Node sink) { localTaintStep*(source, sink) } /** * Holds if taint can flow from `i1` to `i2` in zero or more * local (intra-procedural) steps. */ +pragma[inline] predicate localInstructionTaint(Instruction i1, Instruction i2) { localTaint(DataFlow::instructionNode(i1), DataFlow::instructionNode(i2)) } @@ -135,6 +137,7 @@ predicate localInstructionTaint(Instruction i1, Instruction i2) { * Holds if taint can flow from `e1` to `e2` in zero or more * local (intra-procedural) steps. */ +pragma[inline] predicate localExprTaint(Expr e1, Expr e2) { localTaint(DataFlow::exprNode(e1), DataFlow::exprNode(e2)) } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll index d9ab0bd2fe7..b6d749e18fd 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll @@ -160,6 +160,7 @@ predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) } * 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) { localFlow(exprNode(e1), exprNode(e2)) } /** diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPublic.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPublic.qll index 6e4ba538a40..1e60165d748 100755 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPublic.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPublic.qll @@ -5,12 +5,14 @@ private import TaintTrackingPrivate * Holds if taint propagates from `source` to `sink` in zero or more local * (intra-procedural) steps. */ +pragma[inline] predicate localTaint(DataFlow::Node source, DataFlow::Node sink) { localTaintStep*(source, sink) } /** * Holds if taint can flow from `e1` to `e2` in zero or more * local (intra-procedural) steps. */ +pragma[inline] predicate localExprTaint(Expr e1, Expr e2) { localTaint(DataFlow::exprNode(e1), DataFlow::exprNode(e2)) } diff --git a/docs/ql-libraries/dataflow/dataflow.md b/docs/ql-libraries/dataflow/dataflow.md index 1a83d0c8e05..31a8e12a5cc 100644 --- a/docs/ql-libraries/dataflow/dataflow.md +++ b/docs/ql-libraries/dataflow/dataflow.md @@ -92,7 +92,7 @@ Recommendations: See the C/C++ implementation, which makes use of this feature. Another use of this indirection is to hide synthesized local steps that are only relevant for global flow. See the C# implementation for an example of this. -* Define `predicate localFlow(Node node1, Node node2) { localFlowStep*(node1, node2) }`. +* Define `pragma[inline] predicate localFlow(Node node1, Node node2) { localFlowStep*(node1, node2) }`. * Make the local flow step relation in `simpleLocalFlowStep` follow def-to-first-use and use-to-next-use steps for SSA variables. Def-use steps also work, but the upside of `use-use` steps is that sources defined in terms diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll index 1ae6749877d..695f20110c1 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -74,12 +74,14 @@ private module ThisFlow { * Holds if data can flow from `node1` to `node2` in zero or more * local (intra-procedural) steps. */ +pragma[inline] predicate localFlow(Node node1, Node node2) { localFlowStep*(node1, node2) } /** * 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) { localFlow(exprNode(e1), exprNode(e2)) } /** diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll index 59608d792f2..4797dcfaf59 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -21,12 +21,14 @@ private import semmle.code.java.frameworks.JaxWS * Holds if taint can flow from `src` to `sink` in zero or more * local (intra-procedural) steps. */ +pragma[inline] predicate localTaint(DataFlow::Node src, DataFlow::Node sink) { localTaintStep*(src, sink) } /** * Holds if taint can flow from `src` to `sink` in zero or more * local (intra-procedural) steps. */ +pragma[inline] predicate localExprTaint(Expr src, Expr sink) { localTaint(DataFlow::exprNode(src), DataFlow::exprNode(sink)) } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowUtil.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowUtil.qll index 9bdb7ede42c..4dbf2a5d4cd 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowUtil.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowUtil.qll @@ -15,6 +15,7 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) { simpleLocalFlowStep(nodeFr * Holds if data flows from `source` to `sink` in zero or more local * (intra-procedural) steps. */ +pragma[inline] predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) } /** diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/TaintTrackingPublic.qll b/python/ql/lib/semmle/python/dataflow/new/internal/TaintTrackingPublic.qll index 53697f0e4ab..5b88de4fab9 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/TaintTrackingPublic.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/TaintTrackingPublic.qll @@ -14,12 +14,14 @@ private import semmle.python.Frameworks * Holds if taint propagates from `source` to `sink` in zero or more local * (intra-procedural) steps. */ +pragma[inline] predicate localTaint(DataFlow::Node source, DataFlow::Node sink) { localTaintStep*(source, sink) } /** * Holds if taint can flow from `e1` to `e2` in zero or more local (intra-procedural) * steps. */ +pragma[inline] predicate localExprTaint(Expr e1, Expr e2) { localTaint(DataFlow::exprNode(e1), DataFlow::exprNode(e2)) } diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll index 0d9e222eb89..02aa2539bf3 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll @@ -151,12 +151,14 @@ predicate localFlowStep = localFlowStepImpl/2; * Holds if data flows from `source` to `sink` in zero or more local * (intra-procedural) steps. */ +pragma[inline] predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) } /** * Holds if data can flow from `e1` to `e2` in zero or more * local (intra-procedural) steps. */ +pragma[inline] predicate localExprFlow(CfgNodes::ExprCfgNode e1, CfgNodes::ExprCfgNode e2) { localFlow(exprNode(e1), exprNode(e2)) } diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPublic.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPublic.qll index 3fe5659bdc7..a8cd0442489 100755 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPublic.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPublic.qll @@ -8,12 +8,14 @@ private import FlowSummaryImpl as FlowSummaryImpl * Holds if taint propagates from `source` to `sink` in zero or more local * (intra-procedural) steps. */ +pragma[inline] predicate localTaint(DataFlow::Node source, DataFlow::Node sink) { localTaintStep*(source, sink) } /** * Holds if taint can flow from `e1` to `e2` in zero or more * local (intra-procedural) steps. */ +pragma[inline] predicate localExprTaint(CfgNodes::ExprCfgNode e1, CfgNodes::ExprCfgNode e2) { localTaint(DataFlow::exprNode(e1), DataFlow::exprNode(e2)) }