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

Commit 1b9d9a60 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Enable clang-tidy for sensitive domain. 

Since installd has broad access to lots of sensitive data, enable
as many security-related tidy checks as possible to help avoid bugs.

This change provides a default implementation of create_cache_path(),
calculate_odex_file_path(), and calculate_oat_file_path(), along
with tests to verify behavior against old code.

Replace "dir_rec_t" with std::string, since that's really what it's
been all along.  Increase paranoia of path checking to reject any
paths containing "..", regardless of where it occurs in path string.
Stricter checking of instruction set values.

Remove now-unused char* manipulation utility methods; people should
be using std::string instead.

Test: adb shell /data/nativetest/installd_cache_test/installd_cache_test
Test: adb shell /data/nativetest/installd_service_test/installd_service_test
Test: adb shell /data/nativetest/installd_utils_test/installd_utils_test
Bug: 36655947
Change-Id: Ib706f0b8c1878be64710c00f56dccdfbe215570f
parent a91355be

cmds/installd/Android.bp

+12 −0
Original line number Diff line number Diff line
@@ -4,6 +4,7 @@ cc_defaults { 
    cflags: [

        "-Wall",

        "-Werror",

        "-Wextra",

    ],

    srcs: [

        "CacheItem.cpp",
@@ -25,6 +26,17 @@ cc_defaults { 
    ],



    clang: true,



    tidy: true,

    tidy_checks: [

        "-*",

        "clang-analyzer-security*",

        "cert-*",

        "-cert-err58-cpp",

    ],

    tidy_flags: [

        "-warnings-as-errors=clang-analyzer-security*,cert-*"

    ],

}



//

cmds/installd/InstalldNativeService.cpp

+1 −1
Original line number Diff line number Diff line
@@ -1863,7 +1863,7 @@ binder::Status InstalldNativeService::markBootComplete(const std::string& instru 
    char boot_marker_path[PKG_PATH_MAX];

    sprintf(boot_marker_path,

          "%s/%s/%s/.booting",

          android_data_dir.path,

          android_data_dir.c_str(),

          DALVIK_CACHE,

          instruction_set);

cmds/installd/dexopt.cpp

+101 −19
Original line number Diff line number Diff line
@@ -41,6 +41,7 @@ 
#include <system/thread_defs.h>



#include "dexopt.h"

#include "globals.h"

#include "installd_deps.h"

#include "otapreopt_utils.h"

#include "utils.h"
@@ -156,7 +157,7 @@ static int split_count(const char *str) 
  int count = 0;

  char buf[kPropertyValueMax];



  strncpy(buf, str, sizeof(buf));

  strlcpy(buf, str, sizeof(buf));

  char *pBuf = buf;



  while(strtok_r(pBuf, " ", &ctx) != NULL) {
@@ -333,7 +334,8 @@ static void run_dex2oat(int zip_fd, int oat_fd, int input_vdex_fd, int output_vd 


    bool have_dex2oat_compiler_filter_flag = false;

    if (skip_compilation) {

        strcpy(dex2oat_compiler_filter_arg, "--compiler-filter=extract");

        strlcpy(dex2oat_compiler_filter_arg, "--compiler-filter=extract",

                sizeof(dex2oat_compiler_filter_arg));

        have_dex2oat_compiler_filter_flag = true;

        have_dex2oat_relocation_skip_flag = true;

    } else if (compiler_filter != nullptr) {
@@ -955,14 +957,6 @@ static std::string create_vdex_filename(const std::string& oat_path) { 
    return replace_file_extension(oat_path, ".vdex");

}



static bool add_extension_to_file_name(char* file_name, const char* extension) {

    if (strlen(file_name) + strlen(extension) + 1 > PKG_PATH_MAX) {

        return false;

    }

    strcat(file_name, extension);

    return true;

}



static int open_output_file(const char* file_name, bool recreate, int permissions) {

    int flags = O_RDWR | O_CREAT;

    if (recreate) {
@@ -1198,21 +1192,16 @@ unique_fd maybe_open_dexopt_swap_file(const char* out_oat_path) { 
    if (!ShouldUseSwapFileForDexopt()) {

        return invalid_unique_fd();

    }

    // Make sure there really is enough space.

    char swap_file_name[PKG_PATH_MAX];

    strcpy(swap_file_name, out_oat_path);

    if (!add_extension_to_file_name(swap_file_name, ".swap")) {

        return invalid_unique_fd();

    }

    auto swap_file_name = std::string(out_oat_path) + ".swap";

    unique_fd swap_fd(open_output_file(

            swap_file_name, /*recreate*/true, /*permissions*/0600));

            swap_file_name.c_str(), /*recreate*/true, /*permissions*/0600));

    if (swap_fd.get() < 0) {

        // Could not create swap file. Optimistically go on and hope that we can compile

        // without it.

        ALOGE("installd could not create '%s' for swap during dexopt\n", swap_file_name);

        ALOGE("installd could not create '%s' for swap during dexopt\n", swap_file_name.c_str());

    } else {

        // Immediately unlink. We don't really want to hit flash.

        if (unlink(swap_file_name) < 0) {

        if (unlink(swap_file_name.c_str()) < 0) {

            PLOG(ERROR) << "Couldn't unlink swap file " << swap_file_name;

        }

    }
@@ -2040,5 +2029,98 @@ bool delete_odex(const char* apk_path, const char* instruction_set, const char* 
    return return_value_oat && return_value_art && return_value_vdex;

}



static bool is_absolute_path(const std::string& path) {

    if (path.find('/') != 0 || path.find("..") != std::string::npos) {

        LOG(ERROR) << "Invalid absolute path " << path;

        return false;

    } else {

        return true;

    }

}



static bool is_valid_instruction_set(const std::string& instruction_set) {

    // TODO: add explicit whitelisting of instruction sets

    if (instruction_set.find('/') != std::string::npos) {

        LOG(ERROR) << "Invalid instruction set " << instruction_set;

        return false;

    } else {

        return true;

    }

}



bool calculate_oat_file_path_default(char path[PKG_PATH_MAX], const char *oat_dir,

        const char *apk_path, const char *instruction_set) {

    std::string oat_dir_ = oat_dir;

    std::string apk_path_ = apk_path;

    std::string instruction_set_ = instruction_set;



    if (!is_absolute_path(oat_dir_)) return false;

    if (!is_absolute_path(apk_path_)) return false;

    if (!is_valid_instruction_set(instruction_set_)) return false;



    std::string::size_type end = apk_path_.rfind('.');

    std::string::size_type start = apk_path_.rfind('/', end);

    if (end == std::string::npos || start == std::string::npos) {

        LOG(ERROR) << "Invalid apk_path " << apk_path_;

        return false;

    }



    std::string res_ = oat_dir_ + '/' + instruction_set + '/'

            + apk_path_.substr(start + 1, end - start - 1) + ".odex";

    const char* res = res_.c_str();

    if (strlen(res) >= PKG_PATH_MAX) {

        LOG(ERROR) << "Result too large";

        return false;

    } else {

        strlcpy(path, res, PKG_PATH_MAX);

        return true;

    }

}



bool calculate_odex_file_path_default(char path[PKG_PATH_MAX], const char *apk_path,

        const char *instruction_set) {

    std::string apk_path_ = apk_path;

    std::string instruction_set_ = instruction_set;



    if (!is_absolute_path(apk_path_)) return false;

    if (!is_valid_instruction_set(instruction_set_)) return false;



    std::string::size_type end = apk_path_.rfind('.');

    std::string::size_type start = apk_path_.rfind('/', end);

    if (end == std::string::npos || start == std::string::npos) {

        LOG(ERROR) << "Invalid apk_path " << apk_path_;

        return false;

    }



    std::string oat_dir = apk_path_.substr(0, start + 1) + "oat";

    return calculate_oat_file_path_default(path, oat_dir.c_str(), apk_path, instruction_set);

}



bool create_cache_path_default(char path[PKG_PATH_MAX], const char *src,

        const char *instruction_set) {

    std::string src_ = src;

    std::string instruction_set_ = instruction_set;



    if (!is_absolute_path(src_)) return false;

    if (!is_valid_instruction_set(instruction_set_)) return false;



    for (auto it = src_.begin() + 1; it < src_.end(); ++it) {

        if (*it == '/') {

            *it = '@';

        }

    }



    std::string res_ = android_data_dir + DALVIK_CACHE + '/' + instruction_set_ + src_

            + DALVIK_CACHE_POSTFIX;

    const char* res = res_.c_str();

    if (strlen(res) >= PKG_PATH_MAX) {

        LOG(ERROR) << "Result too large";

        return false;

    } else {

        strlcpy(path, res, PKG_PATH_MAX);

        return true;

    }

}



}  // namespace installd

}  // namespace android

cmds/installd/dexopt.h

+11 −0
Original line number Diff line number Diff line
@@ -17,6 +17,8 @@ 
#ifndef DEXOPT_H_

#define DEXOPT_H_



#include "installd_constants.h"



#include <sys/types.h>



#include <cutils/multiuser.h>
@@ -66,6 +68,15 @@ int dexopt(const char *apk_path, uid_t uid, const char *pkgName, const char *ins 
        const char* volume_uuid, const char* class_loader_context, const char* se_info,

        bool downgrade);



bool calculate_oat_file_path_default(char path[PKG_PATH_MAX], const char *oat_dir,

        const char *apk_path, const char *instruction_set);



bool calculate_odex_file_path_default(char path[PKG_PATH_MAX], const char *apk_path,

        const char *instruction_set);



bool create_cache_path_default(char path[PKG_PATH_MAX], const char *src,

        const char *instruction_set);



}  // namespace installd

}  // namespace android

cmds/installd/globals.cpp

+53 −81
Original line number Diff line number Diff line
@@ -16,15 +16,15 @@ 


#define LOG_TAG "installd"



#include <stdlib.h>

#include <string.h>



#include <log/log.h>              // TODO: Move everything to base::logging.



#include <globals.h>

#include <installd_constants.h>

#include <utils.h>



#include <android-base/logging.h>



#include <stdlib.h>

#include <string.h>



namespace android {

namespace installd {
@@ -44,106 +44,78 @@ static constexpr const char* PROFILES_SUBDIR = "misc/profiles"; // sub-directory 
static constexpr const char* PRIVATE_APP_SUBDIR = "app-private/"; // sub-directory under

                                                                  // ANDROID_DATA



/* Directory records that are used in execution of commands. */

dir_rec_t android_app_dir;

dir_rec_t android_app_ephemeral_dir;

dir_rec_t android_app_lib_dir;

dir_rec_t android_app_private_dir;

dir_rec_t android_asec_dir;

dir_rec_t android_data_dir;

dir_rec_t android_media_dir;

dir_rec_t android_mnt_expand_dir;

dir_rec_t android_profiles_dir;



dir_rec_array_t android_system_dirs;



/**

 * Initialize all the global variables that are used elsewhere. Returns 0 upon

 * success and -1 on error.

 */

void free_globals() {

    size_t i;



    for (i = 0; i < android_system_dirs.count; i++) {

        if (android_system_dirs.dirs[i].path != NULL) {

            free(android_system_dirs.dirs[i].path);

std::string android_app_dir;

std::string android_app_ephemeral_dir;

std::string android_app_lib_dir;

std::string android_app_private_dir;

std::string android_asec_dir;

std::string android_data_dir;

std::string android_media_dir;

std::string android_mnt_expand_dir;

std::string android_profiles_dir;

std::string android_root_dir;



std::vector<std::string> android_system_dirs;



bool init_globals_from_data_and_root() {

    const char* data_path = getenv("ANDROID_DATA");

    if (data_path == nullptr) {

        LOG(ERROR) << "Could not find ANDROID_DATA";

        return false;

    }

    const char* root_path = getenv("ANDROID_ROOT");

    if (root_path == nullptr) {

        LOG(ERROR) << "Could not find ANDROID_ROOT";

        return false;

    }

    return init_globals_from_data_and_root(data_path, root_path);

}



    free(android_system_dirs.dirs);

static std::string ensure_trailing_slash(const std::string& path) {

    if (path.rfind('/') != path.size() - 1) {

        return path + '/';

    } else {

        return path;

    }

}



bool init_globals_from_data_and_root(const char* data, const char* root) {

    // Get the android data directory.

    if (get_path_from_string(&android_data_dir, data) < 0) {

        return false;

    }

    android_data_dir = ensure_trailing_slash(data);



    // Get the android root directory.

    android_root_dir = ensure_trailing_slash(root);



    // Get the android app directory.

    if (copy_and_append(&android_app_dir, &android_data_dir, APP_SUBDIR) < 0) {

        return false;

    }

    android_app_dir = android_data_dir + APP_SUBDIR;



    // Get the android protected app directory.

    if (copy_and_append(&android_app_private_dir, &android_data_dir, PRIVATE_APP_SUBDIR) < 0) {

        return false;

    }

    android_app_private_dir = android_data_dir + PRIVATE_APP_SUBDIR;



    // Get the android ephemeral app directory.

    if (copy_and_append(&android_app_ephemeral_dir, &android_data_dir, EPHEMERAL_APP_SUBDIR) < 0) {

        return false;

    }

    android_app_ephemeral_dir = android_data_dir + EPHEMERAL_APP_SUBDIR;



    // Get the android app native library directory.

    if (copy_and_append(&android_app_lib_dir, &android_data_dir, APP_LIB_SUBDIR) < 0) {

        return false;

    }

    android_app_lib_dir = android_data_dir + APP_LIB_SUBDIR;



    // Get the sd-card ASEC mount point.

    if (get_path_from_env(&android_asec_dir, ASEC_MOUNTPOINT_ENV_NAME) < 0) {

        return false;

    }

    android_asec_dir = ensure_trailing_slash(getenv(ASEC_MOUNTPOINT_ENV_NAME));



    // Get the android media directory.

    if (copy_and_append(&android_media_dir, &android_data_dir, MEDIA_SUBDIR) < 0) {

        return false;

    }

    android_media_dir = android_data_dir + MEDIA_SUBDIR;



    // Get the android external app directory.

    if (get_path_from_string(&android_mnt_expand_dir, "/mnt/expand/") < 0) {

        return false;

    }

    android_mnt_expand_dir = "/mnt/expand/";



    // Get the android profiles directory.

    if (copy_and_append(&android_profiles_dir, &android_data_dir, PROFILES_SUBDIR) < 0) {

        return false;

    }

    android_profiles_dir = android_data_dir + PROFILES_SUBDIR;



    // Take note of the system and vendor directories.

    android_system_dirs.count = 4;



    android_system_dirs.dirs = (dir_rec_t*) calloc(android_system_dirs.count, sizeof(dir_rec_t));

    if (android_system_dirs.dirs == NULL) {

        ALOGE("Couldn't allocate array for dirs; aborting\n");

        return false;

    }



    dir_rec_t android_root_dir;

    if (get_path_from_string(&android_root_dir, root) < 0) {

        return false;

    }



    android_system_dirs.dirs[0].path = build_string2(android_root_dir.path, APP_SUBDIR);

    android_system_dirs.dirs[0].len = strlen(android_system_dirs.dirs[0].path);



    android_system_dirs.dirs[1].path = build_string2(android_root_dir.path, PRIV_APP_SUBDIR);

    android_system_dirs.dirs[1].len = strlen(android_system_dirs.dirs[1].path);



    android_system_dirs.dirs[2].path = strdup("/vendor/app/");

    android_system_dirs.dirs[2].len = strlen(android_system_dirs.dirs[2].path);



    android_system_dirs.dirs[3].path = strdup("/oem/app/");

    android_system_dirs.dirs[3].len = strlen(android_system_dirs.dirs[3].path);

    android_system_dirs.clear();

    android_system_dirs.push_back(android_root_dir + APP_SUBDIR);

    android_system_dirs.push_back(android_root_dir + PRIV_APP_SUBDIR);

    android_system_dirs.push_back("/vendor/app/");

    android_system_dirs.push_back("/oem/app/");



    return true;

}