CPP: Add query for detecting invalid uses of temporary unique pointers.

cpp/ql/src/Security/CWE/CWE-416/Temporaries.qll

@@ -0,0 +1,90 @@
+import cpp
+import semmle.code.cpp.models.implementations.StdContainer
+
+/**
+ * Holds if `e` will be consumed by its parent as a glvalue and does not have
+ * an lvalue-to-rvalue conversion. This means that it will be materialized into
+ * a temporary object.
+ */
+predicate isTemporary(Expr e) {
+  e instanceof TemporaryObjectExpr
+  or
+  e.isPRValueCategory() and
+  e.getUnspecifiedType() instanceof Class and
+  not e.hasLValueToRValueConversion()
+}
+
+/** Holds if `e` is written to a container. */
+predicate isStoredInContainer(Expr e) {
+  exists(StdSequenceContainerInsert insert, Call call, int index |
+    call = insert.getACallToThisFunction() and
+    index = insert.getAValueTypeParameterIndex() and
+    call.getArgument(index) = e
+  )
+  or
+  exists(StdSequenceContainerPush push, Call call, int index |
+    call = push.getACallToThisFunction() and
+    index = push.getAValueTypeParameterIndex() and
+    call.getArgument(index) = e
+  )
+  or
+  exists(StdSequenceEmplace emplace, Call call, int index |
+    call = emplace.getACallToThisFunction() and
+    index = emplace.getAValueTypeParameterIndex() and
+    call.getArgument(index) = e
+  )
+  or
+  exists(StdSequenceEmplaceBack emplaceBack, Call call, int index |
+    call = emplaceBack.getACallToThisFunction() and
+    index = emplaceBack.getAValueTypeParameterIndex() and
+    call.getArgument(index) = e
+  )
+}
+
+/**
+ * Holds if the value of `e` outlives the enclosing full expression. For
+ * example, because the value is stored in a local variable.
+ */
+predicate outlivesFullExpr(Expr e) {
+  not e.getConversion*().hasLValueToRValueConversion() and
+  (
+    any(Assignment assign).getRValue() = e
+    or
+    any(Variable v).getInitializer().getExpr() = e
+    or
+    any(ReturnStmt ret).getExpr() = e
+    or
+    exists(ConditionalExpr cond |
+      outlivesFullExpr(cond) and
+      [cond.getThen(), cond.getElse()] = e
+    )
+    or
+    exists(BinaryOperation bin |
+      outlivesFullExpr(bin) and
+      bin.getAnOperand() = e and
+      not bin instanceof ComparisonOperation
+    )
+    or
+    exists(PointerFieldAccess fa |
+      outlivesFullExpr(fa) and
+      fa.getQualifier() = e
+    )
+    or
+    exists(AddressOfExpr ao |
+      outlivesFullExpr(ao) and
+      ao.getOperand() = e
+    )
+    or
+    exists(ClassAggregateLiteral aggr |
+      outlivesFullExpr(aggr) and
+      aggr.getAFieldExpr(_) = e
+    )
+    or
+    exists(ArrayAggregateLiteral aggr |
+      outlivesFullExpr(aggr) and
+      aggr.getAnElementExpr(_) = e
+    )
+    or
+    isStoredInContainer(e)
+  )
+}

cpp/ql/src/Security/CWE/CWE-416/UseOfStringAfterLifetimeEnds.ql

@@ -14,81 +14,7 @@
 
 import cpp
 import semmle.code.cpp.models.implementations.StdString
-import semmle.code.cpp.models.implementations.StdContainer
+import Temporaries
-
-/**
- * Holds if `e` will be consumed by its parent as a glvalue and does not have
- * an lvalue-to-rvalue conversion. This means that it will be materialized into
- * a temporary object.
- */
-predicate isTemporary(Expr e) {
-  e instanceof TemporaryObjectExpr
-  or
-  e.isPRValueCategory() and
-  e.getUnspecifiedType() instanceof Class and
-  not e.hasLValueToRValueConversion()
-}
-
-/** Holds if `e` is written to a container. */
-predicate isStoredInContainer(Expr e) {
-  exists(StdSequenceContainerInsert insert, Call call, int index |
-    call = insert.getACallToThisFunction() and
-    index = insert.getAValueTypeParameterIndex() and
-    call.getArgument(index) = e
-  )
-  or
-  exists(StdSequenceContainerPush push, Call call, int index |
-    call = push.getACallToThisFunction() and
-    index = push.getAValueTypeParameterIndex() and
-    call.getArgument(index) = e
-  )
-  or
-  exists(StdSequenceEmplace emplace, Call call, int index |
-    call = emplace.getACallToThisFunction() and
-    index = emplace.getAValueTypeParameterIndex() and
-    call.getArgument(index) = e
-  )
-  or
-  exists(StdSequenceEmplaceBack emplaceBack, Call call, int index |
-    call = emplaceBack.getACallToThisFunction() and
-    index = emplaceBack.getAValueTypeParameterIndex() and
-    call.getArgument(index) = e
-  )
-}
-
-/**
- * Holds if the value of `e` outlives the enclosing full expression. For
- * example, because the value is stored in a local variable.
- */
-predicate outlivesFullExpr(Expr e) {
-  any(Assignment assign).getRValue() = e
-  or
-  any(Variable v).getInitializer().getExpr() = e
-  or
-  any(ReturnStmt ret).getExpr() = e
-  or
-  exists(ConditionalExpr cond |
-    outlivesFullExpr(cond) and
-    [cond.getThen(), cond.getElse()] = e
-  )
-  or
-  exists(BinaryOperation bin |
-    outlivesFullExpr(bin) and
-    bin.getAnOperand() = e
-  )
-  or
-  exists(ClassAggregateLiteral aggr |
-    outlivesFullExpr(aggr) and
-    aggr.getAFieldExpr(_) = e
-  )
-  or
-  exists(ArrayAggregateLiteral aggr |
-    outlivesFullExpr(aggr) and
-    aggr.getAnElementExpr(_) = e
-  )
-  or
-  isStoredInContainer(e)
-}
 
 from Call c
 where

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

@@ -0,0 +1,44 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Calling <code>get</code> on a <code>std::unique_ptr</code> object returns a pointer to the underlying allocations.
+When the <code>std::unique_ptr</code> object is destroyed, the pointer returned by <code>get</code> is no
+longer valid. If the pointer is used after the <code>std::unique_ptr</code> object is destroyed, then the behavior is undefined.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Ensure that the pointer returned by <code>get</code> does not outlive the underlying <code>std::unique_ptr</code> object.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example gets a <code>std::unique_ptr</code> object, and then converts the resulting unique pointer to a
+pointer using <code>get</code> so that it can be passed to the <code>work</code> function.
+
+However, the <code>std::unique_ptr</code> object is destroyed as soon as the call
+to <code>get</code> returns. This means that <code>work</code> is given a pointer to invalid memory.
+</p>
+
+<sample src="UseOfUniquePointerAfterLifetimeEndsBad.cpp" />
+
+<p>
+The following example fixes the above code by ensuring that the pointer returned by the call to <code>get</code> does
+not outlive the underlying <code>std::unique_ptr</code> objects. This ensures that the pointer passed to <code>work</code>
+points to valid memory.
+</p>
+
+<sample src="UseOfUniquePointerAfterLifetimeEndsGood.cpp" />
+
+</example>
+<references>
+
+<li><a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM50-CPP.+Do+not+access+freed+memory">MEM50-CPP. Do not access freed memory</a>.</li>
+
+</references>
+</qhelp>

cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.ql

@@ -0,0 +1,36 @@
+/**
+ * @name Use of unique pointer after lifetime ends
+ * @description If a reference to the contents of a unique pointer outlives the underlying object it may lead to unexpected behavior.
+ * @kind problem
+ * @precision high
+ * @id cpp/use-of-uniwue-pointer-after-lifetime-ends
+ * @problem.severity warning
+ * @security-severity 8.8
+ * @tags reliability
+ *       security
+ *       external/cwe/cwe-416
+ *       external/cwe/cwe-664
+ */
+
+import cpp
+import semmle.code.cpp.models.interfaces.PointerWrapper
+import Temporaries
+
+predicate isUniquePointerDerefFunction(Function f) {
+  exists(PointerWrapper wrapper |
+    f = wrapper.getAnUnwrapperFunction() and
+    // We only want unique pointers as the memory behind share pointers may still be
+    // alive after the shared pointer is destroyed.
+    wrapper.(Class).hasQualifiedName(["std", "bsl"], "unique_ptr")
+  )
+}
+
+from Call c
+where
+  outlivesFullExpr(c) and
+  not c.isFromUninstantiatedTemplate(_) and
+  isUniquePointerDerefFunction(c.getTarget()) and
+  isTemporary(c.getQualifier().getFullyConverted())
+select c,
+  "The underlying unique pointer object is destroyed after the call to '" + c.getTarget() +
+    "' returns."

cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEndsBad.cpp

@@ -0,0 +1,10 @@
+#include <memory>
+std::unique_ptr<T> getUniquePointer();
+void work(const T*);
+
+// BAD: the unique pointer is deallocated when `get` returns. So `work`
+// is given a pointer to invalid memory.
+void work_with_unique_ptr_bad() {
+  const T* combined_string = getUniquePointer().get();
+  work(combined_string);
+}

cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEndsGood.cpp

@@ -0,0 +1,10 @@
+#include <memory>
+std::unique_ptr<T> getUniquePointer();
+void work(const T*);
+
+// GOOD: the unique pointer outlives the call to `work`. So the pointer
+// obtainted from `get` is valid.
+void work_with_unique_ptr_good() {
+  auto combined_string = getUniquePointer();
+  work(combined_string.get());
+}

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

@@ -0,0 +1,9 @@
+| test.cpp:156:15:156:15 | call to operator* | The underlying unique pointer object is destroyed after the call to 'operator*' returns. |
+| test.cpp:157:31:157:33 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
+| test.cpp:159:32:159:32 | call to operator-> | The underlying unique pointer object is destroyed after the call to 'operator->' returns. |
+| test.cpp:160:35:160:37 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
+| test.cpp:161:44:161:46 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
+| test.cpp:163:25:163:27 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
+| test.cpp:172:33:172:35 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
+| test.cpp:174:32:174:34 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
+| test.cpp:176:11:176:11 | call to operator* | The underlying unique pointer object is destroyed after the call to 'operator*' returns. |

cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/UseOfUniquePtrAfterLifetimeEnds/UseOfUniquePointerAfterLifetimeEnds.qlref

@@ -0,0 +1 @@
+Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.ql

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

@@ -0,0 +1,203 @@
+typedef unsigned long size_t;
+
+namespace std {
+  template<class T> struct remove_reference { typedef T type; };
+
+  template<class T> struct remove_reference<T &> { typedef T type; };
+
+  template<class T> struct remove_reference<T &&> { typedef T type; };
+
+  template<class T> using remove_reference_t = typename remove_reference<T>::type;
+
+  template< class T > std::remove_reference_t<T>&& move( T&& t );
+
+  template< class T > struct default_delete;
+
+  template<class T>  struct add_lvalue_reference { typedef T& type; };
+}
+
+// --- iterator ---
+
+namespace std {
+  template<class T> struct remove_const { typedef T type; };
+
+  template<class T> struct remove_const<const T> { typedef T type; };
+
+  // `remove_const_t<T>` removes any `const` specifier from `T`
+  template<class T> using remove_const_t = typename remove_const<T>::type;
+
+  struct ptrdiff_t;
+
+  template<class I> struct iterator_traits;
+
+  template <class Category,
+        class value_type,
+        class difference_type = ptrdiff_t,
+        class pointer_type = value_type*,
+        class reference_type = value_type&>
+  struct iterator {
+    typedef Category iterator_category;
+
+    iterator();
+    iterator(iterator<Category, remove_const_t<value_type> > const &other); // non-const -> const conversion constructor
+
+    iterator &operator++();
+    iterator operator++(int);
+    iterator &operator--();
+    iterator operator--(int);
+    bool operator==(iterator other) const;
+    bool operator!=(iterator other) const;
+    reference_type operator*() const;
+    pointer_type operator->() const;
+    iterator operator+(int);
+    iterator operator-(int);
+    iterator &operator+=(int);
+    iterator &operator-=(int);
+    int operator-(iterator);
+    reference_type operator[](int);
+  };
+
+  struct input_iterator_tag {};
+  struct forward_iterator_tag : public input_iterator_tag {};
+  struct bidirectional_iterator_tag : public forward_iterator_tag {};
+  struct random_access_iterator_tag : public bidirectional_iterator_tag {};
+}
+
+// --- string ---
+
+namespace std
+{
+
+  using nullptr_t = decltype(nullptr);
+
+  template<class T, class Deleter = std::default_delete<T>> class unique_ptr {
+  public:
+    using pointer = T*;
+    using element_type = T;
+    using deleter_type = Deleter;
+
+    constexpr unique_ptr() noexcept;
+    constexpr unique_ptr(nullptr_t) noexcept;
+    explicit unique_ptr(pointer p) noexcept;
+    unique_ptr(unique_ptr&& u) noexcept;
+    template<class U, class E> unique_ptr(unique_ptr<U, E>&& u) noexcept;
+    unique_ptr(const unique_ptr&) = delete;
+
+    unique_ptr& operator=(unique_ptr&& u) noexcept;
+    unique_ptr& operator=(std::nullptr_t) noexcept;
+    template<class U, class E> unique_ptr& operator=(unique_ptr<U, E>&& u) noexcept;
+    
+    ~unique_ptr();
+
+    pointer get() const noexcept;
+    deleter_type& get_deleter() noexcept;
+    const deleter_type& get_deleter() const noexcept;
+    explicit operator bool() const noexcept;
+    typename std::add_lvalue_reference<T>::type operator*() const;
+    pointer operator->() const noexcept;
+    pointer release() noexcept;
+    void reset(pointer p = pointer()) noexcept;
+    void swap(unique_ptr& u) noexcept;
+  };
+}
+
+// --- vector ---
+
+namespace std {
+  template <class T> class allocator {
+  public:
+    allocator() throw();
+    typedef size_t size_type;
+  };
+
+  template<class T, class Allocator = allocator<T>>
+  class vector { 
+  public:
+    using value_type = T;
+    using reference = value_type&;
+    using const_reference = const value_type&; 
+    using size_type = unsigned int;
+    using iterator = std::iterator<random_access_iterator_tag, T>;
+    using const_iterator = std::iterator<random_access_iterator_tag, const T>;
+
+    vector() noexcept(noexcept(Allocator()));
+    explicit vector(const Allocator&) noexcept;
+    explicit vector(size_type n, const Allocator& = Allocator());
+    vector(size_type n, const T& value, const Allocator& = Allocator());
+    template<class InputIterator, class IteratorCategory = typename InputIterator::iterator_category> vector(InputIterator first, InputIterator last, const Allocator& = Allocator());
+    ~vector();
+
+    void push_back(const T& x);
+    void push_back(T&& x);
+
+    iterator insert(const_iterator position, const T& x);
+    iterator insert(const_iterator position, T&& x);
+    iterator insert(const_iterator position, size_type n, const T& x);
+    template<class InputIterator> iterator insert(const_iterator position, InputIterator first, InputIterator last);
+
+    template <class... Args> iterator emplace (const_iterator position, Args&&... args);
+    template <class... Args> void emplace_back (Args&&... args);
+  };
+}
+
+struct S {
+  const char* s;
+};
+
+void call(S*);
+
+void call_by_value(S);
+void call_by_ref(S&);
+
+std::unique_ptr<S> get_unique_ptr();
+
+const S* test1(bool b1, bool b2) {
+  auto s1 = *get_unique_ptr(); // GOOD
+  auto s1a = &*get_unique_ptr(); // BAD
+  auto s1b = get_unique_ptr().get(); // BAD
+  auto s1c = get_unique_ptr()->s; // GOOD
+  auto s1d = &(get_unique_ptr()->s); // BAD
+  auto s2 = b1 ? get_unique_ptr().get() : nullptr; // BAD
+  auto s3 = b2 ? nullptr :get_unique_ptr().get(); // BAD
+  const S* s4;
+  s4 = get_unique_ptr().get(); // BAD
+
+  call(get_unique_ptr().get()); // GOOD
+  call(b1 ? get_unique_ptr().get() : nullptr); // GOOD
+  call(b1 ? (b2 ? nullptr : get_unique_ptr().get()) : nullptr); // GOOD
+  call_by_value(*get_unique_ptr()); // GOOD
+  call_by_ref(*get_unique_ptr()); // GOOD
+
+  std::vector<S*> v1;
+  v1.push_back(get_unique_ptr().get()); // BAD
+
+  S* s5[] = { get_unique_ptr().get() }; // BAD
+
+  return &*get_unique_ptr(); // BAD
+}
+
+void test2(bool b1, bool b2) {
+  
+  std::unique_ptr<S> s = get_unique_ptr();
+  auto s1 = s.get(); // GOOD
+  auto s2 = b1 ?  s.get() : nullptr; // GOOD
+  auto s3 = b2 ? nullptr :  s.get(); // GOOD
+  const S* s4;
+  s4 = s.get(); // GOOD
+
+  std::unique_ptr<S>& sRef = s;
+
+  auto s5 = sRef.get(); // GOOD
+  auto s6 = b1 ? sRef.get() : nullptr; // GOOD
+  auto s7 = b2 ? nullptr : sRef.get(); // GOOD
+  const S* s8;
+  s8 = sRef.get(); // GOOD
+
+  std::unique_ptr<S>&& sRefRef = get_unique_ptr();
+
+  auto s9 = sRefRef.get(); // GOOD
+  auto s10 = b1 ? sRefRef.get() : nullptr; // GOOD
+  auto s11 = b2 ? nullptr : sRefRef.get(); // GOOD
+  const S* s12;
+  s12 = sRefRef.get(); // GOOD
+}