Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c597b6dd authored by Calin Juravle's avatar Calin Juravle
Browse files

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
parent 67cfe0c0

cmds/installd/tests/installd_utils_test.cpp

+34 −0
Original line number Diff line number Diff line
@@ -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))

cmds/installd/utils.c

+33 −27
Original line number Diff line number Diff line
@@ -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) {