Fix validation of system paths in installd.
System apps are now installed under their own directory
(system_app_dir/app_dir/app.apk). The new path doesn't pass installd
validation because of obsolete checks which verify that the path does
not contain subdirectories past the system_app_dir.
The CL fixes the validation to accept at most on subdirectory.
Bug: 17109858
Change-Id: I13abb52c0016610ff436f6a26bb6b3b85dc4dfb0
diff --git a/cmds/installd/tests/installd_utils_test.cpp b/cmds/installd/tests/installd_utils_test.cpp
index eeafa10..94e4792 100644
--- a/cmds/installd/tests/installd_utils_test.cpp
+++ b/cmds/installd/tests/installd_utils_test.cpp
@@ -118,6 +118,14 @@
const char *bad_path3 = TEST_APP_DIR "example.com/subdir/pkg.apk";
EXPECT_EQ(-1, validate_apk_path(bad_path3))
<< bad_path3 << " should be rejected as a invalid path";
+
+ const char *bad_path4 = TEST_APP_DIR "example.com/subdir/../pkg.apk";
+ EXPECT_EQ(-1, validate_apk_path(bad_path4))
+ << bad_path4 << " should be rejected as a invalid path";
+
+ const char *bad_path5 = TEST_APP_DIR "example.com1/../example.com2/pkg.apk";
+ EXPECT_EQ(-1, validate_apk_path(bad_path5))
+ << bad_path5 << " should be rejected as a invalid path";
}
TEST_F(UtilsTest, IsValidApkPath_Private) {
@@ -143,6 +151,14 @@
const char *bad_path3 = TEST_APP_PRIVATE_DIR "example.com/subdir/pkg.apk";
EXPECT_EQ(-1, validate_apk_path(bad_path3))
<< bad_path3 << " should be rejected as a invalid path";
+
+ const char *bad_path4 = TEST_APP_PRIVATE_DIR "example.com/subdir/../pkg.apk";
+ EXPECT_EQ(-1, validate_apk_path(bad_path4))
+ << bad_path4 << " should be rejected as a invalid path";
+
+ const char *bad_path5 = TEST_APP_PRIVATE_DIR "example.com1/../example.com2/pkg.apk";
+ EXPECT_EQ(-1, validate_apk_path(bad_path5))
+ << bad_path5 << " should be rejected as a invalid path";
}
@@ -230,6 +246,24 @@
<< badapp3 << " should be rejected not a system path";
}
+TEST_F(UtilsTest, CheckSystemApp_Subdir) {
+ const char *sysapp = TEST_SYSTEM_DIR1 "com.example/com.example.apk";
+ EXPECT_EQ(0, validate_system_app_path(sysapp))
+ << sysapp << " should be allowed as a system path";
+
+ const char *badapp = TEST_SYSTEM_DIR1 "com.example/subdir/com.example.apk";
+ EXPECT_EQ(-1, validate_system_app_path(badapp))
+ << badapp << " should be rejected not a system path";
+
+ const char *badapp1 = TEST_SYSTEM_DIR1 "com.example/subdir/../com.example.apk";
+ EXPECT_EQ(-1, validate_system_app_path(badapp1))
+ << badapp1 << " should be rejected not a system path";
+
+ const char *badapp2 = TEST_SYSTEM_DIR1 "com.example1/../com.example2/com.example.apk";
+ EXPECT_EQ(-1, validate_system_app_path(badapp2))
+ << badapp2 << " should be rejected not a system path";
+}
+
TEST_F(UtilsTest, GetPathFromString_NullPathFail) {
dir_rec_t test1;
EXPECT_EQ(-1, get_path_from_string(&test1, (const char *) NULL))
diff --git a/cmds/installd/utils.c b/cmds/installd/utils.c
index def2fc7..8cd1168 100644
--- a/cmds/installd/utils.c
+++ b/cmds/installd/utils.c
@@ -808,6 +808,33 @@
}
/**
+ * Validate that the path is valid in the context of the provided directory.
+ * The path is allowed to have at most one subdirectory and no indirections
+ * to top level directories (i.e. have "..").
+ */
+static int validate_path(const dir_rec_t* dir, const char* path) {
+ size_t dir_len = dir->len;
+ const char* subdir = strchr(path + dir_len, '/');
+
+ // Only allow the path to have at most one subdirectory.
+ if (subdir != NULL) {
+ ++subdir;
+ if (strchr(subdir, '/') != NULL) {
+ ALOGE("invalid apk path '%s' (subdir?)\n", path);
+ return -1;
+ }
+ }
+
+ // Directories can't have a period directly after the directory markers to prevent "..".
+ if ((path[dir_len] == '.') || ((subdir != NULL) && (*subdir == '.'))) {
+ ALOGE("invalid apk path '%s' (trickery)\n", path);
+ return -1;
+ }
+
+ return 0;
+}
+
+/**
* Checks whether a path points to a system app (.apk file). Returns 0
* if it is a system app or -1 if it is not.
*/
@@ -817,11 +844,7 @@
for (i = 0; i < android_system_dirs.count; i++) {
const size_t dir_len = android_system_dirs.dirs[i].len;
if (!strncmp(path, android_system_dirs.dirs[i].path, dir_len)) {
- if (path[dir_len] == '.' || strchr(path + dir_len, '/') != NULL) {
- ALOGE("invalid system apk path '%s' (trickery)\n", path);
- return -1;
- }
- return 0;
+ return validate_path(android_system_dirs.dirs + i, path);
}
}
@@ -920,37 +943,20 @@
*/
int validate_apk_path(const char *path)
{
- size_t dir_len;
+ const dir_rec_t* dir = NULL;
if (!strncmp(path, android_app_dir.path, android_app_dir.len)) {
- dir_len = android_app_dir.len;
+ dir = &android_app_dir;
} else if (!strncmp(path, android_app_private_dir.path, android_app_private_dir.len)) {
- dir_len = android_app_private_dir.len;
+ dir = &android_app_private_dir;
} else if (!strncmp(path, android_asec_dir.path, android_asec_dir.len)) {
- dir_len = android_asec_dir.len;
+ dir = &android_asec_dir;
} else {
ALOGE("invalid apk path '%s' (bad prefix)\n", path);
return -1;
}
- const char* subdir = strchr(path + dir_len, '/');
-
- // Only allow the path to have at most one subdirectory.
- if (subdir != NULL) {
- ++subdir;
- if (strchr(subdir, '/') != NULL) {
- ALOGE("invalid apk path '%s' (subdir?)\n", path);
- return -1;
- }
- }
-
- // Directories can't have a period directly after the directory markers to prevent "..".
- if ((path[dir_len] == '.') || ((subdir != NULL) && (*subdir == '.'))) {
- ALOGE("invalid apk path '%s' (trickery)\n", path);
- return -1;
- }
-
- return 0;
+ return validate_path(dir, path);
}
int append_and_increment(char** dst, const char* src, size_t* dst_size) {