[Backport] CVE-2021-30518: Heap buffer overflow in Reader Mode.

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2856118:
Replace std::vector with base::ObserverList to support container modification while iterating

TaskTracker saves list of viewers in vector, that needs to be notified
when distillation is completed. At the time of notifying the viewers,
we are indirectly erasing viewers from vector while iterating.

This is causing container-overflow in asan build when vector has more
than one viewer while notifying.

This change is to replace vector with ObserverList that can be modified
during iteration without invalidating the iterator.

Bug: 1203590
Change-Id: I7c7b8237584c48c9ebc2639b9268a6a78c2db4b2
Reviewed-by: Matt Jones <mdjones@chromium.org>
Commit-Queue: Akhila Veerapuraju <dhveerap@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#877492}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 10 insertions, 9 deletions

diff --git a/chromium/components/dom_distiller/core/task_tracker.cc b/chromium/components/dom_distiller/core/task_tracker.cc
index 152ba0f58dc..d3a8598844c 100644
--- a/chromium/components/dom_distiller/core/task_tracker.cc
+++ b/chromium/components/dom_distiller/core/task_tracker.cc
@@ -84,7 +84,7 @@ void TaskTracker::AddSaveCallback(const SaveCallback& callback) {
 
 std::unique_ptr<ViewerHandle> TaskTracker::AddViewer(
     ViewRequestDelegate* delegate) {
-  viewers_.push_back(delegate);
+  viewers_.AddObserver(delegate);
   if (content_ready_) {
     // Distillation for this task has already completed, and so the delegate can
     // be immediately told of the result.
@@ -112,7 +112,7 @@ bool TaskTracker::HasUrl(const GURL& url) const {
 }
 
 void TaskTracker::RemoveViewer(ViewRequestDelegate* delegate) {
-  viewers_.erase(std::remove(viewers_.begin(), viewers_.end(), delegate));
+  viewers_.RemoveObserver(delegate);
   if (viewers_.empty()) {
     MaybeCancel();
   }
@@ -215,8 +215,8 @@ void TaskTracker::DistilledArticleReady(
 }
 
 void TaskTracker::NotifyViewersAndCallbacks() {
-  for (size_t i = 0; i < viewers_.size(); ++i) {
-    NotifyViewer(viewers_[i]);
+  for (auto& viewer : viewers_) {
+    NotifyViewer(&viewer);
   }
 
   // Already inside a callback run SaveCallbacks directly.
@@ -242,8 +242,8 @@ void TaskTracker::DoSaveCallbacks(bool success) {
 
 void TaskTracker::OnArticleDistillationUpdated(
     const ArticleDistillationUpdate& article_update) {
-  for (size_t i = 0; i < viewers_.size(); ++i) {
-    viewers_[i]->OnArticleUpdated(article_update);
+  for (auto& viewer : viewers_) {
+    viewers.OnArticleUpdated(article_update);
   }
 }
 
diff --git a/chromium/components/dom_distiller/core/task_tracker.h b/chromium/components/dom_distiller/core/task_tracker.h
index 14ebd11def6..d0a9e3c4c68 100644
--- a/chromium/components/dom_distiller/core/task_tracker.h
+++ b/chromium/components/dom_distiller/core/task_tracker.h
@@ -12,6 +12,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "components/dom_distiller/core/article_distillation_update.h"
 #include "components/dom_distiller/core/article_entry.h"
 #include "components/dom_distiller/core/distiller.h"
@@ -39,9 +40,9 @@ class ViewerHandle {
 
 // Interface for a DOM distiller entry viewer. Implement this to make a view
 // request and receive the data for an entry when it becomes available.
-class ViewRequestDelegate {
+class ViewRequestDelegate : public base::Observer {
  public:
-  virtual ~ViewRequestDelegate() {}
+  ~ViewRequestDelegate() override {}
   // Called when the distilled article contents are available. The
   // DistilledArticleProto is owned by a TaskTracker instance and is invalidated
   // when the corresponding ViewerHandle is destroyed (or when the
@@ -136,7 +137,7 @@ class TaskTracker {
   std::vector<SaveCallback> save_callbacks_;
   // A ViewRequestDelegate will be added to this list when a view request is
   // made and removed when the corresponding ViewerHandle is destroyed.
-  std::vector<ViewRequestDelegate*> viewers_;
+  base::ObserverList<ViewRequestDelegate> viewers_;
 
   std::unique_ptr<Distiller> distiller_;
   bool blob_fetcher_running_;