Loading cmds/installd/tests/installd_utils_test.cpp +18 −6 Original line number Original line Diff line number Diff line Loading @@ -101,6 +101,11 @@ TEST_F(UtilsTest, IsValidApkPath_Internal) { EXPECT_EQ(0, validate_apk_path(internal1)) EXPECT_EQ(0, validate_apk_path(internal1)) << internal1 << " should be allowed as a valid path"; << internal1 << " should be allowed as a valid path"; // b/16888084 const char *path2 = TEST_APP_DIR "example.com/example.apk"; EXPECT_EQ(0, validate_apk_path(path2)) << path2 << " should be allowed as a valid path"; const char *badint1 = TEST_APP_DIR "../example.apk"; const char *badint1 = TEST_APP_DIR "../example.apk"; EXPECT_EQ(-1, validate_apk_path(badint1)) EXPECT_EQ(-1, validate_apk_path(badint1)) << badint1 << " should be rejected as a invalid path"; << badint1 << " should be rejected as a invalid path"; Loading @@ -109,9 +114,10 @@ TEST_F(UtilsTest, IsValidApkPath_Internal) { EXPECT_EQ(-1, validate_apk_path(badint2)) EXPECT_EQ(-1, validate_apk_path(badint2)) << badint2 << " should be rejected as a invalid path"; << badint2 << " should be rejected as a invalid path"; const char *badint3 = TEST_APP_DIR "example.com/pkg.apk"; // Only one subdir should be allowed. EXPECT_EQ(-1, validate_apk_path(badint3)) const char *bad_path3 = TEST_APP_DIR "example.com/subdir/pkg.apk"; << badint3 << " should be rejected as a invalid path"; EXPECT_EQ(-1, validate_apk_path(bad_path3)) << bad_path3 << " should be rejected as a invalid path"; } } TEST_F(UtilsTest, IsValidApkPath_Private) { TEST_F(UtilsTest, IsValidApkPath_Private) { Loading @@ -120,6 +126,11 @@ TEST_F(UtilsTest, IsValidApkPath_Private) { EXPECT_EQ(0, validate_apk_path(private1)) EXPECT_EQ(0, validate_apk_path(private1)) << private1 << " should be allowed as a valid path"; << private1 << " should be allowed as a valid path"; // b/16888084 const char *path2 = TEST_APP_DIR "example.com/example.apk"; EXPECT_EQ(0, validate_apk_path(path2)) << path2 << " should be allowed as a valid path"; const char *badpriv1 = TEST_APP_PRIVATE_DIR "../example.apk"; const char *badpriv1 = TEST_APP_PRIVATE_DIR "../example.apk"; EXPECT_EQ(-1, validate_apk_path(badpriv1)) EXPECT_EQ(-1, validate_apk_path(badpriv1)) << badpriv1 << " should be rejected as a invalid path"; << badpriv1 << " should be rejected as a invalid path"; Loading @@ -128,9 +139,10 @@ TEST_F(UtilsTest, IsValidApkPath_Private) { EXPECT_EQ(-1, validate_apk_path(badpriv2)) EXPECT_EQ(-1, validate_apk_path(badpriv2)) << badpriv2 << " should be rejected as a invalid path"; << badpriv2 << " should be rejected as a invalid path"; const char *badpriv3 = TEST_APP_PRIVATE_DIR "example.com/pkg.apk"; // Only one subdir should be allowed. EXPECT_EQ(-1, validate_apk_path(badpriv3)) const char *bad_path3 = TEST_APP_PRIVATE_DIR "example.com/subdir/pkg.apk"; << badpriv3 << " should be rejected as a invalid path"; EXPECT_EQ(-1, validate_apk_path(bad_path3)) << bad_path3 << " should be rejected as a invalid path"; } } Loading cmds/installd/utils.c +9 −20 Original line number Original line Diff line number Diff line Loading @@ -914,16 +914,13 @@ int copy_and_append(dir_rec_t* dst, const dir_rec_t* src, const char* suffix) { } } /** /** * Check whether path points to a valid path for an APK file. An ASEC * Check whether path points to a valid path for an APK file. Only one level of * directory is allowed to have one level of subdirectory names. Returns -1 * subdirectory names is allowed. Returns -1 when an invalid path is encountered * when an invalid path is encountered and 0 when a valid path is encountered. * and 0 when a valid path is encountered. */ */ int validate_apk_path(const char *path) int validate_apk_path(const char *path) { { int allowsubdir = 0; char *subdir = NULL; size_t dir_len; size_t dir_len; size_t path_len; if (!strncmp(path, android_app_dir.path, android_app_dir.len)) { if (!strncmp(path, android_app_dir.path, android_app_dir.len)) { dir_len = android_app_dir.len; dir_len = android_app_dir.len; Loading @@ -931,32 +928,24 @@ int validate_apk_path(const char *path) dir_len = android_app_private_dir.len; dir_len = android_app_private_dir.len; } else if (!strncmp(path, android_asec_dir.path, android_asec_dir.len)) { } else if (!strncmp(path, android_asec_dir.path, android_asec_dir.len)) { dir_len = android_asec_dir.len; dir_len = android_asec_dir.len; allowsubdir = 1; } else { } else { ALOGE("invalid apk path '%s' (bad prefix)\n", path); ALOGE("invalid apk path '%s' (bad prefix)\n", path); return -1; return -1; } } path_len = strlen(path); const char* subdir = strchr(path + dir_len, '/'); /* // Only allow the path to have at most one subdirectory. * Only allow the path to have a subdirectory if it's been marked as being allowed. if (subdir != NULL) { */ if ((subdir = strchr(path + dir_len, '/')) != NULL) { ++subdir; ++subdir; if (!allowsubdir if (strchr(subdir, '/') != NULL) { || (path_len > (size_t) (subdir - path) && (strchr(subdir, '/') != NULL))) { ALOGE("invalid apk path '%s' (subdir?)\n", path); ALOGE("invalid apk path '%s' (subdir?)\n", path); return -1; return -1; } } } } /* // Directories can't have a period directly after the directory markers to prevent "..". * Directories can't have a period directly after the directory markers if ((path[dir_len] == '.') || ((subdir != NULL) && (*subdir == '.'))) { * to prevent ".." */ if (path[dir_len] == '.' || (subdir != NULL && ((*subdir == '.') || (strchr(subdir, '/') != NULL)))) { ALOGE("invalid apk path '%s' (trickery)\n", path); ALOGE("invalid apk path '%s' (trickery)\n", path); return -1; return -1; } } Loading Loading
cmds/installd/tests/installd_utils_test.cpp +18 −6 Original line number Original line Diff line number Diff line Loading @@ -101,6 +101,11 @@ TEST_F(UtilsTest, IsValidApkPath_Internal) { EXPECT_EQ(0, validate_apk_path(internal1)) EXPECT_EQ(0, validate_apk_path(internal1)) << internal1 << " should be allowed as a valid path"; << internal1 << " should be allowed as a valid path"; // b/16888084 const char *path2 = TEST_APP_DIR "example.com/example.apk"; EXPECT_EQ(0, validate_apk_path(path2)) << path2 << " should be allowed as a valid path"; const char *badint1 = TEST_APP_DIR "../example.apk"; const char *badint1 = TEST_APP_DIR "../example.apk"; EXPECT_EQ(-1, validate_apk_path(badint1)) EXPECT_EQ(-1, validate_apk_path(badint1)) << badint1 << " should be rejected as a invalid path"; << badint1 << " should be rejected as a invalid path"; Loading @@ -109,9 +114,10 @@ TEST_F(UtilsTest, IsValidApkPath_Internal) { EXPECT_EQ(-1, validate_apk_path(badint2)) EXPECT_EQ(-1, validate_apk_path(badint2)) << badint2 << " should be rejected as a invalid path"; << badint2 << " should be rejected as a invalid path"; const char *badint3 = TEST_APP_DIR "example.com/pkg.apk"; // Only one subdir should be allowed. EXPECT_EQ(-1, validate_apk_path(badint3)) const char *bad_path3 = TEST_APP_DIR "example.com/subdir/pkg.apk"; << badint3 << " should be rejected as a invalid path"; EXPECT_EQ(-1, validate_apk_path(bad_path3)) << bad_path3 << " should be rejected as a invalid path"; } } TEST_F(UtilsTest, IsValidApkPath_Private) { TEST_F(UtilsTest, IsValidApkPath_Private) { Loading @@ -120,6 +126,11 @@ TEST_F(UtilsTest, IsValidApkPath_Private) { EXPECT_EQ(0, validate_apk_path(private1)) EXPECT_EQ(0, validate_apk_path(private1)) << private1 << " should be allowed as a valid path"; << private1 << " should be allowed as a valid path"; // b/16888084 const char *path2 = TEST_APP_DIR "example.com/example.apk"; EXPECT_EQ(0, validate_apk_path(path2)) << path2 << " should be allowed as a valid path"; const char *badpriv1 = TEST_APP_PRIVATE_DIR "../example.apk"; const char *badpriv1 = TEST_APP_PRIVATE_DIR "../example.apk"; EXPECT_EQ(-1, validate_apk_path(badpriv1)) EXPECT_EQ(-1, validate_apk_path(badpriv1)) << badpriv1 << " should be rejected as a invalid path"; << badpriv1 << " should be rejected as a invalid path"; Loading @@ -128,9 +139,10 @@ TEST_F(UtilsTest, IsValidApkPath_Private) { EXPECT_EQ(-1, validate_apk_path(badpriv2)) EXPECT_EQ(-1, validate_apk_path(badpriv2)) << badpriv2 << " should be rejected as a invalid path"; << badpriv2 << " should be rejected as a invalid path"; const char *badpriv3 = TEST_APP_PRIVATE_DIR "example.com/pkg.apk"; // Only one subdir should be allowed. EXPECT_EQ(-1, validate_apk_path(badpriv3)) const char *bad_path3 = TEST_APP_PRIVATE_DIR "example.com/subdir/pkg.apk"; << badpriv3 << " should be rejected as a invalid path"; EXPECT_EQ(-1, validate_apk_path(bad_path3)) << bad_path3 << " should be rejected as a invalid path"; } } Loading
cmds/installd/utils.c +9 −20 Original line number Original line Diff line number Diff line Loading @@ -914,16 +914,13 @@ int copy_and_append(dir_rec_t* dst, const dir_rec_t* src, const char* suffix) { } } /** /** * Check whether path points to a valid path for an APK file. An ASEC * Check whether path points to a valid path for an APK file. Only one level of * directory is allowed to have one level of subdirectory names. Returns -1 * subdirectory names is allowed. Returns -1 when an invalid path is encountered * when an invalid path is encountered and 0 when a valid path is encountered. * and 0 when a valid path is encountered. */ */ int validate_apk_path(const char *path) int validate_apk_path(const char *path) { { int allowsubdir = 0; char *subdir = NULL; size_t dir_len; size_t dir_len; size_t path_len; if (!strncmp(path, android_app_dir.path, android_app_dir.len)) { if (!strncmp(path, android_app_dir.path, android_app_dir.len)) { dir_len = android_app_dir.len; dir_len = android_app_dir.len; Loading @@ -931,32 +928,24 @@ int validate_apk_path(const char *path) dir_len = android_app_private_dir.len; dir_len = android_app_private_dir.len; } else if (!strncmp(path, android_asec_dir.path, android_asec_dir.len)) { } else if (!strncmp(path, android_asec_dir.path, android_asec_dir.len)) { dir_len = android_asec_dir.len; dir_len = android_asec_dir.len; allowsubdir = 1; } else { } else { ALOGE("invalid apk path '%s' (bad prefix)\n", path); ALOGE("invalid apk path '%s' (bad prefix)\n", path); return -1; return -1; } } path_len = strlen(path); const char* subdir = strchr(path + dir_len, '/'); /* // Only allow the path to have at most one subdirectory. * Only allow the path to have a subdirectory if it's been marked as being allowed. if (subdir != NULL) { */ if ((subdir = strchr(path + dir_len, '/')) != NULL) { ++subdir; ++subdir; if (!allowsubdir if (strchr(subdir, '/') != NULL) { || (path_len > (size_t) (subdir - path) && (strchr(subdir, '/') != NULL))) { ALOGE("invalid apk path '%s' (subdir?)\n", path); ALOGE("invalid apk path '%s' (subdir?)\n", path); return -1; return -1; } } } } /* // Directories can't have a period directly after the directory markers to prevent "..". * Directories can't have a period directly after the directory markers if ((path[dir_len] == '.') || ((subdir != NULL) && (*subdir == '.'))) { * to prevent ".." */ if (path[dir_len] == '.' || (subdir != NULL && ((*subdir == '.') || (strchr(subdir, '/') != NULL)))) { ALOGE("invalid apk path '%s' (trickery)\n", path); ALOGE("invalid apk path '%s' (trickery)\n", path); return -1; return -1; } } Loading