diff --git a/change-notes/1.23/analysis-cpp.md b/change-notes/1.23/analysis-cpp.md index e883f885f0b6..62b59a7b8492 100644 --- a/change-notes/1.23/analysis-cpp.md +++ b/change-notes/1.23/analysis-cpp.md @@ -25,3 +25,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. diff --git a/cpp/ql/src/Critical/NewDelete.qll b/cpp/ql/src/Critical/NewDelete.qll index 5cd7ef646d11..39b265556b02 100644 --- a/cpp/ql/src/Critical/NewDelete.qll +++ b/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()) ) ) ) diff --git a/cpp/ql/src/Critical/OverflowStatic.ql b/cpp/ql/src/Critical/OverflowStatic.ql index a0917c470244..bd76173b6f49 100644 --- a/cpp/ql/src/Critical/OverflowStatic.ql +++ b/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() ) } diff --git a/cpp/ql/src/Likely Bugs/Conversion/LossyFunctionResultCast.ql b/cpp/ql/src/Likely Bugs/Conversion/LossyFunctionResultCast.ql index 5a8c3ee84bcc..0b844bdcbf5c 100644 --- a/cpp/ql/src/Likely Bugs/Conversion/LossyFunctionResultCast.ql +++ b/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() ) } diff --git a/cpp/ql/src/Likely Bugs/Likely Typos/UsingStrcpyAsBoolean.ql b/cpp/ql/src/Likely Bugs/Likely Typos/UsingStrcpyAsBoolean.ql index e41b9f7113dc..ddc74cc62057 100644 --- a/cpp/ql/src/Likely Bugs/Likely Typos/UsingStrcpyAsBoolean.ql +++ b/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." diff --git a/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql b/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql index e45be7640bf1..d5ab739f3484 100644 --- a/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql +++ b/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." } diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/DataFlow.qll b/cpp/ql/src/semmle/code/cpp/dataflow/DataFlow.qll index bfb93f77bfa7..3425e9b99016 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/DataFlow.qll +++ b/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 diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking.qll b/cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking.qll index 92cd085927a5..a40579adde16 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking.qll +++ b/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 diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking2.qll b/cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking2.qll index f67a0e711002..bde6c3633471 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking2.qll +++ b/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 diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll index 13f5db09b6c8..9b57e0dbaa12 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll @@ -490,6 +490,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`. */ diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll index e379157a6031..93910382cee9 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll +++ b/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`. diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index e9715925432a..1db97a49c03b 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/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. *