[Backport] CVE-2020-16016: Inappropriate implementation in base.

Cherry-pick of path originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2463857:
Prevent UB if a WeakPtr to an already-destroyed object is dereferenced.

If a WeakPtr references an already-destroyed object, operator-> and
operator* end up simply dereferencing nullptr. However, dereferencing
nullptr is undefined behavior and can be optimized in surprising ways
by compilers. To prevent this from happening, add a defence of last
resort and CHECK that the WeakPtr is still valid.

(cherry picked from commit 0b308a0e37b9d14a335c3b487511b7117c98d74b)

Bug: 817982
Change-Id: Ib3a025c18fbd9d5db88770fced2063135086847b
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#816701}
Reviewed-by: Krishna Govind <govind@chromium.org>
Reviewed-by: Adrian Taylor <adetaylor@chromium.org>
Commit-Queue: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/4280@{#1208}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

2 files changed, 24 insertions, 2 deletions

diff --git a/chromium/base/memory/weak_ptr.h b/chromium/base/memory/weak_ptr.h
index 8228b2b8fbd..99aa2aa32a8 100644
--- a/chromium/base/memory/weak_ptr.h
+++ b/chromium/base/memory/weak_ptr.h
@@ -248,11 +248,11 @@ class WeakPtr : public internal::WeakPtrBase {
   }
 
   T& operator*() const {
-    DCHECK(get() != nullptr);
+    CHECK(ref_.IsValid());
     return *get();
   }
   T* operator->() const {
-    DCHECK(get() != nullptr);
+    CHECK(ref_.IsValid());
     return get();
   }
 
diff --git a/chromium/base/memory/weak_ptr_unittest.cc b/chromium/base/memory/weak_ptr_unittest.cc
index e15d167aa54..d4818025fe9 100644
--- a/chromium/base/memory/weak_ptr_unittest.cc
+++ b/chromium/base/memory/weak_ptr_unittest.cc
@@ -750,4 +750,26 @@ TEST(WeakPtrDeathTest, NonOwnerThreadReferencesObjectAfterDeletion) {
   ASSERT_DCHECK_DEATH(arrow.target.get());
 }
 
+TEST(WeakPtrDeathTest, ArrowOperatorChecksOnBadDereference) {
+  // The default style "fast" does not support multi-threaded tests
+  // (introduces deadlock on Linux).
+  ::testing::FLAGS_gtest_death_test_style = "threadsafe";
+
+  auto target = std::make_unique<Target>();
+  WeakPtr<Target> weak = target->AsWeakPtr();
+  target.reset();
+  EXPECT_CHECK_DEATH(weak->AsWeakPtr());
+}
+
+TEST(WeakPtrDeathTest, StarOperatorChecksOnBadDereference) {
+  // The default style "fast" does not support multi-threaded tests
+  // (introduces deadlock on Linux).
+  ::testing::FLAGS_gtest_death_test_style = "threadsafe";
+
+  auto target = std::make_unique<Target>();
+  WeakPtr<Target> weak = target->AsWeakPtr();
+  target.reset();
+  EXPECT_CHECK_DEATH((*weak).AsWeakPtr());
+}
+
 }  // namespace base