Fix for: copySystemProfile can cause arbitrary file read and write
Bug: 216113220
Fixes: 216113220
Test: atest installd_dexopt_test
Change-Id: Iff44fb23c351ceacf4ce5d8a2394495e7afc84c8
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index ee3a67e..a2de471 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
/**
@@ -2974,7 +2995,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();