diff --git a/cpp/ql/src/Security/CWE/CWE-416/Temporaries.qll b/cpp/ql/src/Security/CWE/CWE-416/Temporaries.qll new file mode 100644 index 00000000000..56d742b31d7 --- /dev/null +++ b/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) + ) +} diff --git a/cpp/ql/src/Security/CWE/CWE-416/UseOfStringAfterLifetimeEnds.ql b/cpp/ql/src/Security/CWE/CWE-416/UseOfStringAfterLifetimeEnds.ql index 5fd75150167..7d776280f91 100644 --- a/cpp/ql/src/Security/CWE/CWE-416/UseOfStringAfterLifetimeEnds.ql +++ b/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 - -/** - * 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) -} +import Temporaries from Call c where diff --git a/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.qhelp b/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.qhelp new file mode 100644 index 00000000000..675defdfc67 --- /dev/null +++ b/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.qhelp @@ -0,0 +1,44 @@ + + + + +

Calling get on a std::unique_ptr object returns a pointer to the underlying allocations. +When the std::unique_ptr object is destroyed, the pointer returned by get is no +longer valid. If the pointer is used after the std::unique_ptr object is destroyed, then the behavior is undefined. +

+ + + +

+Ensure that the pointer returned by get does not outlive the underlying std::unique_ptr object. +

+ + + +

+The following example gets a std::unique_ptr object, and then converts the resulting unique pointer to a +pointer using get so that it can be passed to the work function. + +However, the std::unique_ptr object is destroyed as soon as the call +to get returns. This means that work is given a pointer to invalid memory. +

+ + + +

+The following example fixes the above code by ensuring that the pointer returned by the call to get does +not outlive the underlying std::unique_ptr objects. This ensures that the pointer passed to work +points to valid memory. +

+ + + + + + +
  • MEM50-CPP. Do not access freed memory.
    • + +     +     diff --git a/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.ql b/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.ql new file mode 100644 index 00000000000..4edb9b027d3 --- /dev/null +++ b/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." diff --git a/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEndsBad.cpp b/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEndsBad.cpp new file mode 100644 index 00000000000..b706f1fbb5f --- /dev/null +++ b/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEndsBad.cpp @@ -0,0 +1,10 @@ +#include +std::unique_ptr 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); +} \ No newline at end of file diff --git a/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEndsGood.cpp b/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEndsGood.cpp new file mode 100644 index 00000000000..eee39bd3582 --- /dev/null +++ b/cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEndsGood.cpp @@ -0,0 +1,10 @@ +#include +std::unique_ptr 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()); +} \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/UseOfUniquePtrAfterLifetimeEnds/UseOfUniquePointerAfterLifetimeEnds.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/UseOfUniquePtrAfterLifetimeEnds/UseOfUniquePointerAfterLifetimeEnds.expected new file mode 100644 index 00000000000..8bd30062b57 --- /dev/null +++ b/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. | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/UseOfUniquePtrAfterLifetimeEnds/UseOfUniquePointerAfterLifetimeEnds.qlref b/cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/UseOfUniquePtrAfterLifetimeEnds/UseOfUniquePointerAfterLifetimeEnds.qlref new file mode 100644 index 00000000000..4c613e5c5ac --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/UseOfUniquePtrAfterLifetimeEnds/UseOfUniquePointerAfterLifetimeEnds.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.ql \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/UseOfUniquePtrAfterLifetimeEnds/test.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/UseOfUniquePtrAfterLifetimeEnds/test.cpp new file mode 100644 index 00000000000..c1d55a28c17 --- /dev/null +++ b/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 struct remove_reference { typedef T type; }; + + template struct remove_reference { typedef T type; }; + + template struct remove_reference { typedef T type; }; + + template using remove_reference_t = typename remove_reference::type; + + template< class T > std::remove_reference_t&& move( T&& t ); + + template< class T > struct default_delete; + + template struct add_lvalue_reference { typedef T& type; }; +} + +// --- iterator --- + +namespace std { + template struct remove_const { typedef T type; }; + + template struct remove_const { typedef T type; }; + + // `remove_const_t` removes any `const` specifier from `T` + template using remove_const_t = typename remove_const::type; + + struct ptrdiff_t; + + template struct iterator_traits; + + template + struct iterator { + typedef Category iterator_category; + + iterator(); + iterator(iterator > 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 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 unique_ptr(unique_ptr&& u) noexcept; + unique_ptr(const unique_ptr&) = delete; + + unique_ptr& operator=(unique_ptr&& u) noexcept; + unique_ptr& operator=(std::nullptr_t) noexcept; + template unique_ptr& operator=(unique_ptr&& 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::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 allocator { + public: + allocator() throw(); + typedef size_t size_type; + }; + + template> + 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; + using const_iterator = std::iterator; + + 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 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 iterator insert(const_iterator position, InputIterator first, InputIterator last); + + template iterator emplace (const_iterator position, Args&&... args); + template 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 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 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 = 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& 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&& 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 +} \ No newline at end of file