From 15938534b7a3fba2b39c3f7942834adf55d2a6a6 Mon Sep 17 00:00:00 2001 From: xuxuepeng Date: Mon, 28 Aug 2023 16:38:02 +0800 Subject: [PATCH 120/145] Fix null-ptr and buffer overflow issues Signed-off-by: xuxuepeng --- .../connect/grpc/grpc_containers_service.cc | 5 +-- .../grpc/grpc_containers_service_private.cc | 2 +- .../connect/grpc/grpc_server_tls_auth.cc | 2 +- .../entry/connect/grpc/grpc_server_tls_auth.h | 2 +- .../connect/rest/rest_containers_service.c | 2 +- src/daemon/entry/cri/cni_network_plugin.h | 6 ++-- .../cri/cri_image_manager_service_impl.cc | 2 +- .../cri/cri_image_manager_service_impl.h | 4 +-- .../entry/cri/websocket/service/ws_server.cc | 5 +++ src/daemon/modules/runtime/shim/shim_rt_ops.c | 6 ++++ .../modules/service/service_container.c | 4 +-- src/daemon/modules/spec/parse_volume.c | 5 +++ src/daemon/modules/spec/specs_extend.c | 12 +++++++ src/daemon/modules/spec/specs_mount.c | 34 ++++++++++++++++--- src/daemon/modules/spec/specs_mount.h | 2 -- src/daemon/modules/spec/specs_namespace.c | 6 ++-- src/daemon/modules/spec/specs_security.c | 11 +----- src/daemon/modules/volume/local.c | 3 +- src/daemon/modules/volume/volume.c | 2 +- 19 files changed, 81 insertions(+), 34 deletions(-) diff --git a/src/daemon/entry/connect/grpc/grpc_containers_service.cc b/src/daemon/entry/connect/grpc/grpc_containers_service.cc index f69613ce..ab762853 100644 --- a/src/daemon/entry/connect/grpc/grpc_containers_service.cc +++ b/src/daemon/entry/connect/grpc/grpc_containers_service.cc @@ -18,14 +18,15 @@ #include #include #include -#include "isula_libutils/log.h" +#include +#include + #include "utils.h" #include "error.h" #include "cxxutils.h" #include "stoppable_thread.h" #include "grpc_server_tls_auth.h" #include "container_api.h" -#include "isula_libutils/logger_json_file.h" void protobuf_timestamp_to_grpc(const types_timestamp_t *timestamp, Timestamp *gtimestamp) { diff --git a/src/daemon/entry/connect/grpc/grpc_containers_service_private.cc b/src/daemon/entry/connect/grpc/grpc_containers_service_private.cc index 853336fe..8cf9ae78 100644 --- a/src/daemon/entry/connect/grpc/grpc_containers_service_private.cc +++ b/src/daemon/entry/connect/grpc/grpc_containers_service_private.cc @@ -13,7 +13,7 @@ * Description: provide grpc container service private functions ******************************************************************************/ #include "grpc_containers_service.h" -#include "isula_libutils/log.h" +#include #include "utils.h" #include "error.h" diff --git a/src/daemon/entry/connect/grpc/grpc_server_tls_auth.cc b/src/daemon/entry/connect/grpc/grpc_server_tls_auth.cc index 737bb129..968a6dfe 100644 --- a/src/daemon/entry/connect/grpc/grpc_server_tls_auth.cc +++ b/src/daemon/entry/connect/grpc/grpc_server_tls_auth.cc @@ -24,7 +24,7 @@ std::string auth_plugin = ""; } // namespace AuthorizationPluginConfig namespace GrpcServerTlsAuth { -Status auth(ServerContext *context, std::string action) +Status auth(ServerContext *context, const std::string &action) { #ifdef ENABLE_GRPC_REMOTE_CONNECT const std::multimap &init_metadata = context->client_metadata(); diff --git a/src/daemon/entry/connect/grpc/grpc_server_tls_auth.h b/src/daemon/entry/connect/grpc/grpc_server_tls_auth.h index ee66529f..f429a02d 100644 --- a/src/daemon/entry/connect/grpc/grpc_server_tls_auth.h +++ b/src/daemon/entry/connect/grpc/grpc_server_tls_auth.h @@ -27,7 +27,7 @@ extern std::string auth_plugin; }; // namespace AuthorizationPluginConfig namespace GrpcServerTlsAuth { -Status auth(ServerContext *context, std::string action); +Status auth(ServerContext *context, const std::string &action); }; // namespace GrpcServerTlsAuth #endif // DAEMON_ENTRY_CONNECT_GRPC_GRPC_SERVER_TLS_AUTH_H diff --git a/src/daemon/entry/connect/rest/rest_containers_service.c b/src/daemon/entry/connect/rest/rest_containers_service.c index 397660e2..73eda508 100644 --- a/src/daemon/entry/connect/rest/rest_containers_service.c +++ b/src/daemon/entry/connect/rest/rest_containers_service.c @@ -15,8 +15,8 @@ #include "rest_containers_service.h" #include #include +#include -#include "isula_libutils/log.h" #include "utils.h" #include "error.h" #include "callback.h" diff --git a/src/daemon/entry/cri/cni_network_plugin.h b/src/daemon/entry/cri/cni_network_plugin.h index 434222b5..1518c5f4 100644 --- a/src/daemon/entry/cri/cni_network_plugin.h +++ b/src/daemon/entry/cri/cni_network_plugin.h @@ -32,9 +32,9 @@ namespace Network { #define UNUSED(x) ((void)(x)) -static const std::string CNI_PLUGIN_NAME { "cni" }; -static const std::string DEFAULT_NET_DIR { "/etc/cni/net.d" }; -static const std::string DEFAULT_CNI_DIR { "/opt/cni/bin" }; +const std::string CNI_PLUGIN_NAME { "cni" }; +const std::string DEFAULT_NET_DIR { "/etc/cni/net.d" }; +const std::string DEFAULT_CNI_DIR { "/opt/cni/bin" }; class CNINetwork { public: diff --git a/src/daemon/entry/cri/cri_image_manager_service_impl.cc b/src/daemon/entry/cri/cri_image_manager_service_impl.cc index 69573a70..ad9e8ef1 100644 --- a/src/daemon/entry/cri/cri_image_manager_service_impl.cc +++ b/src/daemon/entry/cri/cri_image_manager_service_impl.cc @@ -1,5 +1,5 @@ /****************************************************************************** - * Copyright (c) Huawei Technologies Co., Ltd. 2020. All rights reserved. + * Copyright (c) Huawei Technologies Co., Ltd. 2017-2020. All rights reserved. * iSulad licensed under the Mulan PSL v2. * You can use this software according to the terms and conditions of the Mulan PSL v2. * You may obtain a copy of Mulan PSL v2 at: diff --git a/src/daemon/entry/cri/cri_image_manager_service_impl.h b/src/daemon/entry/cri/cri_image_manager_service_impl.h index 4c317c1f..b94f8908 100644 --- a/src/daemon/entry/cri/cri_image_manager_service_impl.h +++ b/src/daemon/entry/cri/cri_image_manager_service_impl.h @@ -1,5 +1,5 @@ /****************************************************************************** - * Copyright (c) Huawei Technologies Co., Ltd. 2020. All rights reserved. + * Copyright (c) Huawei Technologies Co., Ltd. 2017-2020. All rights reserved. * iSulad licensed under the Mulan PSL v2. * You can use this software according to the terms and conditions of the Mulan PSL v2. * You may obtain a copy of Mulan PSL v2 at: @@ -19,7 +19,7 @@ #include #include #include -// #include "cri_services.h" + #include "image_api.h" #include "cri_image_manager_service.h" diff --git a/src/daemon/entry/cri/websocket/service/ws_server.cc b/src/daemon/entry/cri/websocket/service/ws_server.cc index 078c856c..6c379d9d 100644 --- a/src/daemon/entry/cri/websocket/service/ws_server.cc +++ b/src/daemon/entry/cri/websocket/service/ws_server.cc @@ -712,6 +712,11 @@ void DoWriteToClient(SessionData *session, const void *data, size_t len, Websock ERROR("Out of memory"); return; } + if (len > MAX_BUFFER_SIZE) { + ERROR("Message exceeds maximum length %d, len = %zu", MAX_BUFFER_SIZE, len); + free(buf); + return; + } // Determine if it is standard output channel or error channel buf[LWS_PRE] = static_cast(channel); diff --git a/src/daemon/modules/runtime/shim/shim_rt_ops.c b/src/daemon/modules/runtime/shim/shim_rt_ops.c index a437399d..d86418b0 100644 --- a/src/daemon/modules/runtime/shim/shim_rt_ops.c +++ b/src/daemon/modules/runtime/shim/shim_rt_ops.c @@ -495,6 +495,12 @@ static int file_read_address(const char *fname, char *addr) goto out; } + if (strlen(buf) >= PATH_MAX) { + ERROR("address in file %s is too long", fname); + ret = -1; + goto out; + } + (void)stpcpy(addr, buf); out: diff --git a/src/daemon/modules/service/service_container.c b/src/daemon/modules/service/service_container.c index 5d6cabf7..f278d8ab 100644 --- a/src/daemon/modules/service/service_container.c +++ b/src/daemon/modules/service/service_container.c @@ -1374,7 +1374,7 @@ static int force_kill(container_t *cont) { int ret = 0; const char *id = cont->common_config->id; - int stop_signal = container_stop_signal(cont); + uint32_t stop_signal = container_stop_signal(cont); ret = kill_with_signal(cont, SIGKILL); if (ret != 0) { @@ -1401,7 +1401,7 @@ int stop_container(container_t *cont, int timeout, bool force, bool restart) { int ret = 0; char *id = NULL; - int stop_signal = 0; + uint32_t stop_signal = 0; if (cont == NULL) { ERROR("Invalid input arguments"); diff --git a/src/daemon/modules/spec/parse_volume.c b/src/daemon/modules/spec/parse_volume.c index 40c4cecb..3d416a05 100644 --- a/src/daemon/modules/spec/parse_volume.c +++ b/src/daemon/modules/spec/parse_volume.c @@ -112,6 +112,11 @@ static int check_mount_source(const defs_mount *m) int append_default_tmpfs_options(defs_mount *m) { + + if (m == NULL) { + return -1; + } + if (util_array_append(&m->options, "noexec") != 0) { ERROR("append default tmpfs options noexec failed"); return -1; diff --git a/src/daemon/modules/spec/specs_extend.c b/src/daemon/modules/spec/specs_extend.c index c8faa102..5ede7936 100644 --- a/src/daemon/modules/spec/specs_extend.c +++ b/src/daemon/modules/spec/specs_extend.c @@ -384,6 +384,10 @@ int merge_env_target_file(oci_runtime_spec *oci_spec, const char *env_target_fil char *env_path = NULL; json_map_string_string *env_map = NULL; + if (oci_spec == NULL) { + return -1; + } + if (env_target_file == NULL) { return 0; } @@ -484,6 +488,10 @@ char *oci_container_get_env(const oci_runtime_spec *oci_spec, const char *key) int make_sure_oci_spec_linux(oci_runtime_spec *oci_spec) { + if (oci_spec == NULL) { + return -1; + } + if (oci_spec->linux == NULL) { oci_spec->linux = util_common_calloc_s(sizeof(oci_runtime_config_linux)); if (oci_spec->linux == NULL) { @@ -495,6 +503,10 @@ int make_sure_oci_spec_linux(oci_runtime_spec *oci_spec) int make_sure_oci_spec_process(oci_runtime_spec *oci_spec) { + if (oci_spec == NULL) { + return -1; + } + if (oci_spec->process == NULL) { oci_spec->process = util_common_calloc_s(sizeof(defs_process)); if (oci_spec->process == NULL) { diff --git a/src/daemon/modules/spec/specs_mount.c b/src/daemon/modules/spec/specs_mount.c index 7ee4c5f9..3b26e091 100644 --- a/src/daemon/modules/spec/specs_mount.c +++ b/src/daemon/modules/spec/specs_mount.c @@ -144,7 +144,7 @@ int adapt_settings_for_mounts(oci_runtime_spec *oci_spec, container_config *cont int ret = 0; char **array_str = NULL; - if (container_spec == NULL) { + if (oci_spec == NULL || container_spec == NULL) { return -1; } @@ -1235,6 +1235,10 @@ int merge_all_devices_and_all_permission(oci_runtime_spec *oci_spec) defs_resources *ptr = NULL; defs_device_cgroup *spec_dev_cgroup = NULL; + if (oci_spec == NULL) { + return -1; + } + ret = merge_all_devices_in_dir("/dev", NULL, NULL, oci_spec); if (ret != 0) { ERROR("Failed to merge all devices in /dev"); @@ -2212,6 +2216,11 @@ int merge_conf_device(oci_runtime_spec *oci_spec, host_config *host_spec) { int ret = 0; + if (oci_spec == NULL || host_spec == NULL) { + ERROR("Invalid input arguments"); + return -1; + } + /* blkio weight devices */ if (host_spec->blkio_weight_device != NULL && host_spec->blkio_weight_device_len != 0) { ret = merge_blkio_weight_device(oci_spec, host_spec->blkio_weight_device, host_spec->blkio_weight_device_len); @@ -2909,7 +2918,7 @@ static int calc_volumes_from_len(host_config *host_spec, size_t *len) char *mode = NULL; container_t *cont = NULL; int ret = 0; - int i = 0; + size_t i = 0; *len = 0; for (i = 0; i < host_spec->volumes_from_len; i++) { @@ -3347,11 +3356,18 @@ out: int merge_conf_mounts(oci_runtime_spec *oci_spec, host_config *host_spec, container_config_v2_common_config *v2_spec) { int ret = 0; - container_config *container_spec = v2_spec->config; + container_config *container_spec = NULL; defs_mount **all_fs_mounts = NULL; size_t all_fs_mounts_len = 0; bool mounted = false; + if (oci_spec == NULL || host_spec == NULL || v2_spec == NULL) { + ERROR("Invalid arguments"); + return -1; + } + + container_spec = v2_spec->config; + ret = im_mount_container_rootfs(v2_spec->image_type, v2_spec->image, v2_spec->id); if (ret != 0) { ERROR("Mount container %s failed when merge mounts", v2_spec->id); @@ -3379,6 +3395,7 @@ int merge_conf_mounts(oci_runtime_spec *oci_spec, host_config *host_spec, contai if (host_spec->host_channel != NULL) { if (!add_host_channel_mount(&all_fs_mounts, &all_fs_mounts_len, host_spec->host_channel)) { ERROR("Failed to merge host channel mount"); + ret = -1; goto out; } } @@ -3402,7 +3419,11 @@ int merge_conf_mounts(oci_runtime_spec *oci_spec, host_config *host_spec, contai /* add ipc mount */ if (v2_spec->shm_path != NULL) { // check whether duplication - add_shm_mount(&all_fs_mounts, &all_fs_mounts_len, v2_spec->shm_path); + if (!add_shm_mount(&all_fs_mounts, &all_fs_mounts_len, v2_spec->shm_path)) { + ERROR("Failed to add shm mount"); + ret = -1; + goto out; + } } if (!host_spec->system_container) { @@ -3446,6 +3467,11 @@ int add_rootfs_mount(const container_config *container_spec) int ret = 0; char *mntparent = NULL; + if (container_spec == NULL) { + ERROR("Invalid arguments"); + return -1; + } + mntparent = conf_get_isulad_mount_rootfs(); if (mntparent == NULL) { ERROR("Failed to get mount parent directory"); diff --git a/src/daemon/modules/spec/specs_mount.h b/src/daemon/modules/spec/specs_mount.h index 3283d92b..8a28f0e2 100644 --- a/src/daemon/modules/spec/specs_mount.h +++ b/src/daemon/modules/spec/specs_mount.h @@ -41,8 +41,6 @@ int set_mounts_readwrite_option(const oci_runtime_spec *oci_spec); int merge_all_devices_and_all_permission(oci_runtime_spec *oci_spec); -bool mount_run_tmpfs(oci_runtime_spec *container, const host_config *host_spec, const char *path); - int merge_conf_device(oci_runtime_spec *oci_spec, host_config *host_spec); int setup_ipc_dirs(host_config *host_spec, container_config_v2_common_config *v2_spec); diff --git a/src/daemon/modules/spec/specs_namespace.c b/src/daemon/modules/spec/specs_namespace.c index 2bf4cc36..3c54911a 100644 --- a/src/daemon/modules/spec/specs_namespace.c +++ b/src/daemon/modules/spec/specs_namespace.c @@ -200,13 +200,15 @@ int get_network_namespace_path(const host_config *host_spec, { SHARE_NAMESPACE_FILE, handle_get_path_from_file }, }; size_t jump_table_size = sizeof(handler_jump_table) / sizeof(handler_jump_table[0]); - const char *network_mode = host_spec->network_mode; + const char *network_mode = NULL; - if (network_mode == NULL || dest_path == NULL) { + if (host_spec == NULL || host_spec->network_mode == NULL || dest_path == NULL) { ERROR("Invalid input"); return -1; } + network_mode = host_spec->network_mode; + for (index = 0; index < jump_table_size; ++index) { if (strncmp(network_mode, handler_jump_table[index].mode, strlen(handler_jump_table[index].mode)) == 0) { ret = handler_jump_table[index].handle(host_spec, network_settings, type, dest_path); diff --git a/src/daemon/modules/spec/specs_security.c b/src/daemon/modules/spec/specs_security.c index d4884097..432339b0 100644 --- a/src/daemon/modules/spec/specs_security.c +++ b/src/daemon/modules/spec/specs_security.c @@ -501,11 +501,6 @@ static size_t docker_seccomp_arches_count(const char *seccomp_architecture, cons size_t count = 0; size_t i = 0; - if (seccomp_architecture == NULL) { - ERROR("Invalid input seccomp architecture"); - return -1; - } - for (i = 0; i < docker_seccomp_spec->arch_map_len; ++i) { if (docker_seccomp_spec->arch_map[i] == NULL || docker_seccomp_spec->arch_map[i]->architecture == NULL) { continue; @@ -532,10 +527,6 @@ static int dup_architectures_to_oci_spec(const char *seccomp_architecture, const } arch_size = docker_seccomp_arches_count(seccomp_architecture, docker_seccomp_spec); - if (arch_size < 0) { - ERROR("Failed to get arches count from docker seccomp spec"); - return -1; - } if (arch_size == 0) { WARN("arch map is not provided in specified seccomp profile"); @@ -743,7 +734,7 @@ int merge_default_seccomp_spec(oci_runtime_spec *oci_spec, const defs_process_ca oci_runtime_config_linux_seccomp *oci_seccomp_spec = NULL; docker_seccomp *docker_seccomp_spec = NULL; - if (oci_spec->process == NULL || oci_spec->process->capabilities == NULL) { + if (oci_spec == NULL || oci_spec->process == NULL || oci_spec->process->capabilities == NULL) { return 0; } diff --git a/src/daemon/modules/volume/local.c b/src/daemon/modules/volume/local.c index 87b90317..c46d4de0 100644 --- a/src/daemon/modules/volume/local.c +++ b/src/daemon/modules/volume/local.c @@ -403,6 +403,7 @@ static struct volume *volume_create_nolock(char *name) if (!map_insert(g_volumes->vols_by_name, v->name, v)) { ERROR("failed to insert volume %s", v->name); + ret = -1; goto out; } @@ -630,7 +631,7 @@ int register_local_volume(char *root_dir) } local_volume_root_dir = util_path_join(root_dir, LOCAL_VOLUME_ROOT_DIR_NAME); - if (root_dir == NULL) { + if (local_volume_root_dir == NULL) { ERROR("out of memory"); ret = -1; goto out; diff --git a/src/daemon/modules/volume/volume.c b/src/daemon/modules/volume/volume.c index 8255aff9..0d348bb4 100644 --- a/src/daemon/modules/volume/volume.c +++ b/src/daemon/modules/volume/volume.c @@ -130,7 +130,7 @@ static int insert_driver(char *name, volume_driver *driver) } if (!map_insert(g_vs.drivers, name, d)) { - ERROR("out of memory"); + ERROR("Failed to insert volume driver %s", name); ret = -1; goto out; } -- 2.40.1