ADHOC/replicant-frameworks_native
3
0
Fork 0
Code Issues 6 Pull Requests Releases Wiki Activity

Fix validation of system paths in installd.

Browse Source 
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
This commit is contained in:
Calin Juravle 2014-08-19 17:43:05 +01:00
parent 67cfe0c066
commit c597b6dd89
2 changed files with 67 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
cmds/installd/tests/installd_utils_test.cpp
View File

@ -118,6 +118,14 @@ TEST_F(UtilsTest, IsValidApkPath_Internal) {
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 @@ TEST_F(UtilsTest, IsValidApkPath_Private) {
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 @@ TEST_F(UtilsTest, CheckSystemApp_BadPathEscapeFail) {
<< 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))

60
cmds/installd/utils.c
View File

@ -807,6 +807,33 @@ void finish_cache_collection(cache_t* cache)
free(cache);
}
/**
* 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 @@ int validate_system_app_path(const char* path) {
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 copy_and_append(dir_rec_t* dst, const dir_rec_t* src, const char* suffix) {
*/
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) {