C++: Promote cpp/iterator-to-expired-container out of experimental

[MathiasVP]

This PR moves the cpp/iterator-to-expired-container out of experimental and into Security-Extended as a medium precision query. There are a couple of simple follow-ups to be done before we can move it to high precision, but moving it to medium precision should not be controversial at this point.

It was mentioned in the past that we should rewrite the query alert message since the alert message currently says that the container is destroyed before the call to begin, but in reality what's going on is that the container is destroyed after the call to begin (but before it can be dereferenced without causing UB). Any ideas for how we should rephrase it? Some ideas:

• "This object is destroyed after call to begin returns."

This isn't ideal because most objects are destroyed after begin returns. The problem is that the object is destroyed before any meaningful things can be done with the return value.

• "This object is as soon as call to begin returns."

This isn't ideal since this is false in many cases. The object is destroyed at the end of the full expression, and that isn't necessary the call to begin.

• "This object is destroyed at the end of the full expression"

This is the most correct, but I think it would make sense to include the call to begin somewhere in the message?

[github-actions]

QHelp previews:

cpp/ql/src/Security/CWE/CWE-416/IteratorToExpiredContainer.qhelp

Iterator to expired container

Using an iterator owned by a container after the lifetime of the container has expired can lead to undefined behavior. This is because the iterator may be invalidated when the container is destroyed, and dereferencing an invalidated iterator is undefined behavior. These problems can be hard to spot due to C++'s complex rules for temporary object lifetimes and their extensions.

Recommendation

Never create an iterator to a temporary container when the iterator is expected to be used after the container's lifetime has expired.

Example

The rules for lifetime extension ensures that the code in lifetime_of_temp_extended is well-defined. This is because the lifetime of the temporary container returned by get_vector is extended to the end of the loop. However, prior to C++23, the lifetime extension rules do not ensure that the container returned by get_vector is extended in lifetime_of_temp_not_extended. This is because the temporary container is not bound to a rvalue reference.

#include <vector>

std::vector<int> get_vector();

void use(int);

void lifetime_of_temp_extended() {
  for(auto x : get_vector()) {
    use(x); // GOOD: The lifetime of the vector returned by `get_vector()` is extended until the end of the loop.
  }
}

// Writes the the values of `v` to an external log and returns it unchanged.
const std::vector<int>& log_and_return_argument(const std::vector<int>& v);

void lifetime_of_temp_not_extended() {
  for(auto x : log_and_return_argument(get_vector())) {
    use(x); // BAD: The lifetime of the vector returned by `get_vector()` is not extended, and the behavior is undefined.
  }
}

References

• CERT C Coding Standard: MEM30-C. Do not access freed memory.
• OWASP: Using freed memory.
• Lifetime safety: Preventing common dangling
• Containers library
• Range-based for loop (since C++11)
• Common Weakness Enumeration: CWE-416.
• Common Weakness Enumeration: CWE-664.

[geoffw0]

LGTM.