Merge changes Ia3eae81c,Iff44fb23
* changes:
Fix for: clearAppProfiles interface can cause arbitrary file truncate
Fix for: copySystemProfile can cause arbitrary file read and write
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index ee3a67e..ae1d3aa 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -230,6 +230,19 @@
}
}
+binder::Status checkArgumentFileName(const std::string& path) {
+ if (path.empty()) {
+ return exception(binder::Status::EX_ILLEGAL_ARGUMENT, "Missing name");
+ }
+ for (const char& c : path) {
+ if (c == '\0' || c == '\n' || c == '/') {
+ return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
+ StringPrintf("Name %s is malformed", path.c_str()));
+ }
+ }
+ return ok();
+}
+
#define ENFORCE_UID(uid) { \
binder::Status status = checkUid((uid)); \
if (!status.isOk()) { \
@@ -266,6 +279,14 @@
} \
}
+#define CHECK_ARGUMENT_FILE_NAME(path) \
+ { \
+ binder::Status status = checkArgumentFileName((path)); \
+ if (!status.isOk()) { \
+ return status; \
+ } \
+ }
+
#ifdef GRANULAR_LOCKS
/**
@@ -1007,6 +1028,12 @@
const std::string& profileName) {
ENFORCE_UID(AID_SYSTEM);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
+ CHECK_ARGUMENT_FILE_NAME(profileName);
+ if (!base::EndsWith(profileName, ".prof")) {
+ return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
+ StringPrintf("Profile name %s does not end with .prof",
+ profileName.c_str()));
+ }
LOCK_PACKAGE();
binder::Status res = ok();
@@ -2974,7 +3001,19 @@
int32_t packageUid, const std::string& packageName, const std::string& profileName,
bool* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
+ CHECK_ARGUMENT_PATH(systemProfile);
+ if (!base::EndsWith(systemProfile, ".prof")) {
+ return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
+ StringPrintf("System profile path %s does not end with .prof",
+ systemProfile.c_str()));
+ }
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
+ CHECK_ARGUMENT_FILE_NAME(profileName);
+ if (!base::EndsWith(profileName, ".prof")) {
+ return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
+ StringPrintf("Profile name %s does not end with .prof",
+ profileName.c_str()));
+ }
LOCK_PACKAGE();
*_aidl_return = copy_system_profile(systemProfile, packageUid, packageName, profileName);
return ok();
diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp
index 800e141..a1c5fae 100644
--- a/cmds/installd/tests/installd_dexopt_test.cpp
+++ b/cmds/installd/tests/installd_dexopt_test.cpp
@@ -1293,6 +1293,58 @@
preparePackageProfile(package_name_, "primary.prof", /*expected_result*/ false);
}
+TEST_F(ProfileTest, ClearAppProfilesOk) {
+ LOG(INFO) << "ClearAppProfilesOk";
+
+ ASSERT_BINDER_SUCCESS(service_->clearAppProfiles(package_name_, "primary.prof"));
+ ASSERT_BINDER_SUCCESS(service_->clearAppProfiles(package_name_, "image_editor.split.prof"));
+}
+
+TEST_F(ProfileTest, ClearAppProfilesFailWrongProfileName) {
+ LOG(INFO) << "ClearAppProfilesFailWrongProfileName";
+
+ ASSERT_BINDER_FAIL(
+ service_->clearAppProfiles(package_name_,
+ "../../../../dalvik-cache/arm64/"
+ "system@app@SecureElement@SecureElement.apk@classes.vdex"));
+ ASSERT_BINDER_FAIL(service_->clearAppProfiles(package_name_, "image_editor.split.apk"));
+}
+
+TEST_F(ProfileTest, CopySystemProfileOk) {
+ LOG(INFO) << "CopySystemProfileOk";
+
+ bool result;
+ ASSERT_BINDER_SUCCESS(
+ service_->copySystemProfile("/data/app/random.string/package.name.random/base.apk.prof",
+ kTestAppUid, package_name_, "primary.prof", &result));
+}
+
+TEST_F(ProfileTest, CopySystemProfileFailWrongSystemProfilePath) {
+ LOG(INFO) << "CopySystemProfileFailWrongSystemProfilePath";
+
+ bool result;
+ ASSERT_BINDER_FAIL(service_->copySystemProfile("../../secret.dat", kTestAppUid, package_name_,
+ "primary.prof", &result));
+ ASSERT_BINDER_FAIL(service_->copySystemProfile("/data/user/package.name/secret.data",
+ kTestAppUid, package_name_, "primary.prof",
+ &result));
+}
+
+TEST_F(ProfileTest, CopySystemProfileFailWrongProfileName) {
+ LOG(INFO) << "CopySystemProfileFailWrongProfileName";
+
+ bool result;
+ ASSERT_BINDER_FAIL(
+ service_->copySystemProfile("/data/app/random.string/package.name.random/base.apk.prof",
+ kTestAppUid, package_name_,
+ "../../../../dalvik-cache/arm64/test.vdex", &result));
+ ASSERT_BINDER_FAIL(
+ service_->copySystemProfile("/data/app/random.string/package.name.random/base.apk.prof",
+ kTestAppUid, package_name_, "/test.prof", &result));
+ ASSERT_BINDER_FAIL(
+ service_->copySystemProfile("/data/app/random.string/package.name.random/base.apk.prof",
+ kTestAppUid, package_name_, "base.apk", &result));
+}
class BootProfileTest : public ProfileTest {
public: