|
@@ -0,0 +1,231 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Andrey Belenko <[email protected]>
|
|
|
+Date: Wed, 19 May 2021 12:19:16 +0200
|
|
|
+Subject: Guard WebContents::DownloadImage() against malformed renderer
|
|
|
+ response
|
|
|
+
|
|
|
+Callers expect that ImageDownloadCallback gets invoked with two vectors
|
|
|
+having the same number of elements (one containing the bitmaps and the
|
|
|
+other one the corresponding sizes).
|
|
|
+
|
|
|
+However, these vectors are populated directly from the Mojo response,
|
|
|
+so there needs to be some browser-process sanitization to protect
|
|
|
+against buggy or compromised renderers.
|
|
|
+
|
|
|
+In this patch, WebContentsImpl::OnDidDownloadImage() mimics a 400 error
|
|
|
+if the response is malformed, similarly to how it's done in other edge
|
|
|
+cases (renderer process dead upon download). Because this scenario is
|
|
|
+a violation of the Mojo API contract, the browser process also issues
|
|
|
+a bad message log (newly-introduced WCI_INVALID_DOWNLOAD_IMAGE_RESULT)
|
|
|
+and shuts down the renderer process.
|
|
|
+
|
|
|
+(cherry picked from commit 034ba14e44f08e8ca84b42350f3238f847e08e5f)
|
|
|
+
|
|
|
+Change-Id: Ic0843e10efc26809fabd8f1bbe506ba1703d1486
|
|
|
+Fixed: 1201446
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2871796
|
|
|
+
|
|
|
+diff --git a/components/favicon/core/favicon_handler.cc b/components/favicon/core/favicon_handler.cc
|
|
|
+index 64eab051de73b0b259c1f31e93f750d352724521..410ffc64e8f70a29a39cc4fb228dfd720bfe04d7 100644
|
|
|
+--- a/components/favicon/core/favicon_handler.cc
|
|
|
++++ b/components/favicon/core/favicon_handler.cc
|
|
|
+@@ -492,6 +492,8 @@ void FaviconHandler::OnDidDownloadFavicon(
|
|
|
+ const GURL& image_url,
|
|
|
+ const std::vector<SkBitmap>& bitmaps,
|
|
|
+ const std::vector<gfx::Size>& original_bitmap_sizes) {
|
|
|
++ DCHECK_EQ(bitmaps.size(), original_bitmap_sizes.size());
|
|
|
++
|
|
|
+ // Mark download as finished.
|
|
|
+ image_download_request_.Cancel();
|
|
|
+
|
|
|
+diff --git a/components/favicon/core/favicon_handler.h b/components/favicon/core/favicon_handler.h
|
|
|
+index 8a1cf2ef85745711e94ec6e7f6f27acb1482cbd6..4192138711de470eee713fc1fb200cc29d333342 100644
|
|
|
+--- a/components/favicon/core/favicon_handler.h
|
|
|
++++ b/components/favicon/core/favicon_handler.h
|
|
|
+@@ -237,7 +237,9 @@ class FaviconHandler {
|
|
|
+ void ScheduleImageDownload(const GURL& image_url,
|
|
|
+ favicon_base::IconType icon_type);
|
|
|
+
|
|
|
+- // Triggered when a download of an image has finished.
|
|
|
++ // Triggered when a download of an image has finished. |bitmaps| and
|
|
|
++ // |original_bitmap_sizes| must contain the same number of elements (i.e. same
|
|
|
++ // vector size).
|
|
|
+ void OnDidDownloadFavicon(
|
|
|
+ favicon_base::IconType icon_type,
|
|
|
+ int id,
|
|
|
+diff --git a/components/favicon/ios/web_favicon_driver.mm b/components/favicon/ios/web_favicon_driver.mm
|
|
|
+index 6efaf651bdf939d83e1a7f9df339f714410565ad..71ae020efd8862d2b33cb91df9180ef63df1f6f9 100644
|
|
|
+--- a/components/favicon/ios/web_favicon_driver.mm
|
|
|
++++ b/components/favicon/ios/web_favicon_driver.mm
|
|
|
+@@ -75,6 +75,7 @@ int WebFaviconDriver::DownloadImage(const GURL& url,
|
|
|
+ for (const auto& frame : frames) {
|
|
|
+ sizes.push_back(gfx::Size(frame.width(), frame.height()));
|
|
|
+ }
|
|
|
++ DCHECK_EQ(frames.size(), sizes.size());
|
|
|
+ }
|
|
|
+ std::move(local_callback)
|
|
|
+ .Run(local_download_id, metadata.http_response_code, local_url,
|
|
|
+diff --git a/components/favicon_base/select_favicon_frames.cc b/components/favicon_base/select_favicon_frames.cc
|
|
|
+index 90b58e621492d0e503f7965de91581c0a451d163..73cd7dd5331a818f413d831b4ae596bc88805a8a 100644
|
|
|
+--- a/components/favicon_base/select_favicon_frames.cc
|
|
|
++++ b/components/favicon_base/select_favicon_frames.cc
|
|
|
+@@ -216,6 +216,7 @@ gfx::ImageSkia CreateFaviconImageSkia(
|
|
|
+ const std::vector<gfx::Size>& original_sizes,
|
|
|
+ int desired_size_in_dip,
|
|
|
+ float* score) {
|
|
|
++ DCHECK_EQ(bitmaps.size(), original_sizes.size());
|
|
|
+
|
|
|
+ const std::vector<float>& favicon_scales = favicon_base::GetFaviconScales();
|
|
|
+ std::vector<int> desired_sizes;
|
|
|
+diff --git a/components/favicon_base/select_favicon_frames.h b/components/favicon_base/select_favicon_frames.h
|
|
|
+index 573a38c79cddf839622488589b71b4d19fbdfba6..eab1e54466763e5a6a6069a29e877da054972fa8 100644
|
|
|
+--- a/components/favicon_base/select_favicon_frames.h
|
|
|
++++ b/components/favicon_base/select_favicon_frames.h
|
|
|
+@@ -38,6 +38,8 @@ extern const float kSelectFaviconFramesInvalidScore;
|
|
|
+ // it inspired by this method.
|
|
|
+ // If an unsupported scale (not in the favicon_base::GetFaviconScales())
|
|
|
+ // is requested, the ImageSkia will automatically scales using lancoz3.
|
|
|
++// |original_sizes| represents the pixel sizes of the favicon bitmaps in
|
|
|
++// |bitmaps|, which also means both vectors must have the same size.
|
|
|
+ gfx::ImageSkia CreateFaviconImageSkia(
|
|
|
+ const std::vector<SkBitmap>& bitmaps,
|
|
|
+ const std::vector<gfx::Size>& original_sizes,
|
|
|
+diff --git a/content/browser/bad_message.h b/content/browser/bad_message.h
|
|
|
+index ab0a195c2da772ea5825bb97e54743318bd06841..efc803efe24f61012882399035f6d96aed799595 100644
|
|
|
+--- a/content/browser/bad_message.h
|
|
|
++++ b/content/browser/bad_message.h
|
|
|
+@@ -256,6 +256,15 @@ enum BadMessageReason {
|
|
|
+ INPUT_ROUTER_INVALID_EVENT_SOURCE = 228,
|
|
|
+ RWH_CLOSE_PORTAL = 233,
|
|
|
+ MSDH_INVALID_STREAM_TYPE = 234,
|
|
|
++ RFH_CREATE_CHILD_FRAME_TOKENS_NOT_FOUND = 235,
|
|
|
++ ASGH_ASSOCIATED_INTERFACE_REQUEST = 236,
|
|
|
++ ASGH_RECEIVED_CONTROL_MESSAGE = 237,
|
|
|
++ CSDH_BAD_OWNER = 238,
|
|
|
++ SYNC_COMPOSITOR_NO_LOCAL_SURFACE_ID = 239,
|
|
|
++ WCI_INVALID_FULLSCREEN_OPTIONS = 240,
|
|
|
++ PAYMENTS_WITHOUT_PERMISSION = 241,
|
|
|
++ WEB_BUNDLE_INVALID_NAVIGATION_URL = 242,
|
|
|
++ WCI_INVALID_DOWNLOAD_IMAGE_RESULT = 243,
|
|
|
+
|
|
|
+ // Please add new elements here. The naming convention is abbreviated class
|
|
|
+ // name (e.g. RenderFrameHost becomes RFH) plus a unique description of the
|
|
|
+diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
|
|
|
+index a7c76413b86fac18f6f1f54c87e67218f094e6b2..67deba5a7ab4265c76fe18990ede82c2705efe70 100644
|
|
|
+--- a/content/browser/web_contents/web_contents_impl.cc
|
|
|
++++ b/content/browser/web_contents/web_contents_impl.cc
|
|
|
+@@ -160,6 +160,7 @@
|
|
|
+ #include "third_party/blink/public/common/security/security_style.h"
|
|
|
+ #include "third_party/blink/public/mojom/frame/frame.mojom.h"
|
|
|
+ #include "third_party/blink/public/mojom/frame/fullscreen.mojom.h"
|
|
|
++#include "third_party/blink/public/mojom/image_downloader/image_downloader.mojom.h"
|
|
|
+ #include "third_party/blink/public/mojom/loader/pause_subresource_loading_handle.mojom.h"
|
|
|
+ #include "third_party/blink/public/mojom/mediastream/media_stream.mojom-shared.h"
|
|
|
+ #include "third_party/skia/include/core/SkBitmap.h"
|
|
|
+@@ -4295,18 +4296,18 @@ int WebContentsImpl::DownloadImageInFrame(
|
|
|
+ // respond with a 400 HTTP error code to indicate that something went wrong.
|
|
|
+ GetUIThreadTaskRunner({})->PostTask(
|
|
|
+ FROM_HERE,
|
|
|
+- base::BindOnce(&WebContentsImpl::OnDidDownloadImage,
|
|
|
+- weak_factory_.GetWeakPtr(), std::move(callback),
|
|
|
+- download_id, url, 400, std::vector<SkBitmap>(),
|
|
|
+- std::vector<gfx::Size>()));
|
|
|
++ base::BindOnce(
|
|
|
++ &WebContentsImpl::OnDidDownloadImage, weak_factory_.GetWeakPtr(),
|
|
|
++ initiator_frame->GetWeakPtr(), std::move(callback), download_id,
|
|
|
++ url, 400, std::vector<SkBitmap>(), std::vector<gfx::Size>()));
|
|
|
+ return download_id;
|
|
|
+ }
|
|
|
+
|
|
|
+ mojo_image_downloader->DownloadImage(
|
|
|
+ url, is_favicon, preferred_size, max_bitmap_size, bypass_cache,
|
|
|
+ base::BindOnce(&WebContentsImpl::OnDidDownloadImage,
|
|
|
+- weak_factory_.GetWeakPtr(), std::move(callback),
|
|
|
+- download_id, url));
|
|
|
++ weak_factory_.GetWeakPtr(), initiator_frame->GetWeakPtr(),
|
|
|
++ std::move(callback), download_id, url));
|
|
|
+ return download_id;
|
|
|
+ }
|
|
|
+
|
|
|
+@@ -6829,12 +6830,28 @@ bool WebContentsImpl::CompletedFirstVisuallyNonEmptyPaint() {
|
|
|
+ }
|
|
|
+
|
|
|
+ void WebContentsImpl::OnDidDownloadImage(
|
|
|
++ base::WeakPtr<RenderFrameHostImpl> rfh,
|
|
|
+ ImageDownloadCallback callback,
|
|
|
+ int id,
|
|
|
+ const GURL& image_url,
|
|
|
+ int32_t http_status_code,
|
|
|
+ const std::vector<SkBitmap>& images,
|
|
|
+ const std::vector<gfx::Size>& original_image_sizes) {
|
|
|
++
|
|
|
++ // Guard against buggy or compromised renderers that could violate the API
|
|
|
++ // contract that |images| and |original_image_sizes| must have the same
|
|
|
++ // length.
|
|
|
++ if (images.size() != original_image_sizes.size()) {
|
|
|
++ if (rfh) {
|
|
|
++ ReceivedBadMessage(rfh->GetProcess(),
|
|
|
++ bad_message::WCI_INVALID_DOWNLOAD_IMAGE_RESULT);
|
|
|
++ }
|
|
|
++ // Respond with a 400 to indicate that something went wrong.
|
|
|
++ std::move(callback).Run(id, 400, image_url, std::vector<SkBitmap>(),
|
|
|
++ std::vector<gfx::Size>());
|
|
|
++ return;
|
|
|
++ }
|
|
|
++
|
|
|
+ std::move(callback).Run(id, http_status_code, image_url, images,
|
|
|
+ original_image_sizes);
|
|
|
+ }
|
|
|
+diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
|
|
|
+index 14586316aedab1e99f1b700ecc0e092c86414ee3..ffd53b3dc3ef0553711106f302e9f69b76b49081 100644
|
|
|
+--- a/content/browser/web_contents/web_contents_impl.h
|
|
|
++++ b/content/browser/web_contents/web_contents_impl.h
|
|
|
+@@ -1403,7 +1403,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
|
|
|
+ std::set<RenderWidgetHostView*>& result);
|
|
|
+
|
|
|
+ // Called with the result of a DownloadImage() request.
|
|
|
+- void OnDidDownloadImage(ImageDownloadCallback callback,
|
|
|
++ void OnDidDownloadImage(base::WeakPtr<RenderFrameHostImpl> rfh,
|
|
|
++ ImageDownloadCallback callback,
|
|
|
+ int id,
|
|
|
+ const GURL& image_url,
|
|
|
+ int32_t http_status_code,
|
|
|
+diff --git a/content/public/browser/web_contents.h b/content/public/browser/web_contents.h
|
|
|
+index ca7651246f812259e43d5dbc429ae4a95ed60e94..a4572de2ee43ef0c2bb3a785f29974ebb8c1362c 100644
|
|
|
+--- a/content/public/browser/web_contents.h
|
|
|
++++ b/content/public/browser/web_contents.h
|
|
|
+@@ -896,8 +896,9 @@ class WebContents : public PageNavigator,
|
|
|
+ // |bitmaps| will be empty on download failure.
|
|
|
+ // |sizes| are the sizes in pixels of the bitmaps before they were resized due
|
|
|
+ // to the max bitmap size passed to DownloadImage(). Each entry in the bitmaps
|
|
|
+- // vector corresponds to an entry in the sizes vector. If a bitmap was
|
|
|
+- // resized, there should be a single returned bitmap.
|
|
|
++ // vector corresponds to an entry in the sizes vector (both vector sizes are
|
|
|
++ // guaranteed to be equal). If a bitmap was resized, there should be a single
|
|
|
++ // returned bitmap.
|
|
|
+ using ImageDownloadCallback =
|
|
|
+ base::OnceCallback<void(int id,
|
|
|
+ int http_status_code,
|
|
|
+diff --git a/third_party/blink/renderer/modules/image_downloader/image_downloader_impl.cc b/third_party/blink/renderer/modules/image_downloader/image_downloader_impl.cc
|
|
|
+index fb0fc97d689c8c6b1f6f9fa27d250e0438550725..740c6c01318a1d9d2914ddef9949db9ced559d14 100644
|
|
|
+--- a/third_party/blink/renderer/modules/image_downloader/image_downloader_impl.cc
|
|
|
++++ b/third_party/blink/renderer/modules/image_downloader/image_downloader_impl.cc
|
|
|
+@@ -79,7 +79,8 @@ SkBitmap ResizeImage(const SkBitmap& image, uint32_t max_image_size) {
|
|
|
+ // size |max_image_size|. Returns the result if it is not empty. Otherwise,
|
|
|
+ // find the smallest image in the array and resize it proportionally to fit
|
|
|
+ // in a box of size |max_image_size|.
|
|
|
+-// Sets |original_image_sizes| to the sizes of |images| before resizing.
|
|
|
++// Sets |original_image_sizes| to the sizes of |images| before resizing. Both
|
|
|
++// output vectors are guaranteed to have the same size.
|
|
|
+ void FilterAndResizeImagesForMaximalSize(
|
|
|
+ const WTF::Vector<SkBitmap>& unfiltered,
|
|
|
+ uint32_t max_image_size,
|
|
|
+@@ -202,6 +203,8 @@ void ImageDownloaderImpl::DidDownloadImage(
|
|
|
+ FilterAndResizeImagesForMaximalSize(images, max_image_size, &result_images,
|
|
|
+ &result_original_image_sizes);
|
|
|
+
|
|
|
++ DCHECK_EQ(result_images.size(), result_original_image_sizes.size());
|
|
|
++
|
|
|
+ std::move(callback).Run(http_status_code, result_images,
|
|
|
+ result_original_image_sizes);
|
|
|
+ }
|