Merge changes Ia3eae81c,Iff44fb23

* changes:
  Fix for: clearAppProfiles interface can cause arbitrary file truncate
  Fix for: copySystemProfile can cause arbitrary file read and write

2 files changed, 91 insertions, 0 deletions

diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index ee3a67e5fd..ae1d3aa63b 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -230,6 +230,19 @@ binder::Status checkArgumentPath(const std::optional<std::string>& path) {
     }
 }
 
+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 @@ binder::Status checkArgumentPath(const std::optional<std::string>& path) {
     }                                                       \
 }
 
+#define CHECK_ARGUMENT_FILE_NAME(path)                         \
+    {                                                          \
+        binder::Status status = checkArgumentFileName((path)); \
+        if (!status.isOk()) {                                  \
+            return status;                                     \
+        }                                                      \
+    }
+
 #ifdef GRANULAR_LOCKS
 
 /**
@@ -1007,6 +1028,12 @@ binder::Status InstalldNativeService::clearAppProfiles(const std::string& packag
         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 @@ binder::Status InstalldNativeService::copySystemProfile(const std::string& syste
         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 800e141d92..a1c5fae8f6 100644
--- a/cmds/installd/tests/installd_dexopt_test.cpp
+++ b/cmds/installd/tests/installd_dexopt_test.cpp
@@ -1293,6 +1293,58 @@ TEST_F(ProfileTest, ProfilePrepareFailProfileChangedUid) {
     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: