updater_tests: lock down test binaries to localhost URL overrides.
Hopefully, this will disincentivize bad actors from taking
UpdaterSetup_test.exe from Chromium src and reconfiguring it to deliver
malware.
Change-Id: I6884dece51acda4887009b64a079ed7138dd55bb
Fixed: 393092963
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6213210
Auto-Submit: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Adam Norberg <norberg@google.com>
Cr-Commit-Position: refs/heads/main@{#1414309}
diff --git a/chrome/updater/external_constants_builder_unittest.cc b/chrome/updater/external_constants_builder_unittest.cc
index 2e80766..bf7fdf4 100644
--- a/chrome/updater/external_constants_builder_unittest.cc
+++ b/chrome/updater/external_constants_builder_unittest.cc
@@ -67,10 +67,10 @@
dict_policies.Set("b", 2);
ExternalConstantsBuilder builder;
- builder.SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
- .SetCrashUploadURL("https://crash.example.com")
- .SetDeviceManagementURL("https://dm.example.com")
- .SetAppLogoURL("https://applogo.example.com/")
+ builder.SetUpdateURL(std::vector<std::string>{"https://localhost/1/www"})
+ .SetCrashUploadURL("https://localhost/1/crash")
+ .SetDeviceManagementURL("https://localhost/1/dm")
+ .SetAppLogoURL("https://localhost/1/applogo/")
.SetUseCUP(false)
.SetInitialDelay(base::Seconds(123))
.SetServerKeepAliveTime(base::Seconds(2))
@@ -90,11 +90,11 @@
std::vector<GURL> urls = verifier->UpdateURL();
ASSERT_EQ(urls.size(), 1ul);
- EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+ EXPECT_EQ(urls[0], GURL("https://localhost/1/www"));
- EXPECT_EQ(verifier->CrashUploadURL(), GURL("https://crash.example.com"));
- EXPECT_EQ(verifier->DeviceManagementURL(), GURL("https://dm.example.com"));
- EXPECT_EQ(verifier->AppLogoURL(), GURL("https://applogo.example.com/"));
+ EXPECT_EQ(verifier->CrashUploadURL(), GURL("https://localhost/1/crash"));
+ EXPECT_EQ(verifier->DeviceManagementURL(), GURL("https://localhost/1/dm"));
+ EXPECT_EQ(verifier->AppLogoURL(), GURL("https://localhost/1/applogo/"));
EXPECT_EQ(verifier->InitialDelay(), base::Seconds(123));
EXPECT_EQ(verifier->ServerKeepAliveTime(), base::Seconds(2));
EXPECT_EQ(verifier->DictPolicies().size(), 2U);
@@ -110,7 +110,7 @@
ExternalConstantsBuilder builder;
EXPECT_TRUE(builder
.SetUpdateURL(std::vector<std::string>{
- "https://www.google.com", "https://www.example.com"})
+ "https://localhost/update", "https://localhost/update2"})
.Overwrite());
scoped_refptr<ExternalConstantsOverrider> verifier =
@@ -121,8 +121,8 @@
std::vector<GURL> urls = verifier->UpdateURL();
ASSERT_EQ(urls.size(), 2ul);
- EXPECT_EQ(urls[0], GURL("https://www.google.com"));
- EXPECT_EQ(urls[1], GURL("https://www.example.com"));
+ EXPECT_EQ(urls[0], GURL("https://localhost/update"));
+ EXPECT_EQ(urls[1], GURL("https://localhost/update2"));
EXPECT_EQ(verifier->CrashUploadURL(), GURL(CRASH_UPLOAD_URL));
EXPECT_EQ(verifier->DeviceManagementURL(),
@@ -137,10 +137,10 @@
ExternalConstantsBuilder builder;
EXPECT_TRUE(builder
.SetUpdateURL(std::vector<std::string>{
- "https://www.google.com", "https://www.example.com"})
- .SetCrashUploadURL("https://crash.example.com")
- .SetDeviceManagementURL("https://dm.example.com")
- .SetAppLogoURL("https://applogo.example.com/")
+ "https://localhost/1/localhost/update", "https://www"})
+ .SetCrashUploadURL("https://localhost/1/crash")
+ .SetDeviceManagementURL("https://localhost/1/dm")
+ .SetAppLogoURL("https://localhost/1/applogo/")
.SetUseCUP(false)
.SetInitialDelay(base::Seconds(123.4))
.SetEnableDiffUpdates(true)
@@ -186,20 +186,20 @@
EXPECT_TRUE(
ExternalConstantsBuilder()
- .SetUpdateURL(std::vector<std::string>{"https://www.google.com"})
- .SetCrashUploadURL("https://crash.google.com")
- .SetDeviceManagementURL("https://dm.google.com")
- .SetAppLogoURL("https://applogo.google.com/")
+ .SetUpdateURL(std::vector<std::string>{"https://localhost/update"})
+ .SetCrashUploadURL("https://localhost/crash")
+ .SetDeviceManagementURL("https://localhost/dm")
+ .SetAppLogoURL("https://localhost/logo/")
.SetUseCUP(true)
.SetInitialDelay(base::Seconds(123.4))
.SetServerKeepAliveTime(base::Seconds(2))
.SetMachineManaged(std::make_optional(true))
.SetDictPolicies(dict_policies)
.SetEnableDiffUpdates(false)
- .SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
- .SetCrashUploadURL("https://crash.example.com")
- .SetDeviceManagementURL("https://dm.example.com")
- .SetAppLogoURL("https://applogo.example.com/")
+ .SetUpdateURL(std::vector<std::string>{"https://localhost/1/www"})
+ .SetCrashUploadURL("https://localhost/1/crash")
+ .SetDeviceManagementURL("https://localhost/1/dm")
+ .SetAppLogoURL("https://localhost/1/applogo/")
.SetUseCUP(false)
.SetInitialDelay(base::Seconds(937.6))
.SetServerKeepAliveTime(base::Seconds(3))
@@ -216,11 +216,11 @@
std::vector<GURL> urls = verifier->UpdateURL();
ASSERT_EQ(urls.size(), 1ul);
- EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+ EXPECT_EQ(urls[0], GURL("https://localhost/1/www"));
- EXPECT_EQ(verifier->CrashUploadURL(), GURL("https://crash.example.com"));
- EXPECT_EQ(verifier->DeviceManagementURL(), GURL("https://dm.example.com"));
- EXPECT_EQ(verifier->AppLogoURL(), GURL("https://applogo.example.com/"));
+ EXPECT_EQ(verifier->CrashUploadURL(), GURL("https://localhost/1/crash"));
+ EXPECT_EQ(verifier->DeviceManagementURL(), GURL("https://localhost/1/dm"));
+ EXPECT_EQ(verifier->AppLogoURL(), GURL("https://localhost/1/applogo/"));
EXPECT_EQ(verifier->InitialDelay(), base::Seconds(937.6));
EXPECT_EQ(verifier->ServerKeepAliveTime(), base::Seconds(3));
EXPECT_EQ(verifier->DictPolicies().size(), 1U);
@@ -238,14 +238,14 @@
dict_policies.Set("b", 2);
EXPECT_TRUE(
- builder.SetUpdateURL(std::vector<std::string>{"https://www.google.com"})
- .SetCrashUploadURL("https://crash.google.com")
- .SetDeviceManagementURL("https://dm.google.com")
- .SetAppLogoURL("https://applogo.google.com/")
+ builder.SetUpdateURL(std::vector<std::string>{"https://localhost/update"})
+ .SetCrashUploadURL("https://localhost/crash")
+ .SetDeviceManagementURL("https://localhost/dm")
+ .SetAppLogoURL("https://localhost/logo/")
.SetUseCUP(false)
.SetInitialDelay(base::Seconds(123.4))
.SetServerKeepAliveTime(base::Seconds(3))
- .SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
+ .SetUpdateURL(std::vector<std::string>{"https://localhost/1/www"})
.SetDictPolicies(dict_policies)
.SetMachineManaged(std::make_optional(true))
.SetEnableDiffUpdates(true)
@@ -260,11 +260,11 @@
std::vector<GURL> urls = verifier->UpdateURL();
ASSERT_EQ(urls.size(), 1ul);
- EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+ EXPECT_EQ(urls[0], GURL("https://localhost/1/www"));
- EXPECT_EQ(verifier->CrashUploadURL(), GURL("https://crash.google.com"));
- EXPECT_EQ(verifier->DeviceManagementURL(), GURL("https://dm.google.com"));
- EXPECT_EQ(verifier->AppLogoURL(), GURL("https://applogo.google.com/"));
+ EXPECT_EQ(verifier->CrashUploadURL(), GURL("https://localhost/crash"));
+ EXPECT_EQ(verifier->DeviceManagementURL(), GURL("https://localhost/dm"));
+ EXPECT_EQ(verifier->AppLogoURL(), GURL("https://localhost/logo/"));
EXPECT_EQ(verifier->InitialDelay(), base::Seconds(123.4));
EXPECT_EQ(verifier->ServerKeepAliveTime(), base::Seconds(3));
EXPECT_EQ(verifier->DictPolicies().size(), 2U);
@@ -321,17 +321,17 @@
dict_policies.Set("b", 2);
EXPECT_TRUE(
- builder.SetUpdateURL(std::vector<std::string>{"https://www.google.com"})
- .SetCrashUploadURL("https://crash.google.com")
- .SetDeviceManagementURL("https://dm.google.com")
- .SetAppLogoURL("https://applogo.google.com/")
+ builder.SetUpdateURL(std::vector<std::string>{"https://localhost/update"})
+ .SetCrashUploadURL("https://localhost/crash")
+ .SetDeviceManagementURL("https://localhost/dm")
+ .SetAppLogoURL("https://localhost/logo/")
.SetUseCUP(false)
.SetInitialDelay(base::Seconds(123.4))
.SetServerKeepAliveTime(base::Seconds(3))
- .SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
- .SetCrashUploadURL("https://crash.example.com")
- .SetDeviceManagementURL("https://dm.example.com")
- .SetAppLogoURL("https://applogo.example.com/")
+ .SetUpdateURL(std::vector<std::string>{"https://localhost/1/www"})
+ .SetCrashUploadURL("https://localhost/1/crash")
+ .SetDeviceManagementURL("https://localhost/1/dm")
+ .SetAppLogoURL("https://localhost/1/applogo/")
.SetDictPolicies(dict_policies)
.SetMachineManaged(std::make_optional(false))
.SetEnableDiffUpdates(true)
@@ -346,11 +346,11 @@
std::vector<GURL> urls = verifier->UpdateURL();
ASSERT_EQ(urls.size(), 1ul);
- EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+ EXPECT_EQ(urls[0], GURL("https://localhost/1/www"));
- EXPECT_EQ(verifier->CrashUploadURL(), GURL("https://crash.example.com"));
- EXPECT_EQ(verifier->DeviceManagementURL(), GURL("https://dm.example.com"));
- EXPECT_EQ(verifier->AppLogoURL(), GURL("https://applogo.example.com/"));
+ EXPECT_EQ(verifier->CrashUploadURL(), GURL("https://localhost/1/crash"));
+ EXPECT_EQ(verifier->DeviceManagementURL(), GURL("https://localhost/1/dm"));
+ EXPECT_EQ(verifier->AppLogoURL(), GURL("https://localhost/1/applogo/"));
EXPECT_EQ(verifier->InitialDelay(), base::Seconds(123.4));
EXPECT_EQ(verifier->ServerKeepAliveTime(), base::Seconds(3));
EXPECT_EQ(verifier->DictPolicies().size(), 2U);
@@ -379,10 +379,10 @@
EXPECT_FALSE(verifier2->UseCUP());
urls = verifier2->UpdateURL();
ASSERT_EQ(urls.size(), 1ul);
- EXPECT_EQ(urls[0], GURL("https://www.example.com"));
- EXPECT_EQ(verifier2->CrashUploadURL(), GURL("https://crash.example.com"));
- EXPECT_EQ(verifier2->DeviceManagementURL(), GURL("https://dm.example.com"));
- EXPECT_EQ(verifier2->AppLogoURL(), GURL("https://applogo.example.com/"));
+ EXPECT_EQ(urls[0], GURL("https://localhost/1/www"));
+ EXPECT_EQ(verifier2->CrashUploadURL(), GURL("https://localhost/1/crash"));
+ EXPECT_EQ(verifier2->DeviceManagementURL(), GURL("https://localhost/1/dm"));
+ EXPECT_EQ(verifier2->AppLogoURL(), GURL("https://localhost/1/applogo/"));
EXPECT_EQ(verifier2->InitialDelay(), base::Seconds(123.4));
EXPECT_EQ(verifier2->ServerKeepAliveTime(), base::Seconds(3));
EXPECT_TRUE(verifier2->IsMachineManaged().has_value());
diff --git a/chrome/updater/external_constants_override.cc b/chrome/updater/external_constants_override.cc
index 4411fe2..5114b13 100644
--- a/chrome/updater/external_constants_override.cc
+++ b/chrome/updater/external_constants_override.cc
@@ -53,6 +53,23 @@
return ret;
}
+// The test binary only ever needs to contact localhost during integration
+// tests. To reduce the program's utility as a mule, crash if there is a
+// non-localhost override.
+GURL CheckURL(const GURL& url) {
+ CHECK(url.is_empty() || url.host() == "localhost" ||
+ url.host() == "127.0.0.1" || url.host() == "not_exist")
+ << "Illegal URL override: " << url;
+ return url;
+}
+
+std::vector<GURL> CheckURLs(const std::vector<GURL>& urls) {
+ for (const auto& url : urls) {
+ CheckURL(url);
+ }
+ return urls;
+}
+
} // anonymous namespace
namespace updater {
@@ -81,9 +98,9 @@
override_values_.Find(kDevOverrideKeyUrl);
switch (update_url_value->type()) {
case base::Value::Type::STRING:
- return {GURL(update_url_value->GetString())};
+ return CheckURLs({GURL(update_url_value->GetString())});
case base::Value::Type::LIST:
- return GURLVectorFromStringList(update_url_value->GetList());
+ return CheckURLs(GURLVectorFromStringList(update_url_value->GetList()));
default:
LOG(FATAL) << "Unexpected type of override[" << kDevOverrideKeyUrl
<< "]: " << base::Value::GetTypeName(update_url_value->type());
@@ -100,7 +117,7 @@
CHECK(crash_upload_url_value->is_string())
<< "Unexpected type of override[" << kDevOverrideKeyCrashUploadUrl
<< "]: " << base::Value::GetTypeName(crash_upload_url_value->type());
- return {GURL(crash_upload_url_value->GetString())};
+ return CheckURL({GURL(crash_upload_url_value->GetString())});
}
GURL ExternalConstantsOverrider::DeviceManagementURL() const {
@@ -112,7 +129,7 @@
CHECK(device_management_url_value->is_string())
<< "Unexpected type of override[" << kDevOverrideKeyDeviceManagementUrl
<< "]: " << base::Value::GetTypeName(device_management_url_value->type());
- return {GURL(device_management_url_value->GetString())};
+ return CheckURL({GURL(device_management_url_value->GetString())});
}
GURL ExternalConstantsOverrider::AppLogoURL() const {
@@ -124,7 +141,7 @@
CHECK(app_logo_url_value->is_string())
<< "Unexpected type of override[" << kDevOverrideKeyAppLogoUrl
<< "]: " << base::Value::GetTypeName(app_logo_url_value->type());
- return {GURL(app_logo_url_value->GetString())};
+ return CheckURL({GURL(app_logo_url_value->GetString())});
}
bool ExternalConstantsOverrider::UseCUP() const {
diff --git a/chrome/updater/external_constants_override_unittest.cc b/chrome/updater/external_constants_override_unittest.cc
index bc556ad..3bb8791 100644
--- a/chrome/updater/external_constants_override_unittest.cc
+++ b/chrome/updater/external_constants_override_unittest.cc
@@ -50,17 +50,18 @@
TEST_F(ExternalConstantsOverriderTest, TestFullOverrides) {
base::Value::Dict overrides;
base::Value::List url_list;
- url_list.Append("https://www.example.com");
- url_list.Append("https://www.google.com");
+ url_list.Append("https://localhost/1/www");
+ url_list.Append("https://localhost/2/www");
base::Value::Dict dict_policies;
dict_policies.Set("a", 1);
dict_policies.Set("b", 2);
overrides.Set(kDevOverrideKeyUseCUP, false);
overrides.Set(kDevOverrideKeyUrl, std::move(url_list));
- overrides.Set(kDevOverrideKeyCrashUploadUrl, "https://crash_test.google.com");
- overrides.Set(kDevOverrideKeyDeviceManagementUrl, "https://dm.google.com");
- overrides.Set(kDevOverrideKeyAppLogoUrl, "https://applogo.google.com/");
+ overrides.Set(kDevOverrideKeyCrashUploadUrl,
+ "https://localhost/2/crash_test");
+ overrides.Set(kDevOverrideKeyDeviceManagementUrl, "https://localhost/2/dm");
+ overrides.Set(kDevOverrideKeyAppLogoUrl, "https://localhost/2/applogo/");
overrides.Set(kDevOverrideKeyInitialDelay, 137.1);
overrides.Set(kDevOverrideKeyServerKeepAliveSeconds, 1);
overrides.Set(kDevOverrideKeyDictPolicies, std::move(dict_policies));
@@ -75,16 +76,17 @@
std::vector<GURL> urls = overrider->UpdateURL();
ASSERT_EQ(urls.size(), 2ul);
- EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+ EXPECT_EQ(urls[0], GURL("https://localhost/1/www"));
EXPECT_TRUE(urls[0].is_valid());
- EXPECT_EQ(urls[1], GURL("https://www.google.com"));
+ EXPECT_EQ(urls[1], GURL("https://localhost/2/www"));
EXPECT_TRUE(urls[1].is_valid());
- EXPECT_EQ(overrider->CrashUploadURL(), GURL("https://crash_test.google.com"));
+ EXPECT_EQ(overrider->CrashUploadURL(),
+ GURL("https://localhost/2/crash_test"));
EXPECT_TRUE(overrider->CrashUploadURL().is_valid());
- EXPECT_EQ(overrider->DeviceManagementURL(), GURL("https://dm.google.com"));
+ EXPECT_EQ(overrider->DeviceManagementURL(), GURL("https://localhost/2/dm"));
EXPECT_TRUE(overrider->DeviceManagementURL().is_valid());
- EXPECT_EQ(overrider->AppLogoURL(), GURL("https://applogo.google.com/"));
+ EXPECT_EQ(overrider->AppLogoURL(), GURL("https://localhost/2/applogo/"));
EXPECT_TRUE(overrider->AppLogoURL().is_valid());
EXPECT_EQ(overrider->InitialDelay(), base::Seconds(137.1));
@@ -98,13 +100,13 @@
TEST_F(ExternalConstantsOverriderTest, TestOverrideUnwrappedURL) {
base::Value::Dict overrides;
- overrides.Set(kDevOverrideKeyUrl, "https://www.example.com");
+ overrides.Set(kDevOverrideKeyUrl, "https://localhost/1/www");
auto overrider = base::MakeRefCounted<ExternalConstantsOverrider>(
std::move(overrides), CreateDefaultExternalConstants());
std::vector<GURL> urls = overrider->UpdateURL();
ASSERT_EQ(urls.size(), 1ul);
- EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+ EXPECT_EQ(urls[0], GURL("https://localhost/1/www"));
EXPECT_TRUE(urls[0].is_valid());
// Non-overridden items should fall back to defaults
diff --git a/chrome/updater/test/integration_tests.cc b/chrome/updater/test/integration_tests.cc
index 76ac2fb6..42d73da 100644
--- a/chrome/updater/test/integration_tests.cc
+++ b/chrome/updater/test/integration_tests.cc
@@ -5541,8 +5541,8 @@
ASSERT_TRUE(WaitForUpdaterExit());
// Redirect network traffic to remote hosts to engage the proxy.
- const GURL update_check_url = GURL("http://update.server.not_exist/update");
- const GURL dm_server_url = GURL("http://dm.server.not_exist/dmapi");
+ const GURL update_check_url = GURL("http://not_exist/update");
+ const GURL dm_server_url = GURL("http://not_exist/dmapi");
EnterTestMode(update_check_url, test_server_->crash_upload_url(),
dm_server_url, {}, base::Minutes(5));
ExpectDeviceManagementPolicyFetchRequest(test_server_.get(), kDMToken,
@@ -5583,8 +5583,8 @@
// Redirect network traffic to remote hosts to engage the proxy.
// Note the test server won't receive additional PAC script download requests
// because Windows caches it.
- const GURL update_check_url = GURL("http://update.server.not_exist/update");
- const GURL dm_server_url = GURL("http://dm.server.not_exist2/dmapi");
+ const GURL update_check_url = GURL("http://not_exist/update");
+ const GURL dm_server_url = GURL("http://not_exist/dmapi");
EnterTestMode(update_check_url, test_server_->crash_upload_url(),
dm_server_url, {}, base::Minutes(5));
ExpectDeviceManagementPolicyFetchRequest(test_server_.get(), kDMToken,
