Merge pull request #11254 from MathiasVP/fix-ssa-flow

C++: Fix spurious reference flow
2026-04-30 19:26:02 +02:00 · 2022-11-23 09:52:28 +00:00
parent cfb8f282d1 fc3d6a1847
commit 349c5cd800
14 changed files with 73 additions and 87 deletions
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
@@ -235,7 +235,13 @@ class Node extends TIRDataFlowNode {
  Expr asIndirectArgument() { result = this.asIndirectArgument(_) }

  /** Gets the positional parameter corresponding to this node, if any. */
-  Parameter asParameter() { result = this.asParameter(0) }
+  Parameter asParameter() {
+    exists(int indirectionIndex | result = this.asParameter(indirectionIndex) |
+      if result.getUnspecifiedType() instanceof ReferenceType
+      then indirectionIndex = 1
+      else indirectionIndex = 0
+    )
+  }

  /**
   * Gets the uninitialized local variable corresponding to this node, if
@@ -1233,35 +1239,11 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
  )
 }

-pragma[noinline]
-private predicate getAddressType(LoadInstruction load, Type t) {
-  exists(Instruction address |
-    address = load.getSourceAddress() and
-    t = address.getResultType()
-  )
-}
-
-/**
- * Like the AST dataflow library, we want to conflate the address and value of a reference. This class
- * represents the `LoadInstruction` that is generated from a reference dereference.
- */
-private class ReferenceDereferenceInstruction extends LoadInstruction {
-  ReferenceDereferenceInstruction() {
-    exists(ReferenceType ref |
-      getAddressType(this, ref) and
-      this.getResultType() = ref.getBaseType()
-    )
-  }
-}
-
 private predicate simpleInstructionLocalFlowStep(Operand opFrom, Instruction iTo) {
  // Treat all conversions as flow, even conversions between different numeric types.
  conversionFlow(opFrom, iTo, false)
  or
  iTo.(CopyInstruction).getSourceValueOperand() = opFrom
-  or
-  // Conflate references and values like in AST dataflow.
-  iTo.(ReferenceDereferenceInstruction).getSourceAddressOperand() = opFrom
 }

 private predicate simpleOperandLocalFlowStep(Instruction iFrom, Operand opTo) {
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
@@ -275,10 +275,10 @@ predicate adjacentDefRead(DefOrUse defOrUse1, UseOrPhi use) {
  exists(IRBlock bb1, int i1, SourceVariable v |
    defOrUse1.asDefOrUse().hasIndexInBlock(bb1, i1, v)
  |
-    exists(IRBlock bb2, int i2 |
-      adjacentDefRead(_, pragma[only_bind_into](bb1), pragma[only_bind_into](i1),
-        pragma[only_bind_into](bb2), pragma[only_bind_into](i2))
-    |
+    exists(IRBlock bb2, int i2, Definition def |
+      adjacentDefRead(pragma[only_bind_into](def), pragma[only_bind_into](bb1),
+        pragma[only_bind_into](i1), pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) and
+      def.getSourceVariable() = v and
      use.asDefOrUse().(UseImpl).hasIndexInBlock(bb2, i2, v)
    )
    or
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
@@ -197,11 +197,11 @@ private module Cached {
  predicate isUse(boolean certain, Operand op, Instruction base, int ind, int indirectionIndex) {
    not ignoreOperand(op) and
    certain = true and
-    exists(LanguageType type, int m, int ind0 |
+    exists(LanguageType type, int upper, int ind0 |
      type = getLanguageType(op) and
-      m = countIndirectionsForCppType(type) and
+      upper = countIndirectionsForCppType(type) and
      isUseImpl(op, base, ind0) and
-      ind = ind0 + [0 .. m] and
+      ind = ind0 + [0 .. upper] and
      indirectionIndex = ind - ind0
    )
  }
@@ -277,7 +277,7 @@ private module Cached {
    int indirectionIndex
  ) {
    certain = true and
-    exists(int ind0, CppType type, int m |
+    exists(int ind0, CppType type, int lower, int upper |
      address =
        [
          instr.(StoreInstruction).getDestinationAddressOperand(),
@@ -288,9 +288,10 @@ private module Cached {
    |
      isDefImpl(address, base, ind0) and
      type = getLanguageType(address) and
-      m = countIndirectionsForCppType(type) and
-      ind = ind0 + [1 .. m] and
-      indirectionIndex = ind - (ind0 + 1)
+      upper = countIndirectionsForCppType(type) and
+      ind = ind0 + [lower .. upper] and
+      indirectionIndex = ind - (ind0 + lower) and
+      if type.hasType(any(Cpp::ArrayType arrayType), true) then lower = 0 else lower = 1
    )
  }

--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected
@@ -85,11 +85,11 @@ postWithInFlow
 | test.cpp:407:10:407:13 | ref arg & ... | PostUpdateNode should not be the target of local flow. |
 | test.cpp:407:11:407:13 | tmp [inner post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:423:21:423:25 | local [inner post update] | PostUpdateNode should not be the target of local flow. |
-| test.cpp:436:19:436:23 | local [inner post update] | PostUpdateNode should not be the target of local flow. |
-| test.cpp:465:3:465:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
-| test.cpp:465:4:465:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
-| test.cpp:470:22:470:22 | x [inner post update] | PostUpdateNode should not be the target of local flow. |
-| test.cpp:499:3:499:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
-| test.cpp:499:4:499:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
-| test.cpp:505:35:505:35 | x [inner post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:441:19:441:23 | local [inner post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:472:3:472:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:472:4:472:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:477:22:477:22 | x [inner post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:506:3:506:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:506:4:506:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:512:35:512:35 | x [inner post update] | PostUpdateNode should not be the target of local flow. |
 viableImplInCallContextTooLarge
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
@@ -71,10 +71,10 @@
 | test.cpp:391:11:391:13 | tmp | test.cpp:391:10:391:13 | & ... |
 | test.cpp:391:17:391:23 | source1 | test.cpp:391:10:391:13 | ref arg & ... |
 | test.cpp:391:17:391:23 | source1 | test.cpp:391:16:391:23 | & ... |
-| test.cpp:480:67:480:67 | s | test.cpp:480:67:480:67 | s |
-| test.cpp:480:67:480:67 | s | test.cpp:481:21:481:21 | s |
-| test.cpp:480:67:480:67 | s | test.cpp:482:20:482:20 | s |
-| test.cpp:481:21:481:21 | s [post update] | test.cpp:480:67:480:67 | s |
-| test.cpp:481:21:481:21 | s [post update] | test.cpp:482:20:482:20 | s |
-| test.cpp:481:24:481:30 | ref arg content | test.cpp:482:23:482:29 | content |
-| test.cpp:482:23:482:29 | content | test.cpp:483:9:483:17 | p_content |
+| test.cpp:487:67:487:67 | s | test.cpp:487:67:487:67 | s |
+| test.cpp:487:67:487:67 | s | test.cpp:488:21:488:21 | s |
+| test.cpp:487:67:487:67 | s | test.cpp:489:20:489:20 | s |
+| test.cpp:488:21:488:21 | s [post update] | test.cpp:487:67:487:67 | s |
+| test.cpp:488:21:488:21 | s [post update] | test.cpp:489:20:489:20 | s |
+| test.cpp:488:24:488:30 | ref arg content | test.cpp:489:23:489:29 | content |
+| test.cpp:489:23:489:29 | content | test.cpp:490:9:490:17 | p_content |
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/ref.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/ref.cpp
@@ -56,13 +56,13 @@ namespace withoutFields {
    sink(x1); // $ ast,ir=55:23 SPURIOUS: ast,ir=53:9

    notAssign(x2, source());
-    sink(x2); // $ SPURIOUS: ast ir=53:13 ir=58:19
+    sink(x2); // $ SPURIOUS: ast ir

    sourceToParamWrapper(x3);
    sink(x3); // $ ast,ir=29:11 SPURIOUS: ast,ir=53:17

    notSource(x4);
-    sink(x4); // $ SPURIOUS: ast ir=44:11 ir=53:21
+    sink(x4); // $ SPURIOUS: ast ir
  }
 }

--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
@@ -89,7 +89,7 @@ void trackUninitialized() {
 void local_references(int &source1, int clean1) {
  sink(source1); // $ ast,ir
  source1 = clean1;
-  sink(source1); // $ SPURIOUS: ir
+  sink(source1); // clean

  // The next two test cases show that the analysis does not understand the "&"
  // on the type at all. It does not understand that the initialization creates
@@ -423,25 +423,32 @@ void intPointerSourceCaller() {
  intPointerSource(&local);
  sink(local); // $ ast,ir=422:7 ast,ir=423:20
 }
-
+// The IR results for this test _are_ equivalent to the AST ones.
+// The IR annotation is just "ir" because the sink of the unitialized source at
+// `433:7` (i.e., the declaration `int local[1];`) is value of `local`, but the sink
+// of the source from `intPointerSource` value of `*local` (i.e., the indirection node
+// of `local`). So unlike AST dataflow, each of these two sinks correspond to a unique
+// source, and thus we don't need to attach a location annotation to it.
 void intPointerSourceCaller2() {
  int local[1];
  intPointerSource(local);
-  sink(local); // $ ast,ir=428:7 ast,ir=429:20
-  sink(*local); // $ ast,ir=428:7 ast,ir=429:20
+  sink(local); // $ ast=433:7 ast=434:20 ir
+  sink(*local); // $ ast=433:7 ast=434:20 ir
 }

 void intArraySourceCaller() {
  int local;
  intArraySource(&local, 1);
-  sink(local); // $ ast,ir=435:7 ast,ir=436:18
+  sink(local); // $ ast,ir=440:7 ast,ir=441:18
 }

+// The IR results for this test _are_ equivalent to the AST ones.
+// See the comment on `intPointerSourceCaller2` for an explanation.
 void intArraySourceCaller2() {
  int local[2];
  intArraySource(local, 2);
-  sink(local); // $ ast,ir=441:7 ast,ir=442:18
-  sink(*local); // $ ast,ir=441:7 ast,ir=442:18
+  sink(local); // $ ast=448:7 ast=449:18 ir
+  sink(*local); // $ ast=448:7 ast=449:18 ir
 }

 ///////////////////////////////////////////////////////////////////////////////
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
@@ -26,11 +26,11 @@
 | test.cpp:416:7:416:11 | local | test.cpp:418:8:418:12 | local |
 | test.cpp:422:7:422:11 | local | test.cpp:423:21:423:25 | local |
 | test.cpp:422:7:422:11 | local | test.cpp:424:8:424:12 | local |
-| test.cpp:428:7:428:11 | local | test.cpp:429:20:429:24 | local |
-| test.cpp:428:7:428:11 | local | test.cpp:430:8:430:12 | local |
-| test.cpp:428:7:428:11 | local | test.cpp:431:9:431:13 | local |
-| test.cpp:435:7:435:11 | local | test.cpp:436:19:436:23 | local |
-| test.cpp:435:7:435:11 | local | test.cpp:437:8:437:12 | local |
-| test.cpp:441:7:441:11 | local | test.cpp:442:18:442:22 | local |
-| test.cpp:441:7:441:11 | local | test.cpp:443:8:443:12 | local |
-| test.cpp:441:7:441:11 | local | test.cpp:444:9:444:13 | local |
+| test.cpp:433:7:433:11 | local | test.cpp:434:20:434:24 | local |
+| test.cpp:433:7:433:11 | local | test.cpp:435:8:435:12 | local |
+| test.cpp:433:7:433:11 | local | test.cpp:436:9:436:13 | local |
+| test.cpp:440:7:440:11 | local | test.cpp:441:19:441:23 | local |
+| test.cpp:440:7:440:11 | local | test.cpp:442:8:442:12 | local |
+| test.cpp:448:7:448:11 | local | test.cpp:449:18:449:22 | local |
+| test.cpp:448:7:448:11 | local | test.cpp:450:8:450:12 | local |
+| test.cpp:448:7:448:11 | local | test.cpp:451:9:451:13 | local |
--- a/cpp/ql/test/library-tests/dataflow/fields/A.cpp
+++ b/cpp/ql/test/library-tests/dataflow/fields/A.cpp
@@ -54,7 +54,7 @@ public:
    B *b = new B();
    b->set(new C1());
    sink(b->get());                // $ ast ir=55:12
-    sink((new B(new C()))->get()); // $ ast ir=57:17
+    sink((new B(new C()))->get()); // $ ast ir
  }

  void f3()
@@ -129,7 +129,7 @@ public:
  {
    B *b = new B();
    f7(b);
-    sink(b->c); // $ ast ir=126:12
+    sink(b->c); // $ ast ir
  }

  class D
--- a/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected
+++ b/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected
@@ -35,7 +35,6 @@ edges
 | A.cpp:57:11:57:24 | new indirection [c] | A.cpp:57:11:57:24 | new indirection [c] |
 | A.cpp:57:17:57:23 | new | A.cpp:23:10:23:10 | c |
 | A.cpp:57:17:57:23 | new | A.cpp:57:11:57:24 | call to B [c] |
-| A.cpp:57:17:57:23 | new | A.cpp:57:17:57:23 | new |
 | A.cpp:64:10:64:15 | call to setOnB indirection [c] | A.cpp:66:10:66:11 | Load indirection [c] |
 | A.cpp:64:21:64:28 | new | A.cpp:64:10:64:15 | call to setOnB indirection [c] |
 | A.cpp:64:21:64:28 | new | A.cpp:64:21:64:28 | new |
@@ -77,7 +76,6 @@ edges
 | A.cpp:126:5:126:5 | set output argument [c] | A.cpp:131:8:131:8 | f7 output argument [c] |
 | A.cpp:126:12:126:18 | new | A.cpp:27:17:27:17 | c |
 | A.cpp:126:12:126:18 | new | A.cpp:126:5:126:5 | set output argument [c] |
-| A.cpp:126:12:126:18 | new | A.cpp:126:12:126:18 | new |
 | A.cpp:131:8:131:8 | f7 output argument [c] | A.cpp:132:10:132:10 | Load indirection [c] |
 | A.cpp:132:10:132:10 | Load indirection [c] | A.cpp:132:10:132:13 | c |
 | A.cpp:132:10:132:10 | Load indirection [c] | A.cpp:132:13:132:13 | FieldAddress indirection |
@@ -924,7 +922,6 @@ nodes
 | A.cpp:57:11:57:24 | call to B [c] | semmle.label | call to B [c] |
 | A.cpp:57:11:57:24 | new indirection [c] | semmle.label | new indirection [c] |
 | A.cpp:57:17:57:23 | new | semmle.label | new |
-| A.cpp:57:17:57:23 | new | semmle.label | new |
 | A.cpp:64:10:64:15 | call to setOnB indirection [c] | semmle.label | call to setOnB indirection [c] |
 | A.cpp:64:21:64:28 | new | semmle.label | new |
 | A.cpp:64:21:64:28 | new | semmle.label | new |
@@ -962,7 +959,6 @@ nodes
 | A.cpp:120:16:120:16 | a | semmle.label | a |
 | A.cpp:126:5:126:5 | set output argument [c] | semmle.label | set output argument [c] |
 | A.cpp:126:12:126:18 | new | semmle.label | new |
-| A.cpp:126:12:126:18 | new | semmle.label | new |
 | A.cpp:131:8:131:8 | f7 output argument [c] | semmle.label | f7 output argument [c] |
 | A.cpp:132:10:132:10 | Load indirection [c] | semmle.label | Load indirection [c] |
 | A.cpp:132:10:132:13 | c | semmle.label | c |
@@ -1772,7 +1768,6 @@ subpaths
 | A.cpp:56:10:56:17 | call to get | A.cpp:55:12:55:19 | new | A.cpp:56:10:56:17 | call to get | call to get flows from $@ | A.cpp:55:12:55:19 | new | new |
 | A.cpp:56:10:56:17 | call to get | A.cpp:55:12:55:19 | new | A.cpp:56:10:56:17 | call to get | call to get flows from $@ | A.cpp:55:12:55:19 | new | new |
 | A.cpp:57:10:57:32 | call to get | A.cpp:57:17:57:23 | new | A.cpp:57:10:57:32 | call to get | call to get flows from $@ | A.cpp:57:17:57:23 | new | new |
-| A.cpp:57:10:57:32 | call to get | A.cpp:57:17:57:23 | new | A.cpp:57:10:57:32 | call to get | call to get flows from $@ | A.cpp:57:17:57:23 | new | new |
 | A.cpp:66:10:66:14 | c | A.cpp:64:21:64:28 | new | A.cpp:66:10:66:14 | c | c flows from $@ | A.cpp:64:21:64:28 | new | new |
 | A.cpp:66:10:66:14 | c | A.cpp:64:21:64:28 | new | A.cpp:66:10:66:14 | c | c flows from $@ | A.cpp:64:21:64:28 | new | new |
 | A.cpp:66:14:66:14 | c | A.cpp:64:21:64:28 | new | A.cpp:66:14:66:14 | c | c flows from $@ | A.cpp:64:21:64:28 | new | new |
@@ -1786,8 +1781,6 @@ subpaths
 | A.cpp:120:12:120:16 | a | A.cpp:98:12:98:18 | new | A.cpp:120:12:120:16 | a | a flows from $@ | A.cpp:98:12:98:18 | new | new |
 | A.cpp:120:16:120:16 | a | A.cpp:98:12:98:18 | new | A.cpp:120:16:120:16 | a | a flows from $@ | A.cpp:98:12:98:18 | new | new |
 | A.cpp:132:10:132:13 | c | A.cpp:126:12:126:18 | new | A.cpp:132:10:132:13 | c | c flows from $@ | A.cpp:126:12:126:18 | new | new |
-| A.cpp:132:10:132:13 | c | A.cpp:126:12:126:18 | new | A.cpp:132:10:132:13 | c | c flows from $@ | A.cpp:126:12:126:18 | new | new |
-| A.cpp:132:13:132:13 | c | A.cpp:126:12:126:18 | new | A.cpp:132:13:132:13 | c | c flows from $@ | A.cpp:126:12:126:18 | new | new |
 | A.cpp:132:13:132:13 | c | A.cpp:126:12:126:18 | new | A.cpp:132:13:132:13 | c | c flows from $@ | A.cpp:126:12:126:18 | new | new |
 | A.cpp:152:10:152:13 | b | A.cpp:143:25:143:31 | new | A.cpp:152:10:152:13 | b | b flows from $@ | A.cpp:143:25:143:31 | new | new |
 | A.cpp:152:10:152:13 | b | A.cpp:150:12:150:18 | new | A.cpp:152:10:152:13 | b | b flows from $@ | A.cpp:150:12:150:18 | new | new |
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/SAMATE/ArithmeticTainted.expected
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/SAMATE/ArithmeticTainted.expected
@@ -1,14 +1,11 @@
 edges
 | examples.cpp:63:26:63:30 | & ... | examples.cpp:66:11:66:14 | data |
 | examples.cpp:63:26:63:30 | & ... | examples.cpp:66:11:66:14 | data |
-| examples.cpp:63:26:63:30 | & ... | examples.cpp:66:11:66:14 | data |
-| examples.cpp:63:26:63:30 | & ... | examples.cpp:66:11:66:14 | data |
 | examples.cpp:63:26:63:30 | fscanf output argument | examples.cpp:66:11:66:14 | data |
 | examples.cpp:63:26:63:30 | fscanf output argument | examples.cpp:66:11:66:14 | data |
 subpaths
 nodes
 | examples.cpp:63:26:63:30 | & ... | semmle.label | & ... |
-| examples.cpp:63:26:63:30 | & ... | semmle.label | & ... |
 | examples.cpp:63:26:63:30 | fscanf output argument | semmle.label | fscanf output argument |
 | examples.cpp:66:11:66:14 | data | semmle.label | data |
 | examples.cpp:66:11:66:14 | data | semmle.label | data |
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/ArithmeticTainted.expected
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/ArithmeticTainted.expected
@@ -2,7 +2,6 @@ edges
 | test2.cpp:12:21:12:21 | v | test2.cpp:14:11:14:11 | v |
 | test2.cpp:12:21:12:21 | v | test2.cpp:14:11:14:11 | v |
 | test2.cpp:25:22:25:23 | & ... | test2.cpp:27:13:27:13 | v |
-| test2.cpp:25:22:25:23 | & ... | test2.cpp:27:13:27:13 | v |
 | test2.cpp:25:22:25:23 | fscanf output argument | test2.cpp:27:13:27:13 | v |
 | test2.cpp:27:13:27:13 | v | test2.cpp:12:21:12:21 | v |
 | test5.cpp:5:5:5:17 | VariableAddress indirection | test5.cpp:17:6:17:18 | call to getTaintedInt |
@@ -34,7 +33,6 @@ nodes
 | test2.cpp:14:11:14:11 | v | semmle.label | v |
 | test2.cpp:14:11:14:11 | v | semmle.label | v |
 | test2.cpp:25:22:25:23 | & ... | semmle.label | & ... |
-| test2.cpp:25:22:25:23 | & ... | semmle.label | & ... |
 | test2.cpp:25:22:25:23 | fscanf output argument | semmle.label | fscanf output argument |
 | test2.cpp:27:13:27:13 | v | semmle.label | v |
 | test5.cpp:5:5:5:17 | VariableAddress indirection | semmle.label | VariableAddress indirection |
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/CleartextTransmission.expected
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/CleartextTransmission.expected
@@ -32,12 +32,16 @@ edges
 | test3.cpp:138:24:138:32 | password1 indirection | test3.cpp:138:24:138:32 | password1 |
 | test3.cpp:138:24:138:32 | password1 indirection | test3.cpp:138:24:138:32 | password1 |
 | test3.cpp:144:16:144:29 | call to get_global_str indirection | test3.cpp:146:15:146:18 | data |
+| test3.cpp:157:19:157:26 | password indirection | test3.cpp:159:15:159:20 | buffer |
+| test3.cpp:157:19:157:26 | password indirection | test3.cpp:159:15:159:20 | buffer |
 | test3.cpp:173:15:173:22 | password indirection | test3.cpp:173:15:173:22 | password |
 | test3.cpp:181:15:181:22 | password indirection | test3.cpp:181:15:181:22 | password |
 | test3.cpp:191:15:191:22 | password indirection | test3.cpp:191:15:191:22 | password |
 | test3.cpp:199:19:199:26 | password indirection | test3.cpp:201:15:201:22 | password |
 | test3.cpp:201:15:201:22 | password indirection | test3.cpp:201:15:201:22 | password |
 | test3.cpp:201:32:201:39 | password indirection | test3.cpp:201:15:201:22 | password |
+| test3.cpp:207:19:207:26 | password indirection | test3.cpp:210:15:210:22 | password |
+| test3.cpp:208:3:208:10 | password indirection | test3.cpp:210:15:210:22 | password |
 | test3.cpp:210:15:210:22 | password indirection | test3.cpp:210:15:210:22 | password |
 | test3.cpp:210:32:210:39 | password indirection | test3.cpp:210:15:210:22 | password |
 | test3.cpp:219:15:219:26 | password_ptr indirection | test3.cpp:219:15:219:26 | password_ptr |
@@ -152,6 +156,9 @@ nodes
 | test3.cpp:140:15:140:17 | ptr | semmle.label | ptr |
 | test3.cpp:144:16:144:29 | call to get_global_str indirection | semmle.label | call to get_global_str indirection |
 | test3.cpp:146:15:146:18 | data | semmle.label | data |
+| test3.cpp:157:19:157:26 | password indirection | semmle.label | password indirection |
+| test3.cpp:157:19:157:26 | password indirection | semmle.label | password indirection |
+| test3.cpp:159:15:159:20 | buffer | semmle.label | buffer |
 | test3.cpp:173:15:173:22 | password | semmle.label | password |
 | test3.cpp:173:15:173:22 | password indirection | semmle.label | password indirection |
 | test3.cpp:181:15:181:22 | password | semmle.label | password |
@@ -162,6 +169,8 @@ nodes
 | test3.cpp:201:15:201:22 | password | semmle.label | password |
 | test3.cpp:201:15:201:22 | password indirection | semmle.label | password indirection |
 | test3.cpp:201:32:201:39 | password indirection | semmle.label | password indirection |
+| test3.cpp:207:19:207:26 | password indirection | semmle.label | password indirection |
+| test3.cpp:208:3:208:10 | password indirection | semmle.label | password indirection |
 | test3.cpp:210:15:210:22 | password | semmle.label | password |
 | test3.cpp:210:15:210:22 | password indirection | semmle.label | password indirection |
 | test3.cpp:210:32:210:39 | password indirection | semmle.label | password indirection |
@@ -285,9 +294,8 @@ subpaths
 | test3.cpp:140:3:140:6 | call to send | test3.cpp:138:24:138:32 | password1 indirection | test3.cpp:140:15:140:17 | ptr | This operation transmits 'ptr', which may contain unencrypted sensitive data from $@. | test3.cpp:138:24:138:32 | password1 indirection | password1 indirection |
 | test3.cpp:140:3:140:6 | call to send | test3.cpp:138:24:138:32 | password1 indirection | test3.cpp:140:15:140:17 | ptr | This operation transmits 'ptr', which may contain unencrypted sensitive data from $@. | test3.cpp:138:24:138:32 | password1 indirection | password1 indirection |
 | test3.cpp:146:3:146:6 | call to send | test3.cpp:126:9:126:23 | global_password indirection | test3.cpp:146:15:146:18 | data | This operation transmits 'data', which may contain unencrypted sensitive data from $@. | test3.cpp:126:9:126:23 | global_password indirection | global_password indirection |
-| test3.cpp:181:3:181:6 | call to recv | test3.cpp:181:15:181:22 | password indirection | test3.cpp:181:15:181:22 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@. | test3.cpp:181:15:181:22 | password indirection | password indirection |
-| test3.cpp:210:3:210:6 | call to send | test3.cpp:210:15:210:22 | password indirection | test3.cpp:210:15:210:22 | password | This operation transmits 'password', which may contain unencrypted sensitive data from $@. | test3.cpp:210:15:210:22 | password indirection | password indirection |
-| test3.cpp:210:3:210:6 | call to send | test3.cpp:210:32:210:39 | password indirection | test3.cpp:210:15:210:22 | password | This operation transmits 'password', which may contain unencrypted sensitive data from $@. | test3.cpp:210:32:210:39 | password indirection | password indirection |
+| test3.cpp:159:3:159:6 | call to send | test3.cpp:157:19:157:26 | password indirection | test3.cpp:159:15:159:20 | buffer | This operation transmits 'buffer', which may contain unencrypted sensitive data from $@. | test3.cpp:157:19:157:26 | password indirection | password indirection |
+| test3.cpp:159:3:159:6 | call to send | test3.cpp:157:19:157:26 | password indirection | test3.cpp:159:15:159:20 | buffer | This operation transmits 'buffer', which may contain unencrypted sensitive data from $@. | test3.cpp:157:19:157:26 | password indirection | password indirection |
 | test3.cpp:228:2:228:5 | call to send | test3.cpp:227:22:227:29 | password indirection | test3.cpp:228:26:228:33 | password | This operation transmits 'password', which may contain unencrypted sensitive data from $@. | test3.cpp:227:22:227:29 | password indirection | password indirection |
 | test3.cpp:228:2:228:5 | call to send | test3.cpp:228:26:228:33 | password indirection | test3.cpp:228:26:228:33 | password | This operation transmits 'password', which may contain unencrypted sensitive data from $@. | test3.cpp:228:26:228:33 | password indirection | password indirection |
 | test3.cpp:241:2:241:6 | call to fgets | test3.cpp:241:8:241:15 | password indirection | test3.cpp:241:8:241:15 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@. | test3.cpp:241:8:241:15 | password indirection | password indirection |
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/test3.cpp
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/test3.cpp
@@ -156,7 +156,7 @@ void test_taint(const char *password)

 		strncpy(buffer, password, 16);
 		buffer[15] = 0;
-		send(val(), buffer, 16, val()); // BAD: `password` is (partially) sent plaintext [NOT DETECTED]
+		send(val(), buffer, 16, val()); // BAD: `password` is (partially) sent plaintext
 	}
 }

@@ -178,7 +178,7 @@ void test_decrypt()
 	{
 		char password[256];

-		recv(val(), password, 256, val()); // GOOD: password is encrypted [FALSE POSITIVE]
+		recv(val(), password, 256, val()); // GOOD: password is encrypted
 		password[255] = 0;

 		decrypt_inplace(password); // proof that `password` was in fact encrypted
@@ -207,7 +207,7 @@ void test_decrypt()
 		encrypt_inplace(password); // proof that `password` is in fact encrypted
 		password[255] = 0;

-		send(val(), password, strlen(password), val()); // GOOD: password is encrypted [FALSE POSITIVE]
+		send(val(), password, strlen(password), val()); // GOOD: password is encrypted
 	}

 	{