Browse Source

chore: cherry-pick fix for 1277917 from chromium (#32787)

Cheng Zhao 3 years ago
parent
commit
f46b3f48af
2 changed files with 177 additions and 0 deletions
  1. 1 0
      patches/chromium/.patches
  2. 176 0
      patches/chromium/cherry-pick-1277917.patch

+ 1 - 0
patches/chromium/.patches

@@ -147,3 +147,4 @@ m98_fs_fix_fileutil_lifetime_issue.patch
 cherry-pick-0081bb347e67.patch
 cleanup_pausablecriptexecutor_usage.patch
 cherry-pick-ebc188ad769e.patch
+cherry-pick-1277917.patch

+ 176 - 0
patches/chromium/cherry-pick-1277917.patch

@@ -0,0 +1,176 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Simon Pelchat <[email protected]>
+Date: Fri, 7 Jan 2022 20:00:37 +0000
+Subject: Avoid UAF on StreamingSearchPrefetchURLLoader.
+
+StreamingSearchPrefetchURLLoader::OnDataAvailable used to "delete this",
+which results in deleting the DataPipeDrainer, which will then be used
+once OnDataAvailable returns. Instead, we post a task to delete the
+URL loader later on.
+
+Bug: 1277917
+Change-Id: I8d78c73a01fff0315b96ccb0e7fe605884b99823
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3365387
+Reviewed-by: Max Curran <[email protected]>
+Reviewed-by: Robert Ogden <[email protected]>
+Commit-Queue: Simon Pelchat <[email protected]>
+Cr-Commit-Position: refs/heads/main@{#956628}
+
+diff --git a/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_request.cc b/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_request.cc
+index 1d3d1ea93c026d371a6ad5678e8ff37dc58f8423..e3eaad96fbada18211f295988c07d218ab67921f 100644
+--- a/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_request.cc
++++ b/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_request.cc
+@@ -5,6 +5,7 @@
+ #include "chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_request.h"
+ 
+ #include "chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_url_loader.h"
++#include "streaming_search_prefetch_request.h"
+ 
+ StreamingSearchPrefetchRequest::StreamingSearchPrefetchRequest(
+     const GURL& prefetch_url,
+@@ -19,7 +20,9 @@ void StreamingSearchPrefetchRequest::StartPrefetchRequestInternal(
+     std::unique_ptr<network::ResourceRequest> resource_request,
+     const net::NetworkTrafficAnnotationTag& network_traffic_annotation) {
+   streaming_url_loader_ = std::make_unique<StreamingSearchPrefetchURLLoader>(
+-      this, profile, std::move(resource_request), network_traffic_annotation);
++      this, profile, std::move(resource_request), network_traffic_annotation,
++      base::BindOnce(&StreamingSearchPrefetchRequest::StopPrefetch,
++                     weak_factory_.GetWeakPtr()));
+ }
+ 
+ std::unique_ptr<SearchPrefetchURLLoader>
+diff --git a/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_request.h b/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_request.h
+index ff051c61e9cbed67d5cffbcac007d6c12a26cc76..ad75d404abe9e743d079725ffd8b01d3a49ae017 100644
+--- a/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_request.h
++++ b/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_request.h
+@@ -47,6 +47,8 @@ class StreamingSearchPrefetchRequest : public BaseSearchPrefetchRequest {
+  private:
+   // The ongoing prefetch request. Null before and after the fetch.
+   std::unique_ptr<StreamingSearchPrefetchURLLoader> streaming_url_loader_;
++
++  base::WeakPtrFactory<StreamingSearchPrefetchRequest> weak_factory_{this};
+ };
+ 
+ #endif  // CHROME_BROWSER_PREFETCH_SEARCH_PREFETCH_STREAMING_SEARCH_PREFETCH_REQUEST_H_
+diff --git a/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_url_loader.cc b/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_url_loader.cc
+index 09a8f5c444c5346dd6d2544c403a4b844e0ba781..2134bdbd2514d86b912b83480ca90242d7f7455e 100644
+--- a/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_url_loader.cc
++++ b/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_url_loader.cc
+@@ -26,15 +26,18 @@
+ #include "services/network/public/cpp/shared_url_loader_factory.h"
+ #include "services/network/public/mojom/early_hints.mojom.h"
+ #include "services/network/public/mojom/url_response_head.mojom.h"
++#include "streaming_search_prefetch_url_loader.h"
+ #include "url/gurl.h"
+ 
+ StreamingSearchPrefetchURLLoader::StreamingSearchPrefetchURLLoader(
+     StreamingSearchPrefetchRequest* streaming_prefetch_request,
+     Profile* profile,
+     std::unique_ptr<network::ResourceRequest> resource_request,
+-    const net::NetworkTrafficAnnotationTag& network_traffic_annotation)
++    const net::NetworkTrafficAnnotationTag& network_traffic_annotation,
++    base::OnceClosure stop_prefetch_closure)
+     : resource_request_(std::move(resource_request)),
+-      streaming_prefetch_request_(streaming_prefetch_request) {
++      streaming_prefetch_request_(streaming_prefetch_request),
++      stop_prefetch_closure_(std::move(stop_prefetch_closure)) {
+   DCHECK(streaming_prefetch_request_);
+   auto url_loader_factory = profile->GetDefaultStoragePartition()
+                                 ->GetURLLoaderFactoryForBrowserProcess();
+@@ -130,7 +133,7 @@ void StreamingSearchPrefetchURLLoader::OnReceiveRedirect(
+   if (streaming_prefetch_request_) {
+     streaming_prefetch_request_->ErrorEncountered();
+   } else {
+-    delete this;
++    PostTaskToStopPrefetchAndDeleteSelf();
+   }
+ }
+ 
+@@ -213,7 +216,7 @@ void StreamingSearchPrefetchURLLoader::OnStartLoadingResponseBodyFromData() {
+       mojo::CreateDataPipe(&options, producer_handle_, consumer_handle);
+ 
+   if (rv != MOJO_RESULT_OK) {
+-    delete this;
++    PostTaskToStopPrefetchAndDeleteSelf();
+     return;
+   }
+ 
+@@ -235,7 +238,7 @@ void StreamingSearchPrefetchURLLoader::OnHandleReady(
+     MojoResult result,
+     const mojo::HandleSignalsState& state) {
+   if (result != MOJO_RESULT_OK) {
+-    delete this;
++    PostTaskToStopPrefetchAndDeleteSelf();
+     return;
+   }
+   PushData();
+@@ -261,7 +264,7 @@ void StreamingSearchPrefetchURLLoader::PushData() {
+     }
+ 
+     if (result != MOJO_RESULT_OK) {
+-      delete this;
++      PostTaskToStopPrefetchAndDeleteSelf();
+       return;
+     }
+ 
+@@ -348,16 +351,24 @@ void StreamingSearchPrefetchURLLoader::OnURLLoaderMojoDisconnect() {
+     DCHECK(streaming_prefetch_request_);
+     streaming_prefetch_request_->ErrorEncountered();
+   } else {
+-    delete this;
++    PostTaskToStopPrefetchAndDeleteSelf();
+   }
+ }
+ 
+ void StreamingSearchPrefetchURLLoader::OnURLLoaderClientMojoDisconnect() {
+   DCHECK(forwarding_client_);
+   DCHECK(!streaming_prefetch_request_);
+-  delete this;
++  PostTaskToStopPrefetchAndDeleteSelf();
+ }
+ 
+ void StreamingSearchPrefetchURLLoader::ClearOwnerPointer() {
+   streaming_prefetch_request_ = nullptr;
+ }
++
++void StreamingSearchPrefetchURLLoader::PostTaskToStopPrefetchAndDeleteSelf() {
++  // To avoid UAF bugs, post a separate task to delete this object.
++  if (stop_prefetch_closure_) {
++    base::SequencedTaskRunnerHandle::Get()->PostTask(
++        FROM_HERE, std::move(stop_prefetch_closure_));
++  }
++}
+diff --git a/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_url_loader.h b/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_url_loader.h
+index 2213fb079e3f29566d67030dd2fa399ce11024e2..ed225db0b4d2b0d24aae967f320a7010c1a45044 100644
+--- a/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_url_loader.h
++++ b/chrome/browser/prefetch/search_prefetch/streaming_search_prefetch_url_loader.h
+@@ -39,7 +39,8 @@ class StreamingSearchPrefetchURLLoader : public network::mojom::URLLoader,
+       StreamingSearchPrefetchRequest* streaming_prefetch_request,
+       Profile* profile,
+       std::unique_ptr<network::ResourceRequest> resource_request,
+-      const net::NetworkTrafficAnnotationTag& network_traffic_annotation);
++      const net::NetworkTrafficAnnotationTag& network_traffic_annotation,
++      base::OnceClosure stop_prefetch_closure);
+ 
+   ~StreamingSearchPrefetchURLLoader() override;
+ 
+@@ -105,6 +106,9 @@ class StreamingSearchPrefetchURLLoader : public network::mojom::URLLoader,
+   // Clears |producer_handle_| and |handle_watcher_|.
+   void Finish();
+ 
++  // Post a task to delete this object by running stop_prefetch_closure_.
++  void PostTaskToStopPrefetchAndDeleteSelf();
++
+   // Sets up mojo forwarding to the navigation path. Resumes
+   // |network_url_loader_| calls. Serves the start of the response to the
+   // navigation path. After this method is called, |this| manages its own
+@@ -164,6 +168,9 @@ class StreamingSearchPrefetchURLLoader : public network::mojom::URLLoader,
+   mojo::ScopedDataPipeProducerHandle producer_handle_;
+   std::unique_ptr<mojo::SimpleWatcher> handle_watcher_;
+ 
++  // Closure to cancel this prefetch. Running this callback will destroy |this|.
++  base::OnceClosure stop_prefetch_closure_;
++
+   base::WeakPtrFactory<StreamingSearchPrefetchURLLoader> weak_factory_{this};
+ };
+