Merge pull request #16440 from MathiasVP/fix-iterator-to-expired-container-fp-2

C++: Fix location of SSA def for local variable addresses

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

@@ -9,6 +9,7 @@ private import semmle.code.cpp.models.interfaces.PartialFlow as PartialFlow
 private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs as FIO
 private import semmle.code.cpp.ir.internal.IRCppLanguage
 private import semmle.code.cpp.ir.dataflow.internal.ModelUtil
+private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedInitialization
 private import DataFlowPrivate
 import SsaInternalsCommon
 
@@ -329,6 +330,17 @@ private predicate sourceVariableHasBaseAndIndex(SourceVariable v, BaseSourceVari
   v.getIndirection() = ind
 }
 
+/**
+ * Gets the instruction that computes the address that's used to
+ * initialize `v`.
+ */
+private Instruction getInitializationTargetAddress(IRVariable v) {
+  exists(TranslatedVariableInitialization init |
+    init.getIRVariable() = v and
+    result = init.getTargetAddress()
+  )
+}
+
 /** An initial definition of an `IRVariable`'s address. */
 private class DefAddressImpl extends DefImpl, TDefAddressImpl {
   BaseIRVariable v;
@@ -347,8 +359,15 @@ private class DefAddressImpl extends DefImpl, TDefAddressImpl {
   final override Node0Impl getValue() { none() }
 
   final override predicate hasIndexInBlock(IRBlock block, int index) {
-    block = v.getIRVariable().getEnclosingIRFunction().getEntryBlock() and
-    index = 0
+    exists(IRVariable var | var = v.getIRVariable() |
+      block.getInstruction(index) = getInitializationTargetAddress(var)
+      or
+      // If there is no translatated element that does initialization of the
+      // variable we place the SSA definition at the entry block of the function.
+      not exists(getInitializationTargetAddress(var)) and
+      block = var.getEnclosingIRFunction().getEntryBlock() and
+      index = 0
+    )
   }
 
   override Cpp::Location getLocation() { result = v.getIRVariable().getLocation() }

cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/IteratorToExpiredContainer/IteratorToExpiredContainer.expected

@@ -1,7 +1,5 @@
 | test.cpp:680:30:680:30 | call to operator[] | This object is destroyed at the end of the full-expression. |
 | test.cpp:683:31:683:32 | call to at | This object is destroyed at the end of the full-expression. |
-| test.cpp:689:46:689:58 | pointer to ~vector output argument | This object is destroyed at the end of the full-expression. |
 | test.cpp:702:27:702:27 | call to operator[] | This object is destroyed at the end of the full-expression. |
 | test.cpp:727:23:727:23 | call to operator[] | This object is destroyed at the end of the full-expression. |
 | test.cpp:735:23:735:23 | call to operator[] | This object is destroyed at the end of the full-expression. |
-| test.cpp:803:3:803:3 | pointer to ~vector output argument | This object is destroyed at the end of the full-expression. |

cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/IteratorToExpiredContainer/test.cpp

@@ -686,7 +686,7 @@ void test() {
   for (auto x : returnRef()[0]) {} // GOOD
   for (auto x : returnRef().at(0)) {} // GOOD
 
-  for(auto it = returnValue().begin(); it != returnValue().end(); ++it) {} // BAD
+  for(auto it = returnValue().begin(); it != returnValue().end(); ++it) {} // BAD [NOT DETECTED]
 
   {
   auto v = returnValue();
@@ -800,5 +800,5 @@ void test5(int i)
     const auto& vvs = returnValue();
     for(const auto& vs : vvs) { }
     ++i;
-  } // GOOD [FALSE POSITIVE]
+  } // GOOD
 }