Merge pull request #1806 from jbj/localExprFlow

C++: Add localExprFlow and localExprTaint

change-notes/1.23/analysis-cpp.md

@@ -28,3 +28,6 @@ The following changes in version 1.23 affect C/C++ analysis in all applications.
   picture of the partial flow paths from a given source. The feature is
   disabled by default and can be enabled for individual configurations by
   overriding `int explorationLimit()`.
+* There is now a `DataFlow::localExprFlow` predicate and a
+  `TaintTracking::localExprTaint` predicate to make it easy to use the most
+  common case of local data flow and taint: from one `Expr` to another.

cpp/ql/src/Critical/NewDelete.qll

@@ -47,7 +47,7 @@ predicate allocExprOrIndirect(Expr alloc, string kind) {
       or
       exists(Expr e |
         allocExprOrIndirect(e, kind) and
-        DataFlow::localFlow(DataFlow::exprNode(e), DataFlow::exprNode(rtn.getExpr()))
+        DataFlow::localExprFlow(e, rtn.getExpr())
       )
     )
   )

cpp/ql/src/Critical/OverflowStatic.ql

@@ -95,7 +95,7 @@ class CallWithBufferSize extends FunctionCall {
 
   int statedSizeValue() {
     exists(Expr statedSizeSrc |
-      DataFlow::localFlow(DataFlow::exprNode(statedSizeSrc), DataFlow::exprNode(statedSizeExpr())) and
+      DataFlow::localExprFlow(statedSizeSrc, statedSizeExpr()) and
       result = statedSizeSrc.getValue().toInt()
     )
   }

cpp/ql/src/Likely Bugs/Conversion/LossyFunctionResultCast.ql

@@ -55,7 +55,7 @@ predicate whiteListWrapped(FunctionCall fc) {
   whitelistPow(fc) or
   exists(Expr e, ReturnStmt rs |
     whiteListWrapped(e) and
-    DataFlow::localFlow(DataFlow::exprNode(e), DataFlow::exprNode(rs.getExpr())) and
+    DataFlow::localExprFlow(e, rs.getExpr()) and
     fc.getTarget() = rs.getEnclosingFunction()
   )
 }

cpp/ql/src/Likely Bugs/Likely Typos/UsingStrcpyAsBoolean.ql

@@ -23,7 +23,7 @@ predicate isBoolean(Expr e1) {
 }
 
 predicate isStringCopyCastedAsBoolean(FunctionCall func, Expr expr1, string msg) {
-  DataFlow::localFlow(DataFlow::exprNode(func), DataFlow::exprNode(expr1)) and
+  DataFlow::localExprFlow(func, expr1) and
   isBoolean(expr1.getConversion*()) and
   func.getTarget() instanceof StrcpyFunction and
   msg = "Return value of " + func.getTarget().getName() + " used as a Boolean."

cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql

@@ -36,12 +36,10 @@ class MallocCall extends FunctionCall
 
 predicate terminationProblem(MallocCall malloc, string msg) {
   malloc.getAllocatedSize() instanceof StrlenCall and
-  not exists(DataFlow::Node def, DataFlow::Node use, FunctionCall fc, MemcpyFunction memcpy, int ix |
-    DataFlow::localFlow(def, use) and
-    def.asExpr() = malloc and
+  not exists(FunctionCall fc, MemcpyFunction memcpy, int ix |
+    DataFlow::localExprFlow(malloc, fc.getArgument(ix)) and
     fc.getTarget() = memcpy and
-    memcpy.hasArrayOutput(ix) and
-    use.asExpr() = fc.getArgument(ix)
+    memcpy.hasArrayOutput(ix)
   ) and
   msg = "This allocation does not include space to null-terminate the string."
 }

cpp/ql/src/semmle/code/cpp/dataflow/DataFlow.qll

@@ -11,8 +11,10 @@
  *
  * To use global (interprocedural) data flow, extend the class
  * `DataFlow::Configuration` as documented on that class. To use local
- * (intraprocedural) data flow, invoke `DataFlow::localFlow` or
- * `DataFlow::LocalFlowStep` with arguments of type `DataFlow::Node`.
+ * (intraprocedural) data flow between expressions, call
+ * `DataFlow::localExprFlow`. For more general cases of local data flow, call
+ * `DataFlow::localFlow` or `DataFlow::localFlowStep` with arguments of type
+ * `DataFlow::Node`.
  */
 import cpp
 

cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking.qll

@@ -6,6 +6,13 @@
  * the information from the source is preserved at the sink. For example, taint
  * propagates from `x` to `x + 100`, but it does not propagate from `x` to `x >
  * 100` since we consider a single bit of information to be too little.
+ *
+ * To use global (interprocedural) taint tracking, extend the class
+ * `TaintTracking::Configuration` as documented on that class. To use local
+ * (intraprocedural) taint tracking between expressions, call
+ * `TaintTracking::localExprTaint`. For more general cases of local taint
+ * tracking, call `TaintTracking::localTaint` or
+ * `TaintTracking::localTaintStep` with arguments of type `DataFlow::Node`.
  */
 import semmle.code.cpp.dataflow.DataFlow
 import semmle.code.cpp.dataflow.DataFlow2

cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking2.qll

@@ -1,11 +1,14 @@
 /**
- * Provides classes for performing local (intra-procedural) and
- * global (inter-procedural) taint-tracking analyses.
+ * Provides a `TaintTracking2` module, which is a copy of the `TaintTracking`
+ * module. Use this class when data-flow configurations or taint-tracking
+ * configurations must depend on each other. Two classes extending
+ * `DataFlow::Configuration` should never depend on each other, but one of them
+ * should instead depend on a `DataFlow2::Configuration`, a
+ * `DataFlow3::Configuration`, or a `DataFlow4::Configuration`. The
+ * `TaintTracking::Configuration` class extends `DataFlow::Configuration`, and
+ * `TaintTracking2::Configuration` extends `DataFlow2::Configuration`.
  *
- * We define _taint propagation_ informally to mean that a substantial part of
- * the information from the source is preserved at the sink. For example, taint
- * propagates from `x` to `x + 100`, but it does not propagate from `x` to `x >
- * 100` since we consider a single bit of information to be too little.
+ * See `semmle.code.cpp.dataflow.TaintTracking` for the full documentation.
  */
 module TaintTracking2 {
   import semmle.code.cpp.dataflow.internal.tainttracking2.TaintTrackingImpl

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll

@@ -496,6 +496,12 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
  */
 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.
+ */
+predicate localExprFlow(Expr e1, Expr e2) { localFlow(exprNode(e1), exprNode(e2)) }
+
 /**
  * Holds if the initial value of `v`, if it is a source, flows to `var`.
  */

cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll

@@ -80,6 +80,14 @@ 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.
+ */
+predicate localExprTaint(Expr e1, Expr e2) {
+  localTaint(DataFlow::exprNode(e1), DataFlow::exprNode(e2))
+}
+
 /**
  * Holds if we do not propagate taint from `fromExpr` to `toExpr`
  * even though `toExpr` is the AST parent of `fromExpr`.

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -178,6 +178,12 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
  */
 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.
+ */
+predicate localExprFlow(Expr e1, Expr e2) { localFlow(exprNode(e1), exprNode(e2)) }
+
 /**
  * A guard that validates some expression.
  *