Fix use after free in I18nSourceStream.

This adds explicit lifetime management between URLRequestChromeJob and
URLDataSourceImpl, by having the job own a shared_refptr to the data
source. This allows the data source to be detached from the backend
yet remain alive as long as the request job is still alive (which
eventually wants to use its i18n template replacements map).

BUG=863318

Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I84caa70dff1e1c65ac430fca11656e85e4a6eebd
Reviewed-on: https://chromium-review.googlesource.com/1156625
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579994}

content/browser/webui/url_data_manager_backend.cc

@@ -119,9 +119,7 @@ class URLRequestChromeJob : public net::URLRequestJob {
     is_gzipped_ = is_gzipped;
   }
 
-  void SetReplacements(const ui::TemplateReplacements* replacements) {
-    replacements_ = replacements;
-  }
+  void SetSource(scoped_refptr<URLDataSourceImpl> source) { source_ = source; }
 
  private:
   ~URLRequestChromeJob() override;
@@ -162,8 +160,10 @@ class URLRequestChromeJob : public net::URLRequestJob {
   // resources in resources.pak use compress="gzip".
   bool is_gzipped_;
 
-  // Replacement dictionary for i18n.
-  const ui::TemplateReplacements* replacements_;
+  // The URLDataSourceImpl that is servicing this request. This is a shared
+  // pointer so that the request can continue to be served even if the source is
+  // detached from the backend that initially owned it.
+  scoped_refptr<URLDataSourceImpl> source_;
 
   // The backend is owned by net::URLRequestContext and always outlives us.
   URLDataManagerBackend* const backend_;
@@ -181,7 +181,6 @@ URLRequestChromeJob::URLRequestChromeJob(net::URLRequest* request,
       data_available_status_(net::OK),
       pending_buf_size_(0),
       is_gzipped_(false),
-      replacements_(nullptr),
       backend_(backend),
       weak_factory_(this) {
   DCHECK(backend);
@@ -247,9 +246,20 @@ std::unique_ptr<net::SourceStream> URLRequestChromeJob::SetUpSourceStream() {
                                                   net::SourceStream::TYPE_GZIP);
   }
 
-  if (replacements_) {
+  // The URLRequestJob and the SourceStreams we are creating are owned by the
+  // same parent URLRequest, thus it is safe to pass the replacements via a raw
+  // pointer.
+  const ui::TemplateReplacements* replacements = nullptr;
+  if (source_)
+    replacements = source_->GetReplacements();
+  if (replacements) {
+    // It is safe to pass the raw replacements directly to the source stream, as
+    // both this URLRequestChromeJob and the I18nSourceStream are owned by the
+    // same root URLRequest. The replacements are owned by the URLDataSourceImpl
+    // which we keep alive via |source_|, ensuring its lifetime is also bound
+    // to the safe URLRequest.
     source_stream = ui::I18nSourceStream::Create(
-        std::move(source_stream), net::SourceStream::TYPE_NONE, replacements_);
+        std::move(source_stream), net::SourceStream::TYPE_NONE, replacements);
   }
 
   return source_stream;
@@ -488,7 +498,7 @@ bool URLDataManagerBackend::StartRequest(const net::URLRequest* request,
   // replacements upon.
   std::string mime_type = source->source()->GetMimeType(path);
   if (mime_type == "text/html")
-    job->SetReplacements(source->GetReplacements());
+    job->SetSource(source);
 
   // Also notifies that the headers are complete.
   job->MimeTypeAvailable(mime_type);

ios/web/webui/url_data_manager_ios_backend.mm

@@ -135,8 +135,8 @@ class URLRequestChromeJob : public net::URLRequestJob {
 
   void set_is_gzipped(bool is_gzipped) { is_gzipped_ = is_gzipped; }
 
-  void set_replacements(const ui::TemplateReplacements* replacements) {
-    replacements_ = replacements;
+  void set_source(scoped_refptr<URLDataSourceIOSImpl> source) {
+    source_ = source;
   }
 
   void set_send_content_type_header(bool send_content_type_header) {
@@ -184,9 +184,10 @@ class URLRequestChromeJob : public net::URLRequestJob {
   // resources in resources.pak use compress="gzip".
   bool is_gzipped_;
 
-  // Replacement dictionary for i18n. Owned by |backend_| and so guaranteed to
-  // outlive us.
-  const ui::TemplateReplacements* replacements_;
+  // The URLDataSourceIOSImpl that is servicing this request. This is a shared
+  // pointer so that the request can continue to be served even if the source is
+  // detached from the backend that initially owned it.
+  scoped_refptr<URLDataSourceIOSImpl> source_;
 
   // If true, sets the "Content-Type: <mime-type>" header.
   bool send_content_type_header_;
@@ -219,7 +220,6 @@ URLRequestChromeJob::URLRequestChromeJob(net::URLRequest* request,
       content_security_policy_frame_source_("frame-src 'none';"),
       deny_xframe_options_(true),
       is_gzipped_(false),
-      replacements_(nullptr),
       send_content_type_header_(false),
       is_incognito_(is_incognito),
       browser_state_(browser_state),
@@ -308,9 +308,20 @@ std::unique_ptr<net::SourceStream> URLRequestChromeJob::SetUpSourceStream() {
                                                   net::SourceStream::TYPE_GZIP);
   }
 
-  if (replacements_) {
+  // The URLRequestJob and the SourceStreams we are creating are owned by the
+  // same parent URLRequest, thus it is safe to pass the replacements via a raw
+  // pointer.
+  const ui::TemplateReplacements* replacements = nullptr;
+  if (source_)
+    replacements = source_->GetReplacements();
+  if (replacements) {
+    // It is safe to pass the raw replacements directly to the source stream, as
+    // both this URLRequestChromeJob and the I18nSourceStream are owned by the
+    // same root URLRequest. The replacements are owned by the URLDataSourceImpl
+    // which we keep alive via |source_|, ensuring its lifetime is also bound
+    // to the safe URLRequest.
     source_stream = ui::I18nSourceStream::Create(
-        std::move(source_stream), net::SourceStream::TYPE_NONE, replacements_);
+        std::move(source_stream), net::SourceStream::TYPE_NONE, replacements);
   }
 
   return source_stream;
@@ -321,7 +332,7 @@ void URLRequestChromeJob::MimeTypeAvailable(URLDataSourceIOSImpl* source,
   set_mime_type(mime_type);
 
   if (mime_type == "text/html")
-    set_replacements(source->GetReplacements());
+    set_source(source);
 
   NotifyHeadersComplete();
 }

ui/base/webui/i18n_source_stream.h

@@ -51,7 +51,7 @@ class UI_BASE_EXPORT I18nSourceStream : public net::FilterSourceStream {
   std::string output_;
 
   // A map of i18n replacement keys and translations.
-  const TemplateReplacements* replacements_;  // weak
+  const TemplateReplacements* replacements_;
 
   DISALLOW_COPY_AND_ASSIGN(I18nSourceStream);
 };