From 5e530a5b505e069d0d321c8d7c4a9c7db69ba5b2 Mon Sep 17 00:00:00 2001 From: DriedYellowPeach Date: Sat, 10 Dec 2022 13:17:57 +0000 Subject: [PATCH 8/9] !209 handle security warning * handle security warning --- src/conf.c | 41 +++++++++++++++++++------------------- src/lcrcontainer.c | 38 +++++++++++++---------------------- src/lcrcontainer_execute.c | 30 +++++++++++++++------------- src/utils.c | 25 +++++++++++++++++++++-- src/utils.h | 1 + 5 files changed, 75 insertions(+), 60 deletions(-) diff --git a/src/conf.c b/src/conf.c index 4f644d98..2b028878 100644 --- a/src/conf.c +++ b/src/conf.c @@ -42,6 +42,7 @@ #define SUB_UID_PATH "/etc/subuid" #define SUB_GID_PATH "/etc/subgid" #define ID_MAP_LEN 100 +#define DEFAULT_BUF_LEN 300 /* files limit checker for cgroup v1 */ static int files_limit_checker_v1(const char *value) @@ -401,7 +402,7 @@ static char *capabilities_join(const char *sep, const char **parts, size_t len) result_len += strlen(parts[iter]) - 4; } - result = calloc(result_len + 1, 1); + result = lcr_util_smart_calloc_s(sizeof(char), result_len + 1); if (result == NULL) { return NULL; } @@ -1018,7 +1019,7 @@ static struct lcr_list *trans_mount_auto_to_lxc(const defs_mount *mount) } buf_len = strlen(type) + strlen(options) + 2; - buf = calloc(buf_len, 1); + buf = lcr_util_smart_calloc_s(sizeof(char), buf_len); if (buf == NULL) { DEBUG("Out of memory"); goto out_free; @@ -1066,7 +1067,7 @@ static struct lcr_list *trans_mount_entry_to_lxc(const defs_mount *mount) } buf_len = strlen(replaced_dest) + strlen(mount->type) + strlen(replaced_source) + strlen(options) + 8; - buf = calloc(buf_len, 1); + buf = lcr_util_smart_calloc_s(sizeof(char), buf_len); if (buf == NULL) { ERROR("Out of memory"); goto out_free; @@ -1202,7 +1203,7 @@ static int trans_one_oci_id_mapping(struct lcr_list *conf, const char *typ, cons { int nret; struct lcr_list *node = NULL; - char buf_value[300] = { 0 }; + char buf_value[DEFAULT_BUF_LEN] = { 0 }; char subid[ID_MAP_LEN] = { 0 }; nret = snprintf(buf_value, sizeof(buf_value), "%s %u %u %u", typ, id->container_id, id->host_id, id->size); @@ -1289,7 +1290,7 @@ out_free: static int trans_conf_int(struct lcr_list *conf, const char *lxc_key, int val) { struct lcr_list *node = NULL; - char buf_value[300] = { 0 }; + char buf_value[DEFAULT_BUF_LEN] = { 0 }; int nret; nret = snprintf(buf_value, sizeof(buf_value), "%d", val); @@ -1307,7 +1308,7 @@ static int trans_conf_int(struct lcr_list *conf, const char *lxc_key, int val) static int trans_conf_uint32(struct lcr_list *conf, const char *lxc_key, uint32_t val) { struct lcr_list *node = NULL; - char buf_value[300] = { 0 }; + char buf_value[DEFAULT_BUF_LEN] = { 0 }; int nret; nret = snprintf(buf_value, sizeof(buf_value), "%u", (unsigned int)val); @@ -1325,7 +1326,7 @@ static int trans_conf_uint32(struct lcr_list *conf, const char *lxc_key, uint32_ static int trans_conf_int64(struct lcr_list *conf, const char *lxc_key, int64_t val) { struct lcr_list *node = NULL; - char buf_value[300] = { 0 }; + char buf_value[DEFAULT_BUF_LEN] = { 0 }; int nret; nret = snprintf(buf_value, sizeof(buf_value), "%lld", (long long)val); @@ -1343,7 +1344,7 @@ static int trans_conf_int64(struct lcr_list *conf, const char *lxc_key, int64_t static int trans_conf_uint64(struct lcr_list *conf, const char *lxc_key, uint64_t val) { struct lcr_list *node = NULL; - char buf_value[300] = { 0 }; + char buf_value[DEFAULT_BUF_LEN] = { 0 }; int nret; nret = snprintf(buf_value, sizeof(buf_value), "%llu", (unsigned long long)val); @@ -1565,7 +1566,7 @@ static int trans_resources_devices_v1(const defs_resources *res, struct lcr_list { int ret = -1; size_t i = 0; - char buf_value[300] = { 0 }; + char buf_value[DEFAULT_BUF_LEN] = { 0 }; for (i = 0; i < res->devices_len; i++) { defs_device_cgroup *lrd = res->devices[i]; @@ -1717,7 +1718,7 @@ static int trans_blkio_wdevice_v1(const defs_resources_block_io *block_io, struc struct lcr_list *node = NULL; int ret = -1; size_t i = 0; - char buf_value[300] = { 0 }; + char buf_value[DEFAULT_BUF_LEN] = { 0 }; for (i = 0; i < block_io->weight_device_len; i++) { int nret; @@ -1769,7 +1770,7 @@ static int trans_blkio_throttle_v1(defs_block_io_device_throttle **throttle, siz for (i = 0; i < len; i++) { if (throttle[i] && throttle[i]->rate != INVALID_INT) { int nret; - char buf_value[300] = { 0x00 }; + char buf_value[DEFAULT_BUF_LEN] = { 0x00 }; nret = snprintf(buf_value, sizeof(buf_value), "%lld:%lld %llu", (long long)throttle[i]->major, (long long)(throttle[i]->minor), (unsigned long long)(throttle[i]->rate)); if (nret < 0 || (size_t)nret >= sizeof(buf_value)) { @@ -1835,7 +1836,7 @@ static int trans_resources_hugetlb_v1(const defs_resources *res, struct lcr_list { int ret = -1; size_t i = 0; - char buf_key[300] = { 0 }; + char buf_key[DEFAULT_BUF_LEN] = { 0 }; for (i = 0; i < res->hugepage_limits_len; i++) { defs_resources_hugepage_limits_element *lrhl = res->hugepage_limits[i]; @@ -1861,7 +1862,7 @@ static int trans_resources_network_v1(const defs_resources *res, struct lcr_list { int ret = -1; size_t i = 0; - char buf_value[300] = { 0 }; + char buf_value[DEFAULT_BUF_LEN] = { 0 }; if (!res->network) { return 0; @@ -1898,7 +1899,7 @@ out: static int trans_resources_pids_v1(const defs_resources *res, struct lcr_list *conf) { int ret = -1; - char buf_value[300] = { 0 }; + char buf_value[DEFAULT_BUF_LEN] = { 0 }; if (res->pids == NULL) { return 0; @@ -2001,7 +2002,7 @@ static int trans_resources_devices_v2(const defs_resources *res, struct lcr_list { int ret = -1; size_t i = 0; - char buf_value[300] = { 0 }; + char buf_value[DEFAULT_BUF_LEN] = { 0 }; for (i = 0; i < res->devices_len; i++) { defs_device_cgroup *lrd = res->devices[i]; @@ -2096,7 +2097,7 @@ static int trans_resources_cpu_weight_v2(const defs_resources *res, struct lcr_l /* trans resources cpu max of cgroup v2, it's called quota/period in cgroup v1 */ static int trans_resources_cpu_max_v2(const defs_resources *res, struct lcr_list *conf) { - char buf_value[300] = {0}; + char buf_value[DEFAULT_BUF_LEN] = {0}; uint64_t period = res->cpu->period; int nret = 0; @@ -2195,7 +2196,7 @@ static int trans_io_weight_v2(const defs_resources_block_io *block_io, struct lc for (i = 0; i < len; i++) { if (weight_device[i] && weight_device[i]->weight != INVALID_INT) { int nret = 0; - char buf_value[300] = { 0x00 }; + char buf_value[DEFAULT_BUF_LEN] = { 0x00 }; weight = lcr_util_trans_blkio_weight_to_io_weight(weight_device[i]->weight); if (weight < CGROUP2_WEIGHT_MIN || weight > CGROUP2_WEIGHT_MAX) { @@ -2250,7 +2251,7 @@ static int trans_io_bfq_weight_v2(const defs_resources_block_io *block_io, struc for (i = 0; i < len; i++) { if (weight_device[i] && weight_device[i]->weight != INVALID_INT) { int nret = 0; - char buf_value[300] = { 0x00 }; + char buf_value[DEFAULT_BUF_LEN] = { 0x00 }; weight = lcr_util_trans_blkio_weight_to_io_weight(weight_device[i]->weight); if (weight < CGROUP2_BFQ_WEIGHT_MIN || weight > CGROUP2_BFQ_WEIGHT_MAX) { @@ -2288,7 +2289,7 @@ static int trans_io_throttle_v2(defs_block_io_device_throttle **throttle, size_t for (i = 0; i < len; i++) { if (throttle[i] && throttle[i]->rate != INVALID_INT) { int nret = 0; - char buf_value[300] = { 0x00 }; + char buf_value[DEFAULT_BUF_LEN] = { 0x00 }; nret = snprintf(buf_value, sizeof(buf_value), "%lld:%lld %s=%llu", (long long)throttle[i]->major, (long long)(throttle[i]->minor), rate_key, (unsigned long long)(throttle[i]->rate)); if (nret < 0 || (size_t)nret >= sizeof(buf_value)) { @@ -2348,7 +2349,7 @@ static int trans_resources_blkio_v2(const defs_resources_block_io *block_io, str static int trans_resources_hugetlb_v2(const defs_resources *res, struct lcr_list *conf) { size_t i = 0; - char buf_key[300] = { 0 }; + char buf_key[DEFAULT_BUF_LEN] = { 0 }; for (i = 0; i < res->hugepage_limits_len; i++) { defs_resources_hugepage_limits_element *lrhl = res->hugepage_limits[i]; diff --git a/src/lcrcontainer.c b/src/lcrcontainer.c index 14cc6c43..ca587e4f 100644 --- a/src/lcrcontainer.c +++ b/src/lcrcontainer.c @@ -181,34 +181,21 @@ int lcr_list_all_containers(const char *lcrpath, struct lcr_container_info **inf static int create_partial(const struct lxc_container *c) { - size_t len = 0; int fd = 0; int ret = 0; struct flock lk; + char path[PATH_MAX] = { 0 }; - if (strlen(c->name) > ((SIZE_MAX - strlen(c->config_path)) - 10)) { - return -1; - } - - // $lxcpath + '/' + $name + '/partial' + \0 - len = strlen(c->config_path) + strlen(c->name) + 10; - - char *path = lcr_util_common_calloc_s(len); - if (path == NULL) { - ERROR("Out of memory in create_partial"); - return -1; - } - - ret = snprintf(path, len, "%s/%s/partial", c->config_path, c->name); - if (ret < 0 || (size_t)ret >= len) { + ret = snprintf(path, PATH_MAX, "%s/%s/partial", c->config_path, c->name); + if (ret < 0 || (size_t)ret >= PATH_MAX) { ERROR("Error writing partial pathname"); - goto out_free; + return -1; } fd = lcr_util_open(path, O_RDWR | O_CREAT | O_EXCL, DEFAULT_SECURE_FILE_MODE); if (fd < 0) { SYSERROR("Error creating partial file: %s", path); - goto out_free; + return -1; } lk.l_type = F_WRLCK; lk.l_whence = SEEK_SET; @@ -217,15 +204,10 @@ static int create_partial(const struct lxc_container *c) if (fcntl(fd, F_SETLKW, &lk) < 0) { SYSERROR("Error locking partial file %s", path); close(fd); - goto out_free; + return -1; } - free(path); return fd; - -out_free: - free(path); - return -1; } static void remove_partial(const struct lxc_container *c) @@ -752,6 +734,10 @@ out_put: void lcr_container_state_free(struct lcr_container_state *lcs) { + if (lcs == NULL) { + return; + } + free(lcs->name); lcs->name = NULL; free(lcs->state); @@ -1045,6 +1031,10 @@ out: void lcr_free_console_config(struct lcr_console_config *config) { + if (config == NULL) { + return; + } + free(config->log_path); config->log_path = NULL; free(config->log_file_size); diff --git a/src/lcrcontainer_execute.c b/src/lcrcontainer_execute.c index 9f4e9515..ac49c501 100644 --- a/src/lcrcontainer_execute.c +++ b/src/lcrcontainer_execute.c @@ -41,6 +41,8 @@ #include "oci_runtime_spec.h" #include "lcrcontainer_extend.h" +#define NUM_STR_LEN 128 + // Cgroup v1 Item Definition #define CGROUP_BLKIO_WEIGHT "blkio.weight" #define CGROUP_CPU_SHARES "cpu.shares" @@ -164,7 +166,7 @@ static int update_resources_cpuset_mems_v2(struct lxc_container *c, const struct static int update_resources_cpu_shares(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { int ret = 0; - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->cpu_shares != 0) { int num = snprintf(numstr, sizeof(numstr), "%llu", (unsigned long long)(cr->cpu_shares)); @@ -186,7 +188,7 @@ out: static int update_resources_cpu_weight_v2(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->cpu_shares == 0) { return 0; @@ -215,7 +217,7 @@ static int update_resources_cpu_weight_v2(struct lxc_container *c, const struct static int update_resources_cpu_period(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { int ret = 0; - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->cpu_period != 0) { int num = snprintf(numstr, sizeof(numstr), "%llu", (unsigned long long)(cr->cpu_period)); @@ -240,7 +242,7 @@ static int update_resources_cpu_max_v2(struct lxc_container *c, const struct lcr int num = 0; uint64_t period = cr->cpu_period; int64_t quota = cr->cpu_quota; - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (quota == 0 && period == 0) { return 0; @@ -318,7 +320,7 @@ out: static int update_resources_cpu_quota(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { int ret = 0; - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->cpu_quota != 0) { int num = snprintf(numstr, sizeof(numstr), "%lld", (long long int)cr->cpu_quota); @@ -398,7 +400,7 @@ static int update_resources_cpu_v2(struct lxc_container *c, const struct lcr_cgr static int update_resources_memory_limit(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { int ret = 0; - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->memory_limit != 0) { int num = snprintf(numstr, sizeof(numstr), "%llu", (unsigned long long)(cr->memory_limit)); @@ -436,7 +438,7 @@ static int trans_int64_to_numstr_with_max(int64_t value, char *numstr, size_t si static int update_resources_memory_limit_v2(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->memory_limit == 0) { return 0; @@ -457,7 +459,7 @@ static int update_resources_memory_limit_v2(struct lxc_container *c, const struc static int update_resources_memory_swap(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { int ret = 0; - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->memory_swap != 0) { int num = snprintf(numstr, sizeof(numstr), "%llu", (unsigned long long)(cr->memory_swap)); @@ -479,7 +481,7 @@ out: static int update_resources_memory_swap_v2(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ int64_t swap = 0; if (cr->memory_swap == 0) { @@ -505,7 +507,7 @@ static int update_resources_memory_swap_v2(struct lxc_container *c, const struct static int update_resources_memory_reservation(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { int ret = 0; - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->memory_reservation != 0) { int num = snprintf(numstr, sizeof(numstr), "%llu", (unsigned long long)(cr->memory_reservation)); @@ -527,7 +529,7 @@ out: static int update_resources_memory_reservation_v2(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->memory_reservation == 0) { return 0; @@ -610,7 +612,7 @@ static int update_resources_mem_v2(struct lxc_container *c, struct lcr_cgroup_re static int update_resources_blkio_weight_v1(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { int ret = 0; - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->blkio_weight != 0) { int num = snprintf(numstr, sizeof(numstr), "%llu", (unsigned long long)(cr->blkio_weight)); @@ -633,7 +635,7 @@ out: static int update_resources_io_weight_v2(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { uint64_t weight = 0; - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->blkio_weight == 0) { return 0; @@ -666,7 +668,7 @@ static int update_resources_io_weight_v2(struct lxc_container *c, const struct l static int update_resources_io_bfq_weight_v2(struct lxc_container *c, const struct lcr_cgroup_resources *cr) { uint64_t weight = 0; - char numstr[128] = {0}; /* max buffer */ + char numstr[NUM_STR_LEN] = {0}; /* max buffer */ if (cr->blkio_weight == 0) { return 0; diff --git a/src/utils.c b/src/utils.c index 16719f67..4b123bb7 100644 --- a/src/utils.c +++ b/src/utils.c @@ -40,6 +40,14 @@ #include "utils.h" #include "log.h" +#if __WORDSIZE == 64 +// current max user memory for 64-machine is 2^47 B +#define MAX_MEMORY_SIZE ((size_t)1 << 47) +#else +// current max user memory for 32-machine is 2^31 B +#define MAX_MEMORY_SIZE ((size_t)1 << 31) +#endif + #define ISSLASH(C) ((C) == '/') #define IS_ABSOLUTE_FILE_NAME(F) (ISSLASH((F)[0])) #define IS_RELATIVE_FILE_NAME(F) (!IS_ABSOLUTE_FILE_NAME(F)) @@ -381,14 +389,27 @@ size_t lcr_array_len(void **orig_array) return length; } +void *lcr_util_smart_calloc_s(size_t unit_size, size_t count) +{ + if (unit_size == 0) { + return NULL; + } + + if (count > (MAX_MEMORY_SIZE / unit_size)) { + return NULL; + } + + return calloc(count, unit_size); +} + /* util common malloc s */ void *lcr_util_common_calloc_s(size_t size) { - if (size == 0) { + if (size == 0 || size > MAX_MEMORY_SIZE) { return NULL; } - return calloc(1, size); + return calloc((size_t)1, size); } int lcr_mem_realloc(void **newptr, size_t newsize, void *oldptr, size_t oldsize) diff --git a/src/utils.h b/src/utils.h index 865b899a..6a3764b8 100644 --- a/src/utils.h +++ b/src/utils.h @@ -198,6 +198,7 @@ int lcr_util_recursive_rmdir(const char *dirpath, int recursive_depth); char *lcr_util_string_replace(const char *needle, const char *replacement, const char *haystack); int lcr_util_open(const char *filename, int flags, mode_t mode); +void *lcr_util_smart_calloc_s(size_t unit_size, size_t count); void *lcr_util_common_calloc_s(size_t size); int lcr_util_safe_int(const char *numstr, int *converted); int lcr_util_check_inherited(bool closeall, int fd_to_ignore); -- 2.25.1