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

Commit f53c08be 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

(cherry picked from commit c597b6dd)

Change-Id: Ic5f15d1864c6af9f4c4b07dc27244ebbb521ad5e
parent 9d761562

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) {