Fix use-after-free in unittest setup code.
When setting the root prefix in a unittest, we need to reset it after
so it isn't accidentally used by other tests.
Bug: chromium:547127
Test: USE="clang asan" FEATURES=test emerge-link update_engine
Change-Id: Ib84bbf265a380976407100a4fba4d2600d62dde6
diff --git a/image_properties.h b/image_properties.h
index 8f8c0f6..6026c2e 100644
--- a/image_properties.h
+++ b/image_properties.h
@@ -74,7 +74,7 @@
const MutableImageProperties& properties);
// Sets the root_prefix used to load files from during unittests to
-// |test_root_prefix|.
+// |test_root_prefix|. Passing a nullptr value resets it to the default.
namespace test {
void SetImagePropertiesRootPrefix(const char* test_root_prefix);
} // namespace test
diff --git a/image_properties_chromeos.cc b/image_properties_chromeos.cc
index 7f38848..8999d6a 100644
--- a/image_properties_chromeos.cc
+++ b/image_properties_chromeos.cc
@@ -44,7 +44,7 @@
const char kDefaultAppId[] = "{87efface-864d-49a5-9bb3-4b050a7c227a}";
// A prefix added to the path, used for testing.
-const char* root_prefix = "";
+const char* root_prefix = nullptr;
std::string GetStringWithDefault(const brillo::KeyValueStore& store,
const std::string& key,
@@ -67,7 +67,9 @@
// |source|. The loaded values are added to the store, possibly overriding
// existing values.
void LoadLsbRelease(LsbReleaseSource source, brillo::KeyValueStore* store) {
- std::string path = root_prefix;
+ std::string path;
+ if (root_prefix)
+ path = root_prefix;
if (source == LsbReleaseSource::kStateful)
path += chromeos_update_engine::kStatefulPartition;
store->Load(base::FilePath(path + kLsbRelease));
diff --git a/omaha_request_params.cc b/omaha_request_params.cc
index 9759935..f6fad1c 100644
--- a/omaha_request_params.cc
+++ b/omaha_request_params.cc
@@ -53,6 +53,11 @@
"stable-channel",
};
+OmahaRequestParams::~OmahaRequestParams() {
+ if (!root_.empty())
+ test::SetImagePropertiesRootPrefix(nullptr);
+}
+
bool OmahaRequestParams::Init(const string& in_app_version,
const string& in_update_url,
bool in_interactive) {
diff --git a/omaha_request_params.h b/omaha_request_params.h
index 131a8f3..b6b6f04 100644
--- a/omaha_request_params.h
+++ b/omaha_request_params.h
@@ -95,7 +95,7 @@
mutable_image_props_.is_powerwash_allowed = false;
}
- virtual ~OmahaRequestParams() = default;
+ virtual ~OmahaRequestParams();
// Setters and getters for the various properties.
inline std::string os_platform() const { return os_platform_; }
diff --git a/omaha_request_params_unittest.cc b/omaha_request_params_unittest.cc
index 9ee2a66..473b101 100644
--- a/omaha_request_params_unittest.cc
+++ b/omaha_request_params_unittest.cc
@@ -357,6 +357,7 @@
EXPECT_FALSE(params.is_powerwash_allowed());
}
OmahaRequestParams out(&fake_system_state_);
+ out.set_root(test_dir_);
EXPECT_TRUE(DoTest(&out, "", ""));
EXPECT_EQ("canary-channel", out.target_channel());
EXPECT_FALSE(out.is_powerwash_allowed());
@@ -378,6 +379,7 @@
EXPECT_TRUE(params.is_powerwash_allowed());
}
OmahaRequestParams out(&fake_system_state_);
+ out.set_root(test_dir_);
EXPECT_TRUE(DoTest(&out, "", ""));
EXPECT_EQ("canary-channel", out.target_channel());
EXPECT_TRUE(out.is_powerwash_allowed());
@@ -400,6 +402,7 @@
EXPECT_FALSE(params.is_powerwash_allowed());
}
OmahaRequestParams out(&fake_system_state_);
+ out.set_root(test_dir_);
EXPECT_TRUE(DoTest(&out, "", ""));
EXPECT_EQ("arm-generic", out.os_board());
EXPECT_EQ("dev-channel", out.target_channel());