diff --git a/community/chromium/APKBUILD b/community/chromium/APKBUILD index 1468ab1c3815905da7cb0a997e413c0804ca9ba7..78c0070e3ba27ecfc36d3775bbea95d56d943d77 100644 --- a/community/chromium/APKBUILD +++ b/community/chromium/APKBUILD @@ -3,7 +3,7 @@ # Maintainer: psykose <alice@ayaya.dev> pkgname=chromium pkgver=109.0.5414.74 -pkgrel=1 +pkgrel=2 pkgdesc="Chromium web browser" url="https://www.chromium.org/Home" arch="aarch64 armv7 x86_64" @@ -109,6 +109,7 @@ source="https://commondatastorage.googleapis.com/chromium-browser-official/chrom chromium-VirtualCursor-standard-layout.patch chromium-revert-drop-of-system-java.patch chromium-use-alpine-target.patch + crbug-1407202-mediarouter-crash.patch credentials-sys-types-header.patch dns-resolver.patch fix-crashpad.patch @@ -867,6 +868,7 @@ f19ba0c0f542115e6f53019659df256471e811a23d2f37569c9d4dfa265c0c1ace3e62c74d7507f8 ac0a80174f95d733f33ddc06fc88cdcf7db0973378c28d8544dc9c19e2dabeac47f91c99b3e7384f650b3405554a9e222543f0860b6acc407c078a8c9180d727 chromium-VirtualCursor-standard-layout.patch c4654d5b23c6f5d9502507e534fe1951d6749c62251e49b6adfe10d1569431e7f7a5a6fa5ff09ec30984415ced27a5e20985df8c91295de34af3c84557fa5b91 chromium-revert-drop-of-system-java.patch 6d1b3476ef09a501aedb67e086c9539ee9555e894bb7697fbc7ce5d99f4c798768a4cbe71d4044d5d8aaa4606351dd8e880af244e77acf671a17eca6f6ecf534 chromium-use-alpine-target.patch +7e484714bcf060e6165677e6f12a57f1c6381ab2ad3c5e4decc1e1b7ed9aa00b1ad19c62359898c874b98809540230567f267f25abebf48e85740b03138e6b01 crbug-1407202-mediarouter-crash.patch 274858323d040ac8c51bac90b6ef91bb075d7b8d92d73952ed700c10a8bae2c2115fb2a9cc6912de79be226c141d7106839fc3486e22c0206e75cb6d8ff65ee6 credentials-sys-types-header.patch 082d5391c177f6e559963f19f79b6d5277af8dd3d9df600b2450e3fcc12025ade76ad97ee147a0b77442824714bfd9ef17ac5b105281ec008a144d58129b3ca2 dns-resolver.patch 9d1edb1e0624ee61825e3af23fbb8c5dbc09d2b92d7769d19f8ca618edae8de8a3e051fedf4ad92c230e1373dc8495922c46971aef93a580c04ad80bc33516c0 fix-crashpad.patch diff --git a/community/chromium/crbug-1407202-mediarouter-crash.patch b/community/chromium/crbug-1407202-mediarouter-crash.patch new file mode 100644 index 0000000000000000000000000000000000000000..6bd5c3aea53eefb47ccfed331a54993a2d80330c --- /dev/null +++ b/community/chromium/crbug-1407202-mediarouter-crash.patch @@ -0,0 +1,372 @@ +From d0c1f5ee1f56c165bdf550c9e3be0d7313587b80 Mon Sep 17 00:00:00 2001 +From: Elly Fong-Jones <ellyjones@chromium.org> +Date: Wed, 18 Jan 2023 22:33:11 +0000 +Subject: [PATCH] media: untangle MediaRouterUI lifetimes + +Currently, MediaRouterUI is owned by MediaItemUIDeviceSelectorView. +There is an observer method named "OnControllerInvalidated" which +MediaItemUIDeviceSelectorView reacts to by deleting the MediaRouterUI it +owns. However, OnControllerInvalidated can actually be called in two +different situations: + +* From MediaRouterUI::TakeMediaRouteStarter(), in which case the + MediaRouterUI object is *not* being destroyed, but should be, because + it can't be safely used after TakeMediaRouteStarter() ends; +* From MediaRouterUI::~MediaRouterUI(), in which case the MediaRouterUI + object *is* being destroyed already and should not be. + +In the second case, only the fact that libc++ nulls out unique_ptr +before destroying the pointed-to object saves us from a use-after-free; +under libstdc++, we UaF immediately by re-entering the destructor. Even +under libc++ though this is still very dangerous, because any observers +that happened to be registered after MediaItemUIDeviceSelectorView will +be invoked after the destruction of the object they're observing. Right +now there are no such other observers, but the fact remains that this +interface is basically a UaF timebomb. + +This change separates "this object is about to be destroyed" (an +observable state) from "please destroy this object, it is no longer +useful" (a callback that is made to the object's owner) by: + +1. Renaming OnControllerInvalidated to OnControllerDestroying, to make + it very clear what is happening to the object, and +2. Adding a RegisterDestructor method to CastDialogController, which + allows MediaItemUIDeviceSelectorView to pass a callback into + MediaRouterUI which MediaRouterUI can use to arrange for its own + destruction. + +This is still a bit tangled and ungainly, but it's safe. A fuller +writeup is on the linked bug. + +Fixed: 1407202 +Change-Id: Id9410de1fbf2cb42f13957dde316b7c9259f192f +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4165967 +Reviewed-by: Peter Kasting <pkasting@chromium.org> +Reviewed-by: Takumi Fujimoto <takumif@chromium.org> +Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> +Cr-Commit-Position: refs/heads/main@{#1094110} +--- + +diff --git a/chrome/browser/ui/media_router/cast_dialog_controller.h b/chrome/browser/ui/media_router/cast_dialog_controller.h +index 2a8de976..c3c0553 100644 +--- a/chrome/browser/ui/media_router/cast_dialog_controller.h ++++ b/chrome/browser/ui/media_router/cast_dialog_controller.h +@@ -24,10 +24,12 @@ + public: + virtual ~Observer() = default; + +- virtual void OnModelUpdated(const CastDialogModel& model) = 0; ++ virtual void OnModelUpdated(const CastDialogModel& model) {} + +- // Observer should drop its reference to the controller when this is called. +- virtual void OnControllerInvalidated() = 0; ++ // Notifies observers that the observed object is being destroyed. Observers ++ // MUST NOT try to destroy the observed object in response - to manage the ++ // lifetime of a CastDialogController, use RegisterDestructor() below. ++ virtual void OnControllerDestroying() {} + }; + + virtual ~CastDialogController() = default; +@@ -55,6 +57,16 @@ + // intended that this API should only be used to transfer ownership to some + // new component that will want to start casting on this dialog box's behalf. + virtual std::unique_ptr<MediaRouteStarter> TakeMediaRouteStarter() = 0; ++ ++ // Registers a callback for when the CastDialogController has given up ++ // ownership of its MediaRouteStarter and is no longer safe to use. The ++ // provided closure must destroy |this| or otherwise ensure it is never used ++ // again. This method can only be called once. ++ // ++ // TODO(https://crbug.com/1408494): It's awkward that CastDialogController has ++ // a state where it exists but is unsafe to use, and doubly awkward that we ++ // have to paper over that with this callback. Can that be fixed? ++ virtual void RegisterDestructor(base::OnceClosure destructor) = 0; + }; + + } // namespace media_router +diff --git a/chrome/browser/ui/media_router/media_router_ui.cc b/chrome/browser/ui/media_router/media_router_ui.cc +index 1865115f..644d131 100644 +--- a/chrome/browser/ui/media_router/media_router_ui.cc ++++ b/chrome/browser/ui/media_router/media_router_ui.cc +@@ -83,6 +83,9 @@ + MediaRouterUI::~MediaRouterUI() { + if (media_route_starter_) + DetachFromMediaRouteStarter(); ++ for (CastDialogController::Observer& observer : observers_) { ++ observer.OnControllerDestroying(); ++ } + } + + // static +@@ -145,9 +148,6 @@ + } + + void MediaRouterUI::DetachFromMediaRouteStarter() { +- for (CastDialogController::Observer& observer : observers_) +- observer.OnControllerInvalidated(); +- + media_route_starter()->RemovePresentationRequestSourceObserver(this); + media_route_starter()->RemoveMediaSinkWithCastModesObserver(this); + } +@@ -181,8 +181,16 @@ + + std::unique_ptr<MediaRouteStarter> MediaRouterUI::TakeMediaRouteStarter() { + DCHECK(media_route_starter_) << "MediaRouteStarter already taken!"; +- DetachFromMediaRouteStarter(); +- return std::move(media_route_starter_); ++ auto starter = std::move(media_route_starter_); ++ if (destructor_) { ++ std::move(destructor_).Run(); // May destroy `this`. ++ } ++ return starter; ++} ++ ++void MediaRouterUI::RegisterDestructor(base::OnceClosure destructor) { ++ DCHECK(!destructor_); ++ destructor_ = std::move(destructor); + } + + bool MediaRouterUI::CreateRoute(const MediaSink::Id& sink_id, +diff --git a/chrome/browser/ui/media_router/media_router_ui.h b/chrome/browser/ui/media_router/media_router_ui.h +index 5c2f14e..7afe775 100644 +--- a/chrome/browser/ui/media_router/media_router_ui.h ++++ b/chrome/browser/ui/media_router/media_router_ui.h +@@ -100,8 +100,10 @@ + void StopCasting(const std::string& route_id) override; + void ClearIssue(const Issue::Id& issue_id) override; + // Note that |MediaRouterUI| should not be used after |TakeMediaRouteStarter| +- // is called. ++ // is called. To enforce that, |TakeMediaRouteStarter| calls the destructor ++ // callback given to |RegisterDestructor| to destroy itself. + std::unique_ptr<MediaRouteStarter> TakeMediaRouteStarter() override; ++ void RegisterDestructor(base::OnceClosure destructor) override; + + // Requests a route be created from the source mapped to + // |cast_mode|, to the sink given by |sink_id|. +@@ -337,6 +339,8 @@ + raw_ptr<MediaRouter> router_; + raw_ptr<LoggerImpl> logger_; + ++ base::OnceClosure destructor_; ++ + // NOTE: Weak pointers must be invalidated before all other member variables. + // Therefore |weak_factory_| must be placed at the end. + base::WeakPtrFactory<MediaRouterUI> weak_factory_{this}; +diff --git a/chrome/browser/ui/media_router/media_router_ui_unittest.cc b/chrome/browser/ui/media_router/media_router_ui_unittest.cc +index 2cc243d1..c33437b 100644 +--- a/chrome/browser/ui/media_router/media_router_ui_unittest.cc ++++ b/chrome/browser/ui/media_router/media_router_ui_unittest.cc +@@ -80,11 +80,11 @@ + } + + MOCK_METHOD1(OnModelUpdated, void(const CastDialogModel& model)); +- void OnControllerInvalidated() override { ++ void OnControllerDestroying() override { + controller_ = nullptr; +- OnControllerInvalidatedInternal(); ++ OnControllerDestroyingInternal(); + } +- MOCK_METHOD0(OnControllerInvalidatedInternal, void()); ++ MOCK_METHOD0(OnControllerDestroyingInternal, void()); + + private: + raw_ptr<CastDialogController> controller_ = nullptr; +@@ -295,7 +295,7 @@ + }))); + NotifyUiOnRoutesUpdated({route}); + +- EXPECT_CALL(observer, OnControllerInvalidatedInternal()); ++ EXPECT_CALL(observer, OnControllerDestroyingInternal()); + ui_.reset(); + } + +diff --git a/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view.cc b/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view.cc +index 34dad46..d843bba 100644 +--- a/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view.cc ++++ b/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view.cc +@@ -222,6 +222,11 @@ + if (cast_controller) { + cast_controller_ = std::move(cast_controller); + cast_controller_->AddObserver(this); ++ cast_controller_->RegisterDestructor( ++ base::BindOnce(&MediaItemUIDeviceSelectorView::DestroyCastController, ++ // Unretained is safe: this callback is held by ++ // cast_controller_, which is owned by this object. ++ base::Unretained(this))); + } + } + +@@ -499,10 +504,6 @@ + observer.OnMediaItemUIDeviceSelectorUpdated(device_entry_ui_map_); + } + +-void MediaItemUIDeviceSelectorView::OnControllerInvalidated() { +- cast_controller_.reset(); +-} +- + void MediaItemUIDeviceSelectorView::OnDeviceSelected(int tag) { + auto it = device_entry_ui_map_.find(tag); + DCHECK(it != device_entry_ui_map_.end()); +@@ -658,5 +659,9 @@ + weak_ptr_factory_.GetWeakPtr())); + } + ++void MediaItemUIDeviceSelectorView::DestroyCastController() { ++ cast_controller_.reset(); ++} ++ + BEGIN_METADATA(MediaItemUIDeviceSelectorView, views::View) + END_METADATA +diff --git a/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view.h b/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view.h +index e950565..222fc20 100644 +--- a/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view.h ++++ b/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view.h +@@ -81,7 +81,6 @@ + + // media_router::CastDialogController::Observer + void OnModelUpdated(const media_router::CastDialogModel& model) override; +- void OnControllerInvalidated() override; + + // MediaItemUIFooterView::Delegate + void OnDeviceSelected(int tag) override; +@@ -121,6 +120,7 @@ + void RecordCastDeviceCount(); + DeviceEntryUI* GetDeviceEntryUI(views::View* view) const; + void RegisterAudioDeviceCallbacks(); ++ void DestroyCastController(); + + bool has_expand_button_been_shown_ = false; + bool have_devices_been_shown_ = false; +diff --git a/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view_unittest.cc b/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view_unittest.cc +index c3bcc6cc..6ae3dde8 100644 +--- a/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view_unittest.cc ++++ b/chrome/browser/ui/views/global_media_controls/media_item_ui_device_selector_view_unittest.cc +@@ -156,6 +156,7 @@ + MOCK_METHOD1(ClearIssue, void(const media_router::Issue::Id& issue_id)); + MOCK_METHOD0(TakeMediaRouteStarter, + std::unique_ptr<media_router::MediaRouteStarter>()); ++ MOCK_METHOD1(RegisterDestructor, void(base::OnceClosure)); + }; + + } // anonymous namespace +diff --git a/chrome/browser/ui/views/media_router/cast_dialog_coordinator_unittest.cc b/chrome/browser/ui/views/media_router/cast_dialog_coordinator_unittest.cc +index f6c80d6a..2dedc7e 100644 +--- a/chrome/browser/ui/views/media_router/cast_dialog_coordinator_unittest.cc ++++ b/chrome/browser/ui/views/media_router/cast_dialog_coordinator_unittest.cc +@@ -40,6 +40,7 @@ + MOCK_METHOD(void, StopCasting, (const std::string& route_id)); + MOCK_METHOD(void, ClearIssue, (const Issue::Id& issue_id)); + MOCK_METHOD(std::unique_ptr<MediaRouteStarter>, TakeMediaRouteStarter, ()); ++ MOCK_METHOD(void, RegisterDestructor, (base::OnceClosure)); + }; + + class CastDialogCoordinatorTest : public TestWithBrowserView { +diff --git a/chrome/browser/ui/views/media_router/cast_dialog_view.cc b/chrome/browser/ui/views/media_router/cast_dialog_view.cc +index e3c7dadb..711d081 100644 +--- a/chrome/browser/ui/views/media_router/cast_dialog_view.cc ++++ b/chrome/browser/ui/views/media_router/cast_dialog_view.cc +@@ -125,9 +125,9 @@ + observer.OnDialogModelUpdated(this); + } + +-void CastDialogView::OnControllerInvalidated() { ++void CastDialogView::OnControllerDestroying() { + controller_ = nullptr; +- // We don't destroy the dialog here because if the invalidation was caused by ++ // We don't destroy the dialog here because if the destruction was caused by + // activating the toolbar icon in order to close the dialog, then it would + // cause the dialog to immediately open again. + } +diff --git a/chrome/browser/ui/views/media_router/cast_dialog_view.h b/chrome/browser/ui/views/media_router/cast_dialog_view.h +index d87fdda..d44d4e0 100644 +--- a/chrome/browser/ui/views/media_router/cast_dialog_view.h ++++ b/chrome/browser/ui/views/media_router/cast_dialog_view.h +@@ -66,7 +66,7 @@ + + // CastDialogController::Observer: + void OnModelUpdated(const CastDialogModel& model) override; +- void OnControllerInvalidated() override; ++ void OnControllerDestroying() override; + + // views::BubbleDialogDelegateView: + void OnPaint(gfx::Canvas* canvas) override; +diff --git a/chrome/browser/ui/views/media_router/cast_dialog_view_browsertest.cc b/chrome/browser/ui/views/media_router/cast_dialog_view_browsertest.cc +index 1c584120..a7af3c8 100644 +--- a/chrome/browser/ui/views/media_router/cast_dialog_view_browsertest.cc ++++ b/chrome/browser/ui/views/media_router/cast_dialog_view_browsertest.cc +@@ -70,6 +70,7 @@ + override { + return nullptr; + } ++ void RegisterDestructor(base::OnceClosure destructor) override {} + }; + + } // namespace +diff --git a/chrome/browser/ui/views/media_router/cast_dialog_view_unittest.cc b/chrome/browser/ui/views/media_router/cast_dialog_view_unittest.cc +index 5326467..988cb07a 100644 +--- a/chrome/browser/ui/views/media_router/cast_dialog_view_unittest.cc ++++ b/chrome/browser/ui/views/media_router/cast_dialog_view_unittest.cc +@@ -91,6 +91,7 @@ + MOCK_METHOD1(StopCasting, void(const std::string& route_id)); + MOCK_METHOD1(ClearIssue, void(const Issue::Id& issue_id)); + MOCK_METHOD0(TakeMediaRouteStarter, std::unique_ptr<MediaRouteStarter>()); ++ MOCK_METHOD1(RegisterDestructor, void(base::OnceClosure)); + }; + + class CastDialogViewTest : public ChromeViewsTestBase { +diff --git a/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.cc b/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.cc +index ad379b2..244d523 100644 +--- a/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.cc ++++ b/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.cc +@@ -51,7 +51,7 @@ + std::move(context)); + } + +- ShowGlobalMeidaControlsDialog(std::move(context)); ++ ShowGlobalMediaControlsDialog(std::move(context)); + return true; + } + +@@ -155,9 +155,20 @@ + initiator(), std::move(start_presentation_context_)) + : MediaRouterUI::CreateWithDefaultMediaSourceAndMirroring( + initiator()); ++ ui_->RegisterDestructor( ++ base::BindOnce(&MediaRouterDialogControllerViews::DestroyMediaRouterUI, ++ // Safe to use base::Unretained here: the callback being ++ // bound is held by the MediaRouterUI we are creating and ++ // owning, and ownership of |ui_| is never transferred ++ // away from this object. ++ base::Unretained(this))); + } + +-void MediaRouterDialogControllerViews::ShowGlobalMeidaControlsDialog( ++void MediaRouterDialogControllerViews::DestroyMediaRouterUI() { ++ ui_.reset(); ++} ++ ++void MediaRouterDialogControllerViews::ShowGlobalMediaControlsDialog( + std::unique_ptr<StartPresentationContext> context) { + // Show the WebContents requesting a dialog. + initiator()->GetDelegate()->ActivateContents(initiator()); +diff --git a/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h b/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h +index 0a5fdb1..7c97211 100644 +--- a/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h ++++ b/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h +@@ -69,13 +69,14 @@ + // MediaRouterUIService::Observer: + void OnServiceDisabled() override; + +- // Initializes |ui_|. ++ // Initializes and destroys |ui_| respectively. + void InitializeMediaRouterUI(); ++ void DestroyMediaRouterUI(); + + // If there exists a media button, show the GMC dialog anchored to the media + // button. Otherwise, show the dialog anchored to the top center of the web + // contents. +- void ShowGlobalMeidaControlsDialog( ++ void ShowGlobalMediaControlsDialog( + std::unique_ptr<StartPresentationContext> context); + + // Returns the media button from the browser that initiates the request to