C++: Use flow states in cpp/command-line-injection

2026-05-01 11:45:14 +02:00 · 2022-03-15 15:08:28 +01:00
parent 4bf35ad188
commit f8198c3123
4 changed files with 86 additions and 159 deletions
--- a/cpp/ql/src/Security/CWE/CWE-078/ExecTainted.ql
+++ b/cpp/ql/src/Security/CWE/CWE-078/ExecTainted.ql
@@ -19,9 +19,9 @@ 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

 Expr sinkAsArgumentIndirection(DataFlow::Node sink) {
  result =
@@ -66,154 +66,69 @@ predicate interestingConcatenation(DataFlow::Node fst, DataFlow::Node snd) {
  )
 }

-class TaintToConcatenationConfiguration extends TaintTracking::Configuration {
-  TaintToConcatenationConfiguration() { this = "TaintToConcatenationConfiguration" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof FlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { interestingConcatenation(sink, _) }
-
-  override predicate isSanitizer(DataFlow::Node node) {
-    node.asInstruction().getResultType() instanceof IntegralType
-    or
-    node.asInstruction().getResultType() instanceof FloatingPointType
-  }
+class ConcatState extends DataFlow::FlowState {
+  ConcatState() { this = "ConcatState" }
 }

-class ExecTaintConfiguration extends TaintTracking2::Configuration {
+class ExecState extends DataFlow::FlowState {
+  DataFlow::Node fst;
+  DataFlow::Node snd;
+
+  ExecState() {
+    this = "ExecState (" + fst.getLocation() + ", " + snd.getLocation() + ")" and
+    interestingConcatenation(fst, snd)
+  }
+
+  DataFlow::Node getFstNode() { result = fst }
+
+  DataFlow::Node getSndNode() { result = snd }
+}
+
+class ExecTaintConfiguration extends TaintTracking::Configuration {
  ExecTaintConfiguration() { this = "ExecTaintConfiguration" }

-  override predicate isSource(DataFlow::Node source) {
-    exists(DataFlow::Node prevSink, TaintToConcatenationConfiguration conf |
-      conf.hasFlow(_, prevSink) and
-      interestingConcatenation(prevSink, source)
-    )
+  override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
+    source instanceof FlowSource and
+    state instanceof ConcatState
  }

-  override predicate isSink(DataFlow::Node sink) {
-    shellCommand(sinkAsArgumentIndirection(sink), _)
+  override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
+    shellCommand(sinkAsArgumentIndirection(sink), _) and
+    state instanceof ExecState
  }

-  override predicate isSanitizerOut(DataFlow::Node node) {
-    isSink(node) // Prevent duplicates along a call chain, since `shellCommand` will include wrappers
-  }
-}
-
-module StitchedPathGraph {
-  // There's a different PathNode class for each DataFlowImplN.qll, so we can't simply combine the
-  // PathGraph predicates directly. Instead, we use a newtype so there's a single type that
-  // contains both sets of PathNodes.
-  newtype TMergedPathNode =
-    TPathNode1(DataFlow::PathNode node) or
-    TPathNode2(DataFlow2::PathNode node)
-
-  // this wraps the toString and location predicates so we can use the merged node type in a
-  // selection
-  class MergedPathNode extends TMergedPathNode {
-    string toString() {
-      exists(DataFlow::PathNode n |
-        this = TPathNode1(n) and
-        result = n.toString()
-      )
-      or
-      exists(DataFlow2::PathNode n |
-        this = TPathNode2(n) and
-        result = n.toString()
-      )
-    }
-
-    DataFlow::Node getNode() {
-      exists(DataFlow::PathNode n |
-        this = TPathNode1(n) and
-        result = n.getNode()
-      )
-      or
-      exists(DataFlow2::PathNode n |
-        this = TPathNode2(n) and
-        result = n.getNode()
-      )
-    }
-
-    DataFlow::PathNode getPathNode1() { this = TPathNode1(result) }
-
-    DataFlow2::PathNode getPathNode2() { this = TPathNode2(result) }
-
-    predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      exists(DataFlow::PathNode n |
-        this = TPathNode1(n) and
-        n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-      )
-      or
-      exists(DataFlow2::PathNode n |
-        this = TPathNode2(n) and
-        n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-      )
-    }
-  }
-
-  query predicate edges(MergedPathNode a, MergedPathNode b) {
-    exists(DataFlow::PathNode an, DataFlow::PathNode bn |
-      a = TPathNode1(an) and
-      b = TPathNode1(bn) and
-      DataFlow::PathGraph::edges(an, bn)
-    )
-    or
-    exists(DataFlow2::PathNode an, DataFlow2::PathNode bn |
-      a = TPathNode2(an) and
-      b = TPathNode2(bn) and
-      DataFlow2::PathGraph::edges(an, bn)
-    )
-    or
-    // This is where paths from the two configurations are connected. `interestingConcatenation`
-    // is the only thing in this module that's actually specific to the query - everything else is
-    // just using types and predicates from the DataFlow library.
-    interestingConcatenation(a.getNode(), b.getNode()) and
-    a instanceof TPathNode1 and
-    b instanceof TPathNode2
-  }
-
-  query predicate nodes(MergedPathNode mpn, string key, string val) {
-    // here we just need the union of the underlying `nodes` predicates
-    exists(DataFlow::PathNode n |
-      mpn = TPathNode1(n) and
-      DataFlow::PathGraph::nodes(n, key, val)
-    )
-    or
-    exists(DataFlow2::PathNode n |
-      mpn = TPathNode2(n) and
-      DataFlow2::PathGraph::nodes(n, key, val)
-    )
-  }
-
-  query predicate subpaths(
-    MergedPathNode arg, MergedPathNode par, MergedPathNode ret, MergedPathNode out
+  override predicate isAdditionalTaintStep(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
  ) {
-    // just forward subpaths from the underlying libraries. This might be slightly awkward when
-    // the concatenation is deep in a call chain.
-    DataFlow::PathGraph::subpaths(arg.getPathNode1(), par.getPathNode1(), ret.getPathNode1(),
-      out.getPathNode1())
-    or
-    DataFlow2::PathGraph::subpaths(arg.getPathNode2(), par.getPathNode2(), ret.getPathNode2(),
-      out.getPathNode2())
+    state1 instanceof ConcatState and
+    state2.(ExecState).getFstNode() = node1 and
+    state2.(ExecState).getSndNode() = node2
+  }
+
+  override predicate isSanitizer(DataFlow::Node node, DataFlow::FlowState state) {
+    (
+      node.asInstruction().getResultType() instanceof IntegralType
+      or
+      node.asInstruction().getResultType() instanceof FloatingPointType
+    ) and
+    state instanceof ConcatState
+  }
+
+  override predicate isSanitizerOut(DataFlow::Node node, DataFlow::FlowState state) {
+    isSink(node, state) // Prevent duplicates along a call chain, since `shellCommand` will include wrappers
  }
 }

-import StitchedPathGraph
-
 from
-  DataFlow::PathNode sourceNode, DataFlow::PathNode concatSink, DataFlow2::PathNode concatSource,
-  DataFlow2::PathNode sinkNode, string taintCause, string callChain,
-  TaintToConcatenationConfiguration conf1, ExecTaintConfiguration conf2
+  ExecTaintConfiguration conf, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode,
+  string taintCause, string callChain, DataFlow::Node concatResult
 where
+  conf.hasFlowPath(sourceNode, sinkNode) and
  taintCause = sourceNode.getNode().(FlowSource).getSourceType() and
-  conf1.hasFlowPath(sourceNode, concatSink) and
-  interestingConcatenation(concatSink.getNode(), concatSource.getNode()) and // this loses call context
-  conf2.hasFlowPath(concatSource, sinkNode) and
-  shellCommand(sinkAsArgumentIndirection(sinkNode.getNode()), callChain)
-select sinkAsArgumentIndirection(sinkNode.getNode()), TPathNode1(sourceNode).(MergedPathNode),
-  TPathNode2(sinkNode).(MergedPathNode),
+  shellCommand(sinkAsArgumentIndirection(sinkNode.getNode()), callChain) and
+  concatResult = sinkNode.getState().(ExecState).getSndNode()
+select sinkAsArgumentIndirection(sinkNode.getNode()), sourceNode, sinkNode,
  "This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to "
-    + callChain, sourceNode, "user input (" + taintCause + ")", concatSource,
-  concatSource.toString()
+    + callChain, sourceNode, "user input (" + taintCause + ")", concatResult,
+  concatResult.toString()
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-078/SAMATE/ExecTainted/ExecTainted.expected
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-078/SAMATE/ExecTainted/ExecTainted.expected
@@ -3,7 +3,6 @@ edges
 | tests.cpp:33:34:33:39 | call to getenv | tests.cpp:38:39:38:49 | environment indirection |
 | tests.cpp:38:25:38:36 | strncat output argument | tests.cpp:26:15:26:23 | ReturnValue |
 | tests.cpp:38:39:38:49 | environment indirection | tests.cpp:38:25:38:36 | strncat output argument |
-| tests.cpp:38:39:38:49 | environment indirection | tests.cpp:38:25:38:36 | strncat output argument |
 | tests.cpp:51:12:51:20 | call to badSource | tests.cpp:53:16:53:19 | data indirection |
 nodes
 | tests.cpp:26:15:26:23 | ReturnValue | semmle.label | ReturnValue |
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-078/semmle/ExecTainted/ExecTainted.expected
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-078/semmle/ExecTainted/ExecTainted.expected
@@ -2,64 +2,55 @@ edges
 | test.cpp:16:20:16:23 | argv | test.cpp:22:45:22:52 | userName indirection |
 | test.cpp:22:13:22:20 | sprintf output argument | test.cpp:23:12:23:19 | command1 indirection |
 | test.cpp:22:45:22:52 | userName indirection | test.cpp:22:13:22:20 | sprintf output argument |
-| test.cpp:22:45:22:52 | userName indirection | test.cpp:22:13:22:20 | sprintf output argument |
 | test.cpp:47:21:47:26 | call to getenv | test.cpp:50:35:50:43 | envCflags indirection |
 | test.cpp:50:11:50:17 | sprintf output argument | test.cpp:51:10:51:16 | command indirection |
 | test.cpp:50:35:50:43 | envCflags indirection | test.cpp:50:11:50:17 | sprintf output argument |
-| test.cpp:50:35:50:43 | envCflags indirection | test.cpp:50:11:50:17 | sprintf output argument |
 | test.cpp:62:9:62:16 | fread output argument | test.cpp:64:20:64:27 | filename indirection |
 | test.cpp:64:11:64:17 | strncat output argument | test.cpp:65:10:65:16 | command indirection |
 | test.cpp:64:20:64:27 | filename indirection | test.cpp:64:11:64:17 | strncat output argument |
-| test.cpp:64:20:64:27 | filename indirection | test.cpp:64:11:64:17 | strncat output argument |
 | test.cpp:82:9:82:16 | fread output argument | test.cpp:84:20:84:27 | filename indirection |
 | test.cpp:84:11:84:17 | strncat output argument | test.cpp:85:32:85:38 | command indirection |
 | test.cpp:84:20:84:27 | filename indirection | test.cpp:84:11:84:17 | strncat output argument |
-| test.cpp:84:20:84:27 | filename indirection | test.cpp:84:11:84:17 | strncat output argument |
 | test.cpp:91:9:91:16 | fread output argument | test.cpp:93:17:93:24 | filename indirection |
 | test.cpp:93:11:93:14 | strncat output argument | test.cpp:94:45:94:48 | path indirection |
 | test.cpp:93:17:93:24 | filename indirection | test.cpp:93:11:93:14 | strncat output argument |
-| test.cpp:93:17:93:24 | filename indirection | test.cpp:93:11:93:14 | strncat output argument |
 | test.cpp:106:20:106:25 | call to getenv | test.cpp:107:33:107:36 | path indirection |
 | test.cpp:107:31:107:31 | call to operator+ | test.cpp:108:18:108:22 | call to c_str indirection |
 | test.cpp:107:33:107:36 | path indirection | test.cpp:107:31:107:31 | call to operator+ |
-| test.cpp:107:33:107:36 | path indirection | test.cpp:107:31:107:31 | call to operator+ |
 | test.cpp:113:20:113:25 | call to getenv | test.cpp:114:19:114:22 | path indirection |
 | test.cpp:114:17:114:17 | Call | test.cpp:114:25:114:29 | call to c_str indirection |
 | test.cpp:114:19:114:22 | path indirection | test.cpp:114:17:114:17 | Call |
-| test.cpp:114:19:114:22 | path indirection | test.cpp:114:17:114:17 | Call |
 | test.cpp:119:20:119:25 | call to getenv | test.cpp:120:19:120:22 | path indirection |
 | test.cpp:120:17:120:17 | Call | test.cpp:120:10:120:30 | call to data indirection |
 | test.cpp:120:19:120:22 | path indirection | test.cpp:120:17:120:17 | Call |
-| test.cpp:120:19:120:22 | path indirection | test.cpp:120:17:120:17 | Call |
 | test.cpp:140:9:140:11 | fread output argument | test.cpp:142:31:142:33 | str indirection |
 | test.cpp:142:11:142:17 | sprintf output argument | test.cpp:143:10:143:16 | command indirection |
 | test.cpp:142:31:142:33 | str indirection | test.cpp:142:11:142:17 | sprintf output argument |
-| test.cpp:142:31:142:33 | str indirection | test.cpp:142:11:142:17 | sprintf output argument |
 | test.cpp:174:9:174:16 | fread output argument | test.cpp:177:20:177:27 | filename indirection |
 | test.cpp:174:9:174:16 | fread output argument | test.cpp:178:22:178:26 | flags indirection |
 | test.cpp:174:9:174:16 | fread output argument | test.cpp:180:22:180:29 | filename indirection |
 | test.cpp:177:13:177:17 | strncat output argument | test.cpp:183:32:183:38 | command indirection |
 | test.cpp:177:20:177:27 | filename indirection | test.cpp:177:13:177:17 | strncat output argument |
-| test.cpp:177:20:177:27 | filename indirection | test.cpp:177:13:177:17 | strncat output argument |
 | test.cpp:178:13:178:19 | strncat output argument | test.cpp:183:32:183:38 | command indirection |
 | test.cpp:178:22:178:26 | flags indirection | test.cpp:178:13:178:19 | strncat output argument |
-| test.cpp:178:22:178:26 | flags indirection | test.cpp:178:13:178:19 | strncat output argument |
 | test.cpp:180:13:180:19 | strncat output argument | test.cpp:183:32:183:38 | command indirection |
 | test.cpp:180:22:180:29 | filename indirection | test.cpp:180:13:180:19 | strncat output argument |
-| test.cpp:180:22:180:29 | filename indirection | test.cpp:180:13:180:19 | strncat output argument |
 | test.cpp:186:47:186:54 | *filename | test.cpp:187:18:187:25 | filename indirection |
 | test.cpp:186:47:186:54 | *filename | test.cpp:188:20:188:24 | flags indirection |
 | test.cpp:186:47:186:54 | filename | test.cpp:187:18:187:25 | filename indirection |
 | test.cpp:186:47:186:54 | filename | test.cpp:188:20:188:24 | flags indirection |
 | test.cpp:187:11:187:15 | strncat output argument | test.cpp:188:11:188:17 | command [post update] |
 | test.cpp:187:11:187:15 | strncat output argument | test.cpp:188:11:188:17 | command [post update] |
+| test.cpp:187:11:187:15 | strncat output argument | test.cpp:188:11:188:17 | command [post update] |
+| test.cpp:187:11:187:15 | strncat output argument | test.cpp:188:11:188:17 | command [post update] |
 | test.cpp:187:18:187:25 | filename indirection | test.cpp:187:11:187:15 | strncat output argument |
 | test.cpp:187:18:187:25 | filename indirection | test.cpp:187:11:187:15 | strncat output argument |
 | test.cpp:188:11:188:17 | command [post update] | test.cpp:188:11:188:17 | command [post update] |
-| test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
-| test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
-| test.cpp:188:11:188:17 | command [post update] | test.cpp:205:10:205:16 | command [post update] |
-| test.cpp:188:11:188:17 | command [post update] | test.cpp:205:10:205:16 | command [post update] |
+| test.cpp:188:11:188:17 | command [post update] | test.cpp:188:11:188:17 | command [post update] |
+| test.cpp:188:11:188:17 | command [post update] | test.cpp:188:11:188:17 | command [post update] |
+| test.cpp:188:11:188:17 | command [post update] | test.cpp:188:11:188:17 | command [post update] |
+| test.cpp:188:11:188:17 | strncat output argument | test.cpp:188:11:188:17 | command [post update] |
+| test.cpp:188:11:188:17 | strncat output argument | test.cpp:188:11:188:17 | command [post update] |
 | test.cpp:188:11:188:17 | strncat output argument | test.cpp:188:11:188:17 | command [post update] |
 | test.cpp:188:11:188:17 | strncat output argument | test.cpp:188:11:188:17 | command [post update] |
 | test.cpp:188:20:188:24 | flags indirection | test.cpp:188:11:188:17 | strncat output argument |
@@ -67,9 +58,13 @@ edges
 | test.cpp:194:9:194:16 | fread output argument | test.cpp:196:26:196:33 | filename |
 | test.cpp:194:9:194:16 | fread output argument | test.cpp:196:26:196:33 | filename indirection |
 | test.cpp:196:10:196:16 | command [post update] | test.cpp:198:32:198:38 | command indirection |
+| test.cpp:196:10:196:16 | command [post update] | test.cpp:198:32:198:38 | command indirection |
 | test.cpp:196:26:196:33 | filename | test.cpp:186:47:186:54 | filename |
+| test.cpp:196:26:196:33 | filename | test.cpp:196:10:196:16 | command [post update] |
+| test.cpp:196:26:196:33 | filename | test.cpp:196:10:196:16 | command [post update] |
 | test.cpp:196:26:196:33 | filename indirection | test.cpp:186:47:186:54 | *filename |
-| test.cpp:205:10:205:16 | command [post update] | test.cpp:207:32:207:38 | command indirection |
+| test.cpp:196:26:196:33 | filename indirection | test.cpp:196:10:196:16 | command [post update] |
+| test.cpp:196:26:196:33 | filename indirection | test.cpp:196:10:196:16 | command [post update] |
 nodes
 | test.cpp:16:20:16:23 | argv | semmle.label | argv |
 | test.cpp:22:13:22:20 | sprintf output argument | semmle.label | sprintf output argument |
@@ -115,22 +110,42 @@ nodes
 | test.cpp:180:13:180:19 | strncat output argument | semmle.label | strncat output argument |
 | test.cpp:180:22:180:29 | filename indirection | semmle.label | filename indirection |
 | test.cpp:183:32:183:38 | command indirection | semmle.label | command indirection |
+| test.cpp:183:32:183:38 | command indirection | semmle.label | command indirection |
+| test.cpp:183:32:183:38 | command indirection | semmle.label | command indirection |
 | test.cpp:186:47:186:54 | *filename | semmle.label | *filename |
 | test.cpp:186:47:186:54 | filename | semmle.label | filename |
 | test.cpp:187:11:187:15 | strncat output argument | semmle.label | strncat output argument |
+| test.cpp:187:11:187:15 | strncat output argument | semmle.label | strncat output argument |
+| test.cpp:187:18:187:25 | filename indirection | semmle.label | filename indirection |
 | test.cpp:187:18:187:25 | filename indirection | semmle.label | filename indirection |
 | test.cpp:188:11:188:17 | command [post update] | semmle.label | command [post update] |
 | test.cpp:188:11:188:17 | command [post update] | semmle.label | command [post update] |
+| test.cpp:188:11:188:17 | command [post update] | semmle.label | command [post update] |
+| test.cpp:188:11:188:17 | command [post update] | semmle.label | command [post update] |
+| test.cpp:188:11:188:17 | command [post update] | semmle.label | command [post update] |
+| test.cpp:188:11:188:17 | command [post update] | semmle.label | command [post update] |
+| test.cpp:188:11:188:17 | command [post update] | semmle.label | command [post update] |
+| test.cpp:188:11:188:17 | command [post update] | semmle.label | command [post update] |
+| test.cpp:188:11:188:17 | strncat output argument | semmle.label | strncat output argument |
 | test.cpp:188:11:188:17 | strncat output argument | semmle.label | strncat output argument |
 | test.cpp:188:20:188:24 | flags indirection | semmle.label | flags indirection |
+| test.cpp:188:20:188:24 | flags indirection | semmle.label | flags indirection |
 | test.cpp:194:9:194:16 | fread output argument | semmle.label | fread output argument |
 | test.cpp:196:10:196:16 | command [post update] | semmle.label | command [post update] |
+| test.cpp:196:10:196:16 | command [post update] | semmle.label | command [post update] |
 | test.cpp:196:26:196:33 | filename | semmle.label | filename |
 | test.cpp:196:26:196:33 | filename indirection | semmle.label | filename indirection |
 | test.cpp:198:32:198:38 | command indirection | semmle.label | command indirection |
-| test.cpp:205:10:205:16 | command [post update] | semmle.label | command [post update] |
-| test.cpp:207:32:207:38 | command indirection | semmle.label | command indirection |
+| test.cpp:198:32:198:38 | command indirection | semmle.label | command indirection |
 subpaths
+| test.cpp:196:26:196:33 | filename | test.cpp:186:47:186:54 | filename | test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
+| test.cpp:196:26:196:33 | filename | test.cpp:186:47:186:54 | filename | test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
+| test.cpp:196:26:196:33 | filename | test.cpp:186:47:186:54 | filename | test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
+| test.cpp:196:26:196:33 | filename | test.cpp:186:47:186:54 | filename | test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
+| test.cpp:196:26:196:33 | filename indirection | test.cpp:186:47:186:54 | *filename | test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
+| test.cpp:196:26:196:33 | filename indirection | test.cpp:186:47:186:54 | *filename | test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
+| test.cpp:196:26:196:33 | filename indirection | test.cpp:186:47:186:54 | *filename | test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
+| test.cpp:196:26:196:33 | filename indirection | test.cpp:186:47:186:54 | *filename | test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
 #select
 | test.cpp:23:12:23:19 | command1 | test.cpp:16:20:16:23 | argv | test.cpp:23:12:23:19 | command1 indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string) | test.cpp:16:20:16:23 | argv | user input (a command-line argument) | test.cpp:22:13:22:20 | sprintf output argument | sprintf output argument |
 | test.cpp:51:10:51:16 | command | test.cpp:47:21:47:26 | call to getenv | test.cpp:51:10:51:16 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string) | test.cpp:47:21:47:26 | call to getenv | user input (an environment variable) | test.cpp:50:11:50:17 | sprintf output argument | sprintf output argument |
@@ -146,5 +161,3 @@ subpaths
 | test.cpp:183:32:183:38 | command | test.cpp:174:9:174:16 | fread output argument | test.cpp:183:32:183:38 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl | test.cpp:174:9:174:16 | fread output argument | user input (String read by fread) | test.cpp:180:13:180:19 | strncat output argument | strncat output argument |
 | test.cpp:198:32:198:38 | command | test.cpp:194:9:194:16 | fread output argument | test.cpp:198:32:198:38 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl | test.cpp:194:9:194:16 | fread output argument | user input (String read by fread) | test.cpp:187:11:187:15 | strncat output argument | strncat output argument |
 | test.cpp:198:32:198:38 | command | test.cpp:194:9:194:16 | fread output argument | test.cpp:198:32:198:38 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl | test.cpp:194:9:194:16 | fread output argument | user input (String read by fread) | test.cpp:188:11:188:17 | strncat output argument | strncat output argument |
-| test.cpp:207:32:207:38 | command | test.cpp:194:9:194:16 | fread output argument | test.cpp:207:32:207:38 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl | test.cpp:194:9:194:16 | fread output argument | user input (String read by fread) | test.cpp:187:11:187:15 | strncat output argument | strncat output argument |
-| test.cpp:207:32:207:38 | command | test.cpp:194:9:194:16 | fread output argument | test.cpp:207:32:207:38 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl | test.cpp:194:9:194:16 | fread output argument | user input (String read by fread) | test.cpp:188:11:188:17 | strncat output argument | strncat output argument |
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-078/semmle/ExecTainted/test.cpp
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-078/semmle/ExecTainted/test.cpp
@@ -199,7 +199,7 @@ void test17(FILE *f) {
 }

 void test18() {
-  // GOOD [FALSE POSITIVE]
+  // GOOD
  char command[1000] = "ls ", flags[1000] = "-l", filename[1000] = ".";

  concat(command, flags, filename);