diff options
author | Daniel Cheng <dcheng@chromium.org> | 2022-06-14 19:11:17 +0000 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2022-07-25 14:22:40 +0000 |
commit | 95bda789ea62fe9ef2aeff10ab0aa4b81ed8e718 (patch) | |
tree | af925bdd3ea6362cd6be0430a21115e041197758 | |
parent | 1c316daff337ed065e66e42141d60209a957be22 (diff) | |
download | qtwebengine-chromium-95bda789ea62fe9ef2aeff10ab0aa4b81ed8e718.tar.gz |
[Backport] CVE-2022-2156: Use after free in Base
Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/3703779:
[M102] Ensure raw_ptr<T> and T* are treated identically in //base callback.
There are safety checks associated with raw pointers (e.g. ensuring
receiver pointers are not raw pointers). Make sure these checks are
applied whether the input type is T* or raw_ptr<T>.
- Implement base::IsPointer<T> and base::RemovePointer<T>, which are
similar to std::is_pointer<T> and std::remove_pointer<T>, except they
also consider raw_ptr<T> a raw pointer type.
- Fix failures from the strengthened asserts: WebAppInstallFinalizer
does not need a callback at all, while the privacy sandbox dialog
tests can safely use base::Unretained().
- Add test cases to cover this in the //base callback nocompile test
suite.
- Fix the existing nocompile tests, which did not escape `||` and
inadvertently matched any error text.
(cherry picked from commit 00c072a2c7f24921af3bbf8441abb34ecb0551a6)
Bug: 1335458
Change-Id: I470e3d5bc35ed52bf125136db738a868ef90b7e7
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1013266}
Cr-Commit-Position: refs/branch-heads/5005@{#1173}
Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r-- | chromium/base/bind_internal.h | 16 | ||||
-rw-r--r-- | chromium/base/memory/raw_ptr.h | 32 | ||||
-rw-r--r-- | chromium/base/memory/raw_scoped_refptr_mismatch_checker.h | 5 |
3 files changed, 43 insertions, 10 deletions
diff --git a/chromium/base/bind_internal.h b/chromium/base/bind_internal.h index a7aba35bb6c..5315534e478 100644 --- a/chromium/base/bind_internal.h +++ b/chromium/base/bind_internal.h @@ -801,8 +801,8 @@ bool QueryCancellationTraits(const BindStateBase* base, template <typename Functor, typename Receiver, typename... Unused> std::enable_if_t< !(MakeFunctorTraits<Functor>::is_method && - std::is_pointer<std::decay_t<Receiver>>::value && - IsRefCountedType<std::remove_pointer_t<std::decay_t<Receiver>>>::value)> + IsPointerV<std::decay_t<Receiver>> && + IsRefCountedType<RemovePointerT<std::decay_t<Receiver>>>::value)> BanUnconstructedRefCountedReceiver(const Receiver& receiver, Unused&&...) {} template <typename Functor> @@ -812,8 +812,8 @@ void BanUnconstructedRefCountedReceiver() {} template <typename Functor, typename Receiver, typename... Unused> std::enable_if_t< MakeFunctorTraits<Functor>::is_method && - std::is_pointer<std::decay_t<Receiver>>::value && - IsRefCountedType<std::remove_pointer_t<std::decay_t<Receiver>>>::value> + IsPointerV<std::decay_t<Receiver>> && + IsRefCountedType<RemovePointerT<std::decay_t<Receiver>>>::value> BanUnconstructedRefCountedReceiver(const Receiver& receiver, Unused&&...) { DCHECK(receiver); @@ -946,8 +946,8 @@ struct MakeBindStateTypeImpl<true, Functor, Receiver, BoundArgs...> { static_assert(!std::is_array<std::remove_reference_t<Receiver>>::value, "First bound argument to a method cannot be an array."); static_assert( - !std::is_pointer<DecayedReceiver>::value || - IsRefCountedType<std::remove_pointer_t<DecayedReceiver>>::value, + !IsPointerV<DecayedReceiver> || + IsRefCountedType<RemovePointerT<DecayedReceiver>>::value, "Receivers may not be raw pointers. If using a raw pointer here is safe" " and has no lifetime concerns, use base::Unretained() and document why" " it's safe."); @@ -957,8 +957,8 @@ struct MakeBindStateTypeImpl<true, Functor, Receiver, BoundArgs...> { public: using Type = BindState< std::decay_t<Functor>, - std::conditional_t<std::is_pointer<DecayedReceiver>::value, - scoped_refptr<std::remove_pointer_t<DecayedReceiver>>, + std::conditional_t<IsPointerV<DecayedReceiver>, + scoped_refptr<RemovePointerT<DecayedReceiver>>, DecayedReceiver>, std::decay_t<BoundArgs>...>; }; diff --git a/chromium/base/memory/raw_ptr.h b/chromium/base/memory/raw_ptr.h index f22003573c7..56f0bd20d5a 100644 --- a/chromium/base/memory/raw_ptr.h +++ b/chromium/base/memory/raw_ptr.h @@ -579,6 +579,38 @@ class raw_ptr { friend class raw_ptr; }; + +// Template helpers for working with T* or raw_ptr<T>. +template <typename T> +struct IsPointer : std::false_type {}; + +template <typename T> +struct IsPointer<T*> : std::true_type {}; + +template <typename T, typename I> +struct IsPointer<raw_ptr<T, I>> : std::true_type {}; + +template <typename T> +inline constexpr bool IsPointerV = IsPointer<T>::value; + +template <typename T> +struct RemovePointer { + using type = T; +}; + +template <typename T> +struct RemovePointer<T*> { + using type = T; +}; + +template <typename T, typename I> +struct RemovePointer<raw_ptr<T, I>> { + using type = T; +}; + +template <typename T> +using RemovePointerT = typename RemovePointer<T>::type; + } // namespace base using base::raw_ptr; diff --git a/chromium/base/memory/raw_scoped_refptr_mismatch_checker.h b/chromium/base/memory/raw_scoped_refptr_mismatch_checker.h index 9e50458ec98..7afae066fa3 100644 --- a/chromium/base/memory/raw_scoped_refptr_mismatch_checker.h +++ b/chromium/base/memory/raw_scoped_refptr_mismatch_checker.h @@ -7,6 +7,7 @@ #include <type_traits> +#include "base/memory/raw_ptr.h" #include "base/template_util.h" // It is dangerous to post a task with a T* argument where T is a subtype of @@ -35,8 +36,8 @@ struct IsRefCountedType<T, // pointer type and are convertible to a RefCounted(Base|ThreadSafeBase) type. template <typename T> struct NeedsScopedRefptrButGetsRawPtr - : conjunction<std::is_pointer<T>, - IsRefCountedType<std::remove_pointer_t<T>>> { + : conjunction<base::IsPointer<T>, + IsRefCountedType<base::RemovePointerT<T>>> { static_assert(!std::is_reference<T>::value, "NeedsScopedRefptrButGetsRawPtr requires non-reference type."); }; |