diff --git a/cpp/ql/src/Security/CWE/CWE-078/ExecTainted.ql b/cpp/ql/src/Security/CWE/CWE-078/ExecTainted.ql index 7a3877f638ce..e50c5df03fad 100644 --- a/cpp/ql/src/Security/CWE/CWE-078/ExecTainted.ql +++ b/cpp/ql/src/Security/CWE/CWE-078/ExecTainted.ql @@ -19,6 +19,7 @@ import semmle.code.cpp.security.Security import semmle.code.cpp.valuenumbering.GlobalValueNumbering import semmle.code.cpp.ir.IR import semmle.code.cpp.ir.dataflow.TaintTracking +import semmle.code.cpp.ir.dataflow.TaintTracking2 import semmle.code.cpp.security.FlowSources import semmle.code.cpp.models.implementations.Strcat import DataFlow::PathGraph @@ -83,6 +84,32 @@ class ExecState extends DataFlow::FlowState { DataFlow::Node getFstNode() { result = fst } DataFlow::Node getSndNode() { result = snd } + + /** Holds if this is a possible `ExecState` for `sink`. */ + predicate isFeasibleForSink(DataFlow::Node sink) { + any(ExecStateConfiguration conf).hasFlow(snd, sink) + } +} + +/** + * A `TaintTracking` configuration that's used to find the relevant `ExecState`s for a + * given sink. This avoids a cartesian product between all sinks and all `ExecState`s in + * `ExecTaintConfiguration::isSink`. + */ +class ExecStateConfiguration extends TaintTracking2::Configuration { + ExecStateConfiguration() { this = "ExecStateConfiguration" } + + override predicate isSource(DataFlow::Node source) { + exists(ExecState state | state.getSndNode() = source) + } + + override predicate isSink(DataFlow::Node sink) { + shellCommand(sinkAsArgumentIndirection(sink), _) + } + + override predicate isSanitizerOut(DataFlow::Node node) { + isSink(node, _) // Prevent duplicates along a call chain, since `shellCommand` will include wrappers + } } class ExecTaintConfiguration extends TaintTracking::Configuration { @@ -94,8 +121,8 @@ class ExecTaintConfiguration extends TaintTracking::Configuration { } override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) { - shellCommand(sinkAsArgumentIndirection(sink), _) and - state instanceof ExecState + any(ExecStateConfiguration conf).isSink(sink) and + state.(ExecState).isFeasibleForSink(sink) } override predicate isAdditionalTaintStep(