Merge pull request #16367 from MathiasVP/better-documentation-for-ite…

…rator-to-expired-container C++: Improve documentation for `cpp/iterator-to-expired-container`
github · May 1, 2024 · dc4604f · dc4604f
2 parents 541effb + 40b6e16
commit dc4604f
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 17 deletions.
diff --git a/cpp/ql/src/Security/CWE/CWE-416/IteratorToExpiredContainer.qhelp b/cpp/ql/src/Security/CWE/CWE-416/IteratorToExpiredContainer.qhelp
@@ -30,6 +30,12 @@ This is because the temporary container is not bound to a rvalue reference.
 </p>
 <sample src="IteratorToExpiredContainerExtendedLifetime.cpp" />
 
+<p>
+To fix <code>lifetime_of_temp_not_extended</code>, consider rewriting the code so that the lifetime of the temporary object is extended.
+In <code>fixed_lifetime_of_temp_not_extended</code>, the lifetime of the temporary object has been extended by storing it in an rvalue reference.
+</p>
+<sample src="IteratorToExpiredContainerExtendedLifetime-fixed.cpp" />
+
 </example>
 <references>
 

diff --git a/cpp/ql/src/Security/CWE/CWE-416/IteratorToExpiredContainer.ql b/cpp/ql/src/Security/CWE/CWE-416/IteratorToExpiredContainer.ql
@@ -62,10 +62,9 @@ DataFlow::Node getADestroyedNode(DataFlow::Node n) {
   )
 }
 
-predicate destroyedToBeginSink(DataFlow::Node sink, FunctionCall fc) {
+predicate destroyedToBeginSink(DataFlow::Node sink) {
   exists(CallInstruction call |
     call = sink.asOperand().(ThisArgumentOperand).getCall() and
-    fc = call.getUnconvertedResultExpression() and
     call.getStaticCallTarget() instanceof BeginOrEndFunction
   )
 }
@@ -90,7 +89,7 @@ private predicate qualifierToDestroyed(DataFlow::Node node1, DataFlow::Node node
 module Config0 implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { qualifierToDestroyed(_, source) }
 
-  predicate isSink(DataFlow::Node sink) { destroyedToBeginSink(sink, _) }
+  predicate isSink(DataFlow::Node sink) { destroyedToBeginSink(sink) }
 }
 
 module Flow0 = DataFlow::Global<Config0>;
@@ -150,9 +149,9 @@ module Config implements DataFlow::StateConfigSig {
 
 module Flow = DataFlow::GlobalWithState<Config>;
 
-from Flow::PathNode source, Flow::PathNode sink, FunctionCall beginOrEnd, DataFlow::Node mid
+from Flow::PathNode source, Flow::PathNode sink, DataFlow::Node mid
 where
   Flow::flowPath(source, sink) and
-  destroyedToBeginSink(sink.getNode(), beginOrEnd) and
+  destroyedToBeginSink(sink.getNode()) and
   sink.getState() = Config::DestroyedToBegin(mid)
-select mid, "This object is destroyed before $@ is called.", beginOrEnd, beginOrEnd.toString()
+select mid, "This object is destroyed at the end of the full-expression."
diff --git a/cpp/ql/src/Security/CWE/CWE-416/IteratorToExpiredContainerExtendedLifetime-fixed.cpp b/cpp/ql/src/Security/CWE/CWE-416/IteratorToExpiredContainerExtendedLifetime-fixed.cpp
@@ -0,0 +1,6 @@
+void fixed_lifetime_of_temp_not_extended() {
+  auto&& v = get_vector();
+  for(auto x : log_and_return_argument(v)) {
+    use(x); // GOOD: The lifetime of the container returned by `get_vector()` has been extended to the lifetime of `v`.
+  }
+}
diff --git a/...y/CWE/CWE-416/semmle/tests/IteratorToExpiredContainer/IteratorToExpiredContainer.expected b/...y/CWE/CWE-416/semmle/tests/IteratorToExpiredContainer/IteratorToExpiredContainer.expected
@@ -1,11 +1,6 @@
-| test.cpp:680:30:680:30 | call to operator[] | This object is destroyed before $@ is called. | test.cpp:680:17:680:17 | call to begin | call to begin |
-| test.cpp:680:30:680:30 | call to operator[] | This object is destroyed before $@ is called. | test.cpp:680:17:680:17 | call to end | call to end |
-| test.cpp:683:31:683:32 | call to at | This object is destroyed before $@ is called. | test.cpp:683:17:683:17 | call to begin | call to begin |
-| test.cpp:683:31:683:32 | call to at | This object is destroyed before $@ is called. | test.cpp:683:17:683:17 | call to end | call to end |
-| test.cpp:689:46:689:58 | pointer to ~vector output argument | This object is destroyed before $@ is called. | test.cpp:689:60:689:62 | call to end | call to end |
-| test.cpp:702:27:702:27 | call to operator[] | This object is destroyed before $@ is called. | test.cpp:703:19:703:23 | call to begin | call to begin |
-| test.cpp:702:27:702:27 | call to operator[] | This object is destroyed before $@ is called. | test.cpp:703:36:703:38 | call to end | call to end |
-| test.cpp:727:23:727:23 | call to operator[] | This object is destroyed before $@ is called. | test.cpp:750:17:750:17 | call to begin | call to begin |
-| test.cpp:727:23:727:23 | call to operator[] | This object is destroyed before $@ is called. | test.cpp:750:17:750:17 | call to end | call to end |
-| test.cpp:735:23:735:23 | call to operator[] | This object is destroyed before $@ is called. | test.cpp:759:17:759:17 | call to begin | call to begin |
-| test.cpp:735:23:735:23 | call to operator[] | This object is destroyed before $@ is called. | test.cpp:759:17:759:17 | call to end | call to end |
+| 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. |