From b5f80d7d58b57ef4c7b67faf13072a2a99736d7f Mon Sep 17 00:00:00 2001 From: Serg Date: Tue, 14 Jun 2022 13:43:51 -0400 Subject: [PATCH 1/3] Prevents Ethereum DApps requests popups from backgrounded pages --- .../brave_wallet_provider_delegate_impl.cc | 7 ++++++ .../brave_wallet_provider_delegate_impl.h | 1 + .../browser/brave_wallet_provider_delegate.h | 1 + .../browser/ethereum_provider_impl.cc | 24 +++++++++++++++++++ components/resources/wallet_strings.grdp | 1 + 5 files changed, 34 insertions(+) diff --git a/browser/brave_wallet/brave_wallet_provider_delegate_impl.cc b/browser/brave_wallet/brave_wallet_provider_delegate_impl.cc index 40051d6966d7..dbe72b9bec2a 100644 --- a/browser/brave_wallet/brave_wallet_provider_delegate_impl.cc +++ b/browser/brave_wallet/brave_wallet_provider_delegate_impl.cc @@ -17,6 +17,7 @@ #include "brave/components/permissions/contexts/brave_wallet_permission_context.h" #include "components/content_settings/core/common/content_settings.h" #include "content/public/browser/render_frame_host.h" +#include "content/public/browser/visibility.h" #include "content/public/browser/web_contents.h" namespace brave_wallet { @@ -93,6 +94,12 @@ url::Origin BraveWalletProviderDelegateImpl::GetOrigin() const { return rfh ? rfh->GetLastCommittedOrigin() : url::Origin(); } +bool BraveWalletProviderDelegateImpl::IsTabVisible() { + return web_contents_ + ? web_contents_->GetVisibility() == content::Visibility::VISIBLE + : false; +} + void BraveWalletProviderDelegateImpl::ShowPanel() { ::brave_wallet::ShowPanel(web_contents_); } diff --git a/browser/brave_wallet/brave_wallet_provider_delegate_impl.h b/browser/brave_wallet/brave_wallet_provider_delegate_impl.h index 0b9f77d80ec7..9670d7e7e743 100644 --- a/browser/brave_wallet/brave_wallet_provider_delegate_impl.h +++ b/browser/brave_wallet/brave_wallet_provider_delegate_impl.h @@ -35,6 +35,7 @@ class BraveWalletProviderDelegateImpl : public BraveWalletProviderDelegate, const BraveWalletProviderDelegateImpl&) = delete; ~BraveWalletProviderDelegateImpl() override; + bool IsTabVisible() override; void ShowPanel() override; void WalletInteractionDetected() override; void ShowWalletOnboarding() override; diff --git a/components/brave_wallet/browser/brave_wallet_provider_delegate.h b/components/brave_wallet/browser/brave_wallet_provider_delegate.h index af1ac48ea0ad..509468426fab 100644 --- a/components/brave_wallet/browser/brave_wallet_provider_delegate.h +++ b/components/brave_wallet/browser/brave_wallet_provider_delegate.h @@ -31,6 +31,7 @@ class BraveWalletProviderDelegate { delete; virtual ~BraveWalletProviderDelegate() = default; + virtual bool IsTabVisible() = 0; virtual void ShowPanel() = 0; virtual void WalletInteractionDetected() = 0; virtual void ShowWalletOnboarding() = 0; diff --git a/components/brave_wallet/browser/ethereum_provider_impl.cc b/components/brave_wallet/browser/ethereum_provider_impl.cc index c32b58798b13..c30014a17a21 100644 --- a/components/brave_wallet/browser/ethereum_provider_impl.cc +++ b/components/brave_wallet/browser/ethereum_provider_impl.cc @@ -1000,6 +1000,23 @@ void EthereumProviderImpl::CommonRequestOrSendAsync(base::Value input_value, return; } + // That check prevents from pop ups from backgrounded pages. + // We need to add any method that requires a dialog to interact with. + if ((method == kEthRequestAccounts || method == kAddEthereumChainMethod || + method == kSwitchEthereumChainMethod || method == kEthSendTransaction || + method == kEthSign || method == kPersonalSign || + method == kPersonalEcRecover || method == kEthSignTypedDataV3 || + method == kEthSignTypedDataV4 || method == kEthGetEncryptionPublicKey || + method == kEthDecrypt || method == kWalletWatchAsset || + method == kRequestPermissionsMethod) && + !delegate_->IsTabVisible()) { + SendErrorOnRequest( + mojom::ProviderError::kResourceUnavailable, + l10n_util::GetStringUTF8(IDS_WALLET_TAB_IS_NOT_ACTIVE_ERROR), + std::move(callback), base::Value()); + return; + } + if (method == kEthAccounts) { GetAllowedAccounts( false, @@ -1244,6 +1261,13 @@ void EthereumProviderImpl::ContinueRequestEthereumPermissions( } void EthereumProviderImpl::Enable(EnableCallback callback) { + if (!delegate_->IsTabVisible()) { + SendErrorOnRequest( + mojom::ProviderError::kResourceUnavailable, + l10n_util::GetStringUTF8(IDS_WALLET_TAB_IS_NOT_ACTIVE_ERROR), + std::move(callback), base::Value()); + return; + } RequestEthereumPermissions(std::move(callback), base::Value(), "", delegate_->GetOrigin()); delegate_->WalletInteractionDetected(); diff --git a/components/resources/wallet_strings.grdp b/components/resources/wallet_strings.grdp index 27bca055b9a5..eac0eb2a8a9c 100644 --- a/components/resources/wallet_strings.grdp +++ b/components/resources/wallet_strings.grdp @@ -468,4 +468,5 @@ Transaction confirmed Transaction error Transaction dropped + The tab is not active From 5a17e856699128e84b101580dd49d548b82d5fd3 Mon Sep 17 00:00:00 2001 From: Serg Date: Tue, 14 Jun 2022 13:59:49 -0400 Subject: [PATCH 2/3] Adds iOS handlers for DApps isTabVisible functionality --- .../brave_wallet/brave_wallet_provider_delegate_ios+private.h | 1 + .../api/brave_wallet/brave_wallet_provider_delegate_ios.h | 1 + .../api/brave_wallet/brave_wallet_provider_delegate_ios.mm | 4 ++++ 3 files changed, 6 insertions(+) diff --git a/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios+private.h b/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios+private.h index 0bb90c19c95d..084d32ba6698 100644 --- a/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios+private.h +++ b/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios+private.h @@ -25,6 +25,7 @@ class BraveWalletProviderDelegateBridge private: __weak id bridge_; + bool IsTabVisible() override; void ShowPanel() override; void WalletInteractionDetected() override; url::Origin GetOrigin() const override; diff --git a/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios.h b/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios.h index 1eef1dcc0e7f..67fe0d8fb7bb 100644 --- a/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios.h +++ b/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios.h @@ -24,6 +24,7 @@ typedef void (^GetAllowedAccountsCallback)(bool success, OBJC_EXPORT @protocol BraveWalletProviderDelegate +- (bool)isTabVisible; - (void)showPanel; - (URLOriginIOS*)getOrigin; - (void)walletInteractionDetected; diff --git a/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios.mm b/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios.mm index 262b27a5b4c7..82521992d4e2 100644 --- a/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios.mm +++ b/ios/browser/api/brave_wallet/brave_wallet_provider_delegate_ios.mm @@ -14,6 +14,10 @@ namespace brave_wallet { +bool BraveWalletProviderDelegateBridge::IsTabVisible() { + return [bridge_ isTabVisible]; +} + void BraveWalletProviderDelegateBridge::ShowPanel() { [bridge_ showPanel]; } From 86054487559919c2ecb66bfde7281141e93bc995 Mon Sep 17 00:00:00 2001 From: Serg Date: Tue, 14 Jun 2022 22:07:34 -0400 Subject: [PATCH 3/3] Adds browser tests on Ethereum DApp sending a request inside an inactive tab --- .../ethereum_provider_browsertest.cc | 145 ++++++++++++++++++ test/BUILD.gn | 1 + test/data/brave-wallet/ethereum_provider.html | 32 ++++ 3 files changed, 178 insertions(+) create mode 100644 browser/brave_wallet/ethereum_provider_browsertest.cc create mode 100644 test/data/brave-wallet/ethereum_provider.html diff --git a/browser/brave_wallet/ethereum_provider_browsertest.cc b/browser/brave_wallet/ethereum_provider_browsertest.cc new file mode 100644 index 000000000000..c3d6367e1c82 --- /dev/null +++ b/browser/brave_wallet/ethereum_provider_browsertest.cc @@ -0,0 +1,145 @@ +/* Copyright (c) 2022 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "base/command_line.h" +#include "base/memory/raw_ptr.h" +#include "base/path_service.h" +#include "base/test/bind.h" +#include "brave/browser/brave_wallet/keyring_service_factory.h" +#include "brave/components/brave_wallet/browser/keyring_service.h" +#include "brave/components/constants/brave_paths.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/ui_test_utils.h" +#include "components/network_session_configurator/common/network_switches.h" +#include "content/public/browser/web_contents.h" +#include "content/public/test/browser_test.h" +#include "content/public/test/browser_test_utils.h" +#include "content/public/test/test_utils.h" +#include "net/dns/mock_host_resolver.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "url/gurl.h" + +namespace { + +std::string CheckForEventScript(const std::string& event_var) { + return base::StringPrintf(R"(function waitForEvent() { + if (%s) { + window.domAutomationController.send(true); + } + } + setInterval(waitForEvent, 100);)", + event_var.c_str()); +} + +} // namespace + +namespace brave_wallet { + +class EthereumProviderBrowserTest : public InProcessBrowserTest { + public: + EthereumProviderBrowserTest() + : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {} + + ~EthereumProviderBrowserTest() override = default; + + void SetUpCommandLine(base::CommandLine* command_line) override { + InProcessBrowserTest::SetUpCommandLine(command_line); + command_line->AppendSwitch(switches::kIgnoreCertificateErrors); + } + + void SetUpOnMainThread() override { + host_resolver()->AddRule("*", "127.0.0.1"); + + brave::RegisterPathProvider(); + base::FilePath test_data_dir; + base::PathService::Get(brave::DIR_TEST_DATA, &test_data_dir); + test_data_dir = test_data_dir.AppendASCII("brave-wallet"); + https_server_.ServeFilesFromDirectory(test_data_dir); + https_server_.SetSSLConfig(net::EmbeddedTestServer::CERT_TEST_NAMES); + ASSERT_TRUE(https_server()->Start()); + + keyring_service_ = + KeyringServiceFactory::GetServiceForContext(browser()->profile()); + } + + content::WebContents* web_contents() { + return browser()->tab_strip_model()->GetActiveWebContents(); + } + + net::EmbeddedTestServer* https_server() { return &https_server_; } + + void RestoreWallet() { + const char mnemonic[] = + "drip caution abandon festival order clown oven regular absorb " + "evidence crew where"; + base::RunLoop run_loop; + keyring_service_->RestoreWallet( + mnemonic, "brave123", false, + base::BindLambdaForTesting([&](bool success) { + ASSERT_TRUE(success); + run_loop.Quit(); + })); + run_loop.Run(); + } + + private: + net::test_server::EmbeddedTestServer https_server_; + raw_ptr keyring_service_ = nullptr; +}; + +IN_PROC_BROWSER_TEST_F(EthereumProviderBrowserTest, InactiveTabRequest) { + RestoreWallet(); + GURL url = https_server()->GetURL("a.com", "/ethereum_provider.html"); + + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + content::WebContents* first_tab_web_contents = web_contents(); + + // Add a new tab and switch to it + ASSERT_TRUE(AddTabAtIndex(1, url, ui::PAGE_TRANSITION_TYPED)); + ASSERT_EQ(browser()->tab_strip_model()->active_index(), 1); + + // Add an asset using the first page web contents + ASSERT_TRUE( + ExecJs(first_tab_web_contents, + "wallet_watchAsset('ERC20', " + "'0x6B175474E89094C44Da98b954EedeAC495271d0F', 'USDC', 6)")); + base::RunLoop().RunUntilIdle(); + + auto result_first = + EvalJs(first_tab_web_contents, CheckForEventScript("inactiveTabError"), + content::EXECUTE_SCRIPT_USE_MANUAL_REPLY); + EXPECT_EQ(base::Value(true), result_first.value); +} + +IN_PROC_BROWSER_TEST_F(EthereumProviderBrowserTest, ActiveTabRequest) { + RestoreWallet(); + GURL url = https_server()->GetURL("a.com", "/ethereum_provider.html"); + + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + + // Add a new tab and switch to it + ASSERT_TRUE(AddTabAtIndex(1, url, ui::PAGE_TRANSITION_TYPED)); + ASSERT_EQ(browser()->tab_strip_model()->active_index(), 1); + + // Switch back to first tab + browser()->tab_strip_model()->ActivateTabAt(0); + ASSERT_EQ(browser()->tab_strip_model()->active_index(), 0); + + // Add an asset using the first page web contents + ASSERT_TRUE( + ExecJs(web_contents(), + "wallet_watchAsset('ERC20', " + "'0x6B175474E89094C44Da98b954EedeAC495271d0F', 'USDC', 6)")); + base::RunLoop().RunUntilIdle(); + + auto result_first = + EvalJs(web_contents(), CheckForEventScript("!inactiveTabError"), + content::EXECUTE_SCRIPT_USE_MANUAL_REPLY); + EXPECT_EQ(base::Value(true), result_first.value); +} + +} // namespace brave_wallet diff --git a/test/BUILD.gn b/test/BUILD.gn index f1fb6e19604d..df736f87a448 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -648,6 +648,7 @@ if (!is_android) { "//brave/browser/brave_wallet/brave_wallet_service_browsertest.cc", "//brave/browser/brave_wallet/brave_wallet_sign_message_browsertest.cc", "//brave/browser/brave_wallet/brave_wallet_tab_helper_browsertest.cc", + "//brave/browser/brave_wallet/ethereum_provider_browsertest.cc", "//brave/browser/brave_wallet/send_transaction_browsertest.cc", "//brave/browser/brave_wallet/solana_provider_browsertest.cc", "//brave/browser/brave_wallet/solana_provider_renderer_browsertest.cc", diff --git a/test/data/brave-wallet/ethereum_provider.html b/test/data/brave-wallet/ethereum_provider.html new file mode 100644 index 000000000000..a807b94b42bf --- /dev/null +++ b/test/data/brave-wallet/ethereum_provider.html @@ -0,0 +1,32 @@ + + + + + + + +