Commit 3f3295df authored by Oksana Zhuravlova's avatar Oksana Zhuravlova Committed by Commit Bot

[mojo] Call ReportBadMessage if no browser binder found

This change modifies BrowserInterfaceBrokerImpl::GetInterface() to
trigger a bad message error on its host's receiver.
Browser test added to verify the behaviour.
Empty binders added for interfaces requested by blink but
not bound by content (will mostly be called in tests).

Bug: 1047680
Change-Id: I242cb11fff56666b6b6bd29e49c2180954aa1227
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062696Reviewed-by: default avatarcalamity <calamity@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747877}
parent cd335066
...@@ -14,3 +14,6 @@ interface Bar { ...@@ -14,3 +14,6 @@ interface Bar {
GetBar() => (string value); GetBar() => (string value);
}; };
interface Baz {
GetBaz() => (string value);
};
...@@ -250,3 +250,28 @@ IN_PROC_BROWSER_TEST_F(MojoWebUIControllerBrowserTest, ...@@ -250,3 +250,28 @@ IN_PROC_BROWSER_TEST_F(MojoWebUIControllerBrowserTest,
.error.empty()); .error.empty());
EXPECT_TRUE(web_contents->IsCrashed()); EXPECT_TRUE(web_contents->IsCrashed());
} }
// Attempting to access bindings crashes the renderer when access not allowed.
IN_PROC_BROWSER_TEST_F(MojoWebUIControllerBrowserTest, CrashForNoBinder) {
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(NavigateToURL(web_contents, content::GetWebUIURL("foo")));
content::ScopedAllowRendererCrashes allow;
content::RenderProcessHostWatcher watcher(
web_contents->GetMainFrame()->GetProcess(),
content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
// Attempt to bind an interface with no browser binders registered.
EXPECT_FALSE(content::EvalJs(web_contents,
"(async () => {"
" let bazRemote = test.mojom.Baz.getRemote();"
" let resp = await bazRemote.getBaz();"
" return resp.value;"
"})()")
.error.empty());
watcher.Wait();
EXPECT_TRUE(web_contents->IsCrashed());
}
...@@ -1802,6 +1802,10 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, DoesNotCacheIfWebGL) { ...@@ -1802,6 +1802,10 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, DoesNotCacheIfWebGL) {
FROM_HERE); FROM_HERE);
} }
// Since blink::mojom::HidService binder is not added in
// content/browser/browser_interface_binders.cc for Android, this test is not
// applicable for this OS.
#if !defined(OS_ANDROID)
IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, DoesNotCacheIfWebHID) { IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, DoesNotCacheIfWebHID) {
ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(embedded_test_server()->Start());
...@@ -1835,6 +1839,7 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, DoesNotCacheIfWebHID) { ...@@ -1835,6 +1839,7 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, DoesNotCacheIfWebHID) {
ExpectBlocklistedFeature( ExpectBlocklistedFeature(
blink::scheduler::WebSchedulerTrackedFeature::kWebHID, FROM_HERE); blink::scheduler::WebSchedulerTrackedFeature::kWebHID, FROM_HERE);
} }
#endif // !defined(OS_ANDROID)
IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
DoesNotCacheIfAcquiredWakeLock) { DoesNotCacheIfAcquiredWakeLock) {
......
...@@ -80,7 +80,9 @@ ...@@ -80,7 +80,9 @@
#include "third_party/blink/public/mojom/geolocation/geolocation_service.mojom.h" #include "third_party/blink/public/mojom/geolocation/geolocation_service.mojom.h"
#include "third_party/blink/public/mojom/idle/idle_manager.mojom.h" #include "third_party/blink/public/mojom/idle/idle_manager.mojom.h"
#include "third_party/blink/public/mojom/indexeddb/indexeddb.mojom.h" #include "third_party/blink/public/mojom/indexeddb/indexeddb.mojom.h"
#include "third_party/blink/public/mojom/insecure_input/insecure_input_service.mojom.h"
#include "third_party/blink/public/mojom/keyboard_lock/keyboard_lock.mojom.h" #include "third_party/blink/public/mojom/keyboard_lock/keyboard_lock.mojom.h"
#include "third_party/blink/public/mojom/loader/navigation_predictor.mojom.h"
#include "third_party/blink/public/mojom/locks/lock_manager.mojom.h" #include "third_party/blink/public/mojom/locks/lock_manager.mojom.h"
#include "third_party/blink/public/mojom/mediasession/media_session.mojom.h" #include "third_party/blink/public/mojom/mediasession/media_session.mojom.h"
#include "third_party/blink/public/mojom/mediastream/media_devices.mojom.h" #include "third_party/blink/public/mojom/mediastream/media_devices.mojom.h"
...@@ -90,6 +92,7 @@ ...@@ -90,6 +92,7 @@
#include "third_party/blink/public/mojom/payments/payment_app.mojom.h" #include "third_party/blink/public/mojom/payments/payment_app.mojom.h"
#include "third_party/blink/public/mojom/permissions/permission.mojom.h" #include "third_party/blink/public/mojom/permissions/permission.mojom.h"
#include "third_party/blink/public/mojom/picture_in_picture/picture_in_picture.mojom.h" #include "third_party/blink/public/mojom/picture_in_picture/picture_in_picture.mojom.h"
#include "third_party/blink/public/mojom/prerender/prerender.mojom.h"
#include "third_party/blink/public/mojom/presentation/presentation.mojom.h" #include "third_party/blink/public/mojom/presentation/presentation.mojom.h"
#include "third_party/blink/public/mojom/quota/quota_dispatcher_host.mojom.h" #include "third_party/blink/public/mojom/quota/quota_dispatcher_host.mojom.h"
#include "third_party/blink/public/mojom/sms/sms_receiver.mojom.h" #include "third_party/blink/public/mojom/sms/sms_receiver.mojom.h"
...@@ -104,6 +107,7 @@ ...@@ -104,6 +107,7 @@
#include "third_party/blink/public/mojom/webtransport/quic_transport_connector.mojom.h" #include "third_party/blink/public/mojom/webtransport/quic_transport_connector.mojom.h"
#include "third_party/blink/public/mojom/worker/dedicated_worker_host_factory.mojom.h" #include "third_party/blink/public/mojom/worker/dedicated_worker_host_factory.mojom.h"
#include "third_party/blink/public/mojom/worker/shared_worker_connector.mojom.h" #include "third_party/blink/public/mojom/worker/shared_worker_connector.mojom.h"
#include "third_party/blink/public/public_buildflags.h"
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
#include "content/browser/installedapp/installed_app_provider_impl.h" #include "content/browser/installedapp/installed_app_provider_impl.h"
...@@ -120,6 +124,7 @@ ...@@ -120,6 +124,7 @@
#include "services/device/public/mojom/nfc.mojom.h" #include "services/device/public/mojom/nfc.mojom.h"
#include "third_party/blink/public/mojom/hid/hid.mojom.h" #include "third_party/blink/public/mojom/hid/hid.mojom.h"
#include "third_party/blink/public/mojom/input/input_host.mojom.h" #include "third_party/blink/public/mojom/input/input_host.mojom.h"
#include "third_party/blink/public/mojom/unhandled_tap_notifier/unhandled_tap_notifier.mojom.h"
#endif #endif
#if BUILDFLAG(ENABLE_MEDIA_REMOTING) #if BUILDFLAG(ENABLE_MEDIA_REMOTING)
...@@ -449,6 +454,13 @@ BindServiceWorkerReceiverForOriginAndCOEP( ...@@ -449,6 +454,13 @@ BindServiceWorkerReceiverForOriginAndCOEP(
base::Unretained(host), method, cross_origin_embedder_policy); base::Unretained(host), method, cross_origin_embedder_policy);
} }
template <typename Interface>
void EmptyBinderForFrame(RenderFrameHost* host,
mojo::PendingReceiver<Interface> receiver) {
DLOG(ERROR) << "Empty binder for interface " << Interface::Name_
<< " for the frame/document scope";
}
VibrationManagerBinder& GetVibrationManagerBinderOverride() { VibrationManagerBinder& GetVibrationManagerBinderOverride() {
static base::NoDestructor<VibrationManagerBinder> binder; static base::NoDestructor<VibrationManagerBinder> binder;
return *binder; return *binder;
...@@ -672,6 +684,23 @@ void PopulateFrameBinders(RenderFrameHostImpl* host, ...@@ -672,6 +684,23 @@ void PopulateFrameBinders(RenderFrameHostImpl* host,
void PopulateBinderMapWithContext( void PopulateBinderMapWithContext(
RenderFrameHostImpl* host, RenderFrameHostImpl* host,
service_manager::BinderMapWithContext<RenderFrameHost*>* map) { service_manager::BinderMapWithContext<RenderFrameHost*>* map) {
// Register empty binders for interfaces not bound by content but requested
// by blink.
// This avoids renderer kills when no binder is found in the absence of the
// production embedder (such as in tests).
map->Add<blink::mojom::InsecureInputService>(base::BindRepeating(
&EmptyBinderForFrame<blink::mojom::InsecureInputService>));
map->Add<blink::mojom::PrerenderProcessor>(base::BindRepeating(
&EmptyBinderForFrame<blink::mojom::PrerenderProcessor>));
map->Add<payments::mojom::PaymentRequest>(base::BindRepeating(
&EmptyBinderForFrame<payments::mojom::PaymentRequest>));
map->Add<blink::mojom::AnchorElementMetricsHost>(base::BindRepeating(
&EmptyBinderForFrame<blink::mojom::AnchorElementMetricsHost>));
#if BUILDFLAG(ENABLE_UNHANDLED_TAP)
map->Add<blink::mojom::UnhandledTapNotifier>(base::BindRepeating(
&EmptyBinderForFrame<blink::mojom::UnhandledTapNotifier>));
#endif
map->Add<blink::mojom::BackgroundFetchService>( map->Add<blink::mojom::BackgroundFetchService>(
base::BindRepeating(&BackgroundFetchServiceImpl::CreateForFrame)); base::BindRepeating(&BackgroundFetchServiceImpl::CreateForFrame));
map->Add<blink::mojom::ColorChooserFactory>( map->Add<blink::mojom::ColorChooserFactory>(
...@@ -724,6 +753,10 @@ const url::Origin& GetContextForHost(DedicatedWorkerHost* host) { ...@@ -724,6 +753,10 @@ const url::Origin& GetContextForHost(DedicatedWorkerHost* host) {
void PopulateDedicatedWorkerBinders(DedicatedWorkerHost* host, void PopulateDedicatedWorkerBinders(DedicatedWorkerHost* host,
service_manager::BinderMap* map) { service_manager::BinderMap* map) {
// Do nothing for interfaces that the renderer might request, but doesn't
// always expect to be bound.
map->Add<blink::mojom::FeatureObserver>(base::DoNothing());
// static binders // static binders
map->Add<blink::mojom::ScreenEnumeration>( map->Add<blink::mojom::ScreenEnumeration>(
base::BindRepeating(&ScreenEnumerationImpl::Create)); base::BindRepeating(&ScreenEnumerationImpl::Create));
...@@ -808,6 +841,10 @@ url::Origin GetContextForHost(SharedWorkerHost* host) { ...@@ -808,6 +841,10 @@ url::Origin GetContextForHost(SharedWorkerHost* host) {
void PopulateSharedWorkerBinders(SharedWorkerHost* host, void PopulateSharedWorkerBinders(SharedWorkerHost* host,
service_manager::BinderMap* map) { service_manager::BinderMap* map) {
// Do nothing for interfaces that the renderer might request, but doesn't
// always expect to be bound.
map->Add<blink::mojom::FeatureObserver>(base::DoNothing());
// static binders // static binders
map->Add<blink::mojom::ScreenEnumeration>( map->Add<blink::mojom::ScreenEnumeration>(
base::BindRepeating(&ScreenEnumerationImpl::Create)); base::BindRepeating(&ScreenEnumerationImpl::Create));
...@@ -884,6 +921,10 @@ void PopulateServiceWorkerBinders(ServiceWorkerProviderHost* host, ...@@ -884,6 +921,10 @@ void PopulateServiceWorkerBinders(ServiceWorkerProviderHost* host,
service_manager::BinderMap* map) { service_manager::BinderMap* map) {
DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId()); DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
// Do nothing for interfaces that the renderer might request, but doesn't
// always expect to be bound.
map->Add<blink::mojom::FeatureObserver>(base::DoNothing());
// static binders // static binders
map->Add<blink::mojom::ScreenEnumeration>( map->Add<blink::mojom::ScreenEnumeration>(
base::BindRepeating(&ScreenEnumerationImpl::Create)); base::BindRepeating(&ScreenEnumerationImpl::Create));
......
...@@ -30,8 +30,11 @@ class BrowserInterfaceBrokerImpl : public blink::mojom::BrowserInterfaceBroker { ...@@ -30,8 +30,11 @@ class BrowserInterfaceBrokerImpl : public blink::mojom::BrowserInterfaceBroker {
auto interface_name = receiver.interface_name().value(); auto interface_name = receiver.interface_name().value();
auto pipe = receiver.PassPipe(); auto pipe = receiver.PassPipe();
if (!binder_map_.TryBind(interface_name, &pipe)) { if (!binder_map_.TryBind(interface_name, &pipe)) {
binder_map_with_context_.TryBind(internal::GetContextForHost(host_), if (!binder_map_with_context_.TryBind(internal::GetContextForHost(host_),
interface_name, &pipe); interface_name, &pipe)) {
host_->ReportNoBinderForInterface("No binder found for interface " +
interface_name);
}
} }
} }
......
...@@ -3350,6 +3350,10 @@ void RenderFrameHostImpl::DownloadURL( ...@@ -3350,6 +3350,10 @@ void RenderFrameHostImpl::DownloadURL(
StartDownload(std::move(parameters), std::move(blob_url_token)); StartDownload(std::move(parameters), std::move(blob_url_token));
} }
void RenderFrameHostImpl::ReportNoBinderForInterface(const std::string& error) {
broker_receiver_.ReportBadMessage(error + " for the frame/document scope");
}
void RenderFrameHostImpl::RequestTextSurroundingSelection( void RenderFrameHostImpl::RequestTextSurroundingSelection(
blink::mojom::LocalFrame::GetTextSurroundingSelectionCallback callback, blink::mojom::LocalFrame::GetTextSurroundingSelectionCallback callback,
int max_length) { int max_length) {
......
...@@ -1416,6 +1416,8 @@ class CONTENT_EXPORT RenderFrameHostImpl ...@@ -1416,6 +1416,8 @@ class CONTENT_EXPORT RenderFrameHostImpl
void ScaleFactorChanged(float scale) override; void ScaleFactorChanged(float scale) override;
void ContentsPreferredSizeChanged(const gfx::Size& pref_size) override; void ContentsPreferredSizeChanged(const gfx::Size& pref_size) override;
void ReportNoBinderForInterface(const std::string& error);
protected: protected:
friend class RenderFrameHostFactory; friend class RenderFrameHostFactory;
......
...@@ -116,4 +116,9 @@ ServiceWorkerProviderHost::GetWeakPtr() { ...@@ -116,4 +116,9 @@ ServiceWorkerProviderHost::GetWeakPtr() {
return weak_factory_.GetWeakPtr(); return weak_factory_.GetWeakPtr();
} }
void ServiceWorkerProviderHost::ReportNoBinderForInterface(
const std::string& error) {
broker_receiver_.ReportBadMessage(error + " for the service worker scope");
}
} // namespace content } // namespace content
...@@ -77,6 +77,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost { ...@@ -77,6 +77,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost {
base::WeakPtr<ServiceWorkerProviderHost> GetWeakPtr(); base::WeakPtr<ServiceWorkerProviderHost> GetWeakPtr();
void ReportNoBinderForInterface(const std::string& error);
private: private:
// Unique among all provider hosts. // Unique among all provider hosts.
const int provider_id_; const int provider_id_;
......
...@@ -222,6 +222,10 @@ void DedicatedWorkerHost::StartScriptLoad( ...@@ -222,6 +222,10 @@ void DedicatedWorkerHost::StartScriptLoad(
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
} }
void DedicatedWorkerHost::ReportNoBinderForInterface(const std::string& error) {
broker_receiver_.ReportBadMessage(error + " for the dedicated worker scope");
}
void DedicatedWorkerHost::DidStartScriptLoad( void DedicatedWorkerHost::DidStartScriptLoad(
bool success, bool success,
std::unique_ptr<blink::PendingURLLoaderFactoryBundle> std::unique_ptr<blink::PendingURLLoaderFactoryBundle>
......
...@@ -121,6 +121,8 @@ class DedicatedWorkerHost final : public blink::mojom::DedicatedWorkerHost, ...@@ -121,6 +121,8 @@ class DedicatedWorkerHost final : public blink::mojom::DedicatedWorkerHost,
mojo::PendingRemote<blink::mojom::BlobURLToken> blob_url_token, mojo::PendingRemote<blink::mojom::BlobURLToken> blob_url_token,
mojo::Remote<blink::mojom::DedicatedWorkerHostFactoryClient> client); mojo::Remote<blink::mojom::DedicatedWorkerHostFactoryClient> client);
void ReportNoBinderForInterface(const std::string& error);
private: private:
// RenderProcessHostObserver: // RenderProcessHostObserver:
void RenderProcessExited(RenderProcessHost* render_process_host, void RenderProcessExited(RenderProcessHost* render_process_host,
......
...@@ -414,6 +414,10 @@ base::WeakPtr<SharedWorkerHost> SharedWorkerHost::AsWeakPtr() { ...@@ -414,6 +414,10 @@ base::WeakPtr<SharedWorkerHost> SharedWorkerHost::AsWeakPtr() {
return weak_factory_.GetWeakPtr(); return weak_factory_.GetWeakPtr();
} }
void SharedWorkerHost::ReportNoBinderForInterface(const std::string& error) {
broker_receiver_.ReportBadMessage(error + " for the shared worker scope");
}
void SharedWorkerHost::AddClient( void SharedWorkerHost::AddClient(
mojo::PendingRemote<blink::mojom::SharedWorkerClient> client, mojo::PendingRemote<blink::mojom::SharedWorkerClient> client,
GlobalFrameRoutingId client_render_frame_host_id, GlobalFrameRoutingId client_render_frame_host_id,
......
...@@ -147,6 +147,8 @@ class CONTENT_EXPORT SharedWorkerHost : public blink::mojom::SharedWorkerHost, ...@@ -147,6 +147,8 @@ class CONTENT_EXPORT SharedWorkerHost : public blink::mojom::SharedWorkerHost,
base::WeakPtr<SharedWorkerHost> AsWeakPtr(); base::WeakPtr<SharedWorkerHost> AsWeakPtr();
void ReportNoBinderForInterface(const std::string& error);
private: private:
friend class SharedWorkerHostTest; friend class SharedWorkerHostTest;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment