From 95bda789ea62fe9ef2aeff10ab0aa4b81ed8e718 Mon Sep 17 00:00:00 2001
From: Daniel Cheng <dcheng@chromium.org>
Date: Tue, 14 Jun 2022 19:11:17 +0000
Subject: [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>
---
 chromium/base/bind_internal.h                      | 16 +++++------
 chromium/base/memory/raw_ptr.h                     | 32 ++++++++++++++++++++++
 .../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.");
 };
-- 
cgit v1.2.1