From f7edf9518a6b11de69d8cfe2c1dbf9bf3a1dacbb Mon Sep 17 00:00:00 2001 From: liuhao Date: Thu, 4 Jul 2019 12:08:17 +0800 Subject: [PATCH 113/138] Malloc parameter check and judgment Signed-off-by: wujing Signed-off-by: LiFeng --- src/lxc/attach.c | 2 +- src/lxc/commands.c | 9 ++- src/lxc/conf.c | 20 ++++-- src/lxc/confile.c | 8 +-- src/lxc/json/container_start_generate_config.c | 12 +++- src/lxc/json/defs.c | 8 +++ src/lxc/json/json_common.c | 97 +++++++++----------------- src/lxc/json/logger_json_file.c | 6 +- src/lxc/lxccontainer.c | 12 +++- src/lxc/path.c | 8 ++- src/lxc/start.c | 6 +- src/lxc/terminal.c | 2 +- src/lxc/utils.c | 4 +- 13 files changed, 104 insertions(+), 90 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index d7b16e3..ac4bd39 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -1188,7 +1188,7 @@ static int create_attach_timeout_thread(int64_t attach_timeout, pid_t pid) struct attach_timeout_conf *timeout_conf = NULL; timeout_conf = malloc(sizeof(struct attach_timeout_conf)); - if (!timeout_conf) { + if (timeout_conf == NULL) { ERROR("Failed to malloc attach timeout conf"); ret = -1; goto out; diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 0802a16..fa02a4b 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -1075,11 +1075,16 @@ int lxc_cmd_set_terminal_fifos(const char *name, const char *lxcpath, const char const char *cmd_out_fifo = out_fifo ? out_fifo : none_fifo_name; const char *cmd_err_fifo = err_fifo ? err_fifo : none_fifo_name; + if (len + strlen(cmd_in_fifo) + strlen(split) + strlen(cmd_out_fifo) + + strlen(split) + strlen(cmd_err_fifo) == SIZE_MAX) + return -1; len += strlen(cmd_in_fifo) + strlen(split) + strlen(cmd_out_fifo) + strlen(split) + strlen(cmd_err_fifo) + 1; tmp = malloc(len); - if (!tmp) + if (tmp == NULL) + return -1; + ret = snprintf(tmp, len, "%s%s%s%s%s", cmd_in_fifo, split, cmd_out_fifo, split, cmd_err_fifo); + if (ret < 0) return -1; - snprintf(tmp, len, "%s%s%s%s%s", cmd_in_fifo, split, cmd_out_fifo, split, cmd_err_fifo); struct lxc_cmd_rr cmd = { .req = { diff --git a/src/lxc/conf.c b/src/lxc/conf.c index 996d267..3ecccbd 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -4393,7 +4393,11 @@ int lxc_drop_caps(struct lxc_conf *conf) // caplist[i] is 1 if we keep capability i caplist = malloc(numcaps * sizeof(int)); - memset(caplist, 0, numcaps * sizeof(int)); + if (caplist == NULL) { + ERROR("Out of memory"); + return -1; + } + (void)memset(caplist, 0, numcaps * sizeof(int)); lxc_list_for_each(iterator, caps) { @@ -4488,8 +4492,8 @@ static char* generate_json_str(const char *name, const char *lxcpath, const char cpid = "-1"; } - if ((SIZE_MAX - strlen(name) - strlen(cpid) - strlen(rootfs) - strlen(lxcpath) - strlen(name)) < - (strlen("{\"ociVersion\":\"\",\"id\":\"\",\"pid\":,\"root\":\"\",\"bundle\":\"\"}") + 1 + 1)) { + if ((strlen(name) + strlen(cpid) + strlen(rootfs) + strlen(lxcpath) + strlen(name)) > + SIZE_MAX - (strlen("{\"ociVersion\":\"\",\"id\":\"\",\"pid\":,\"root\":\"\",\"bundle\":\"\"}") - 1 - 1)) { ERROR("Out of memory"); ret = -1; goto out_free; @@ -4499,7 +4503,7 @@ static char* generate_json_str(const char *name, const char *lxcpath, const char size = strlen("{\"ociVersion\":\"\",\"id\":\"\",\"pid\":,\"root\":\"\",\"bundle\":\"\"}") + strlen(name) + strlen(cpid) + strlen(rootfs) + strlen(lxcpath) + 1 + strlen(name) + 1; inmsg = malloc(size); - if (!inmsg) { + if (inmsg == NULL) { ERROR("Out of memory"); ret = -1; goto out_free; @@ -4531,9 +4535,11 @@ static char **merge_ocihook_env(char **oldenvs, size_t env_len, size_t *merge_en }; char *lxcenv_buf = NULL; + if (result_len > SIZE_MAX - (sizeof(lxc_envs) / sizeof(char *)) - 1) + return NULL; result_len += (sizeof(lxc_envs) / sizeof(char *)) + 1; result = malloc(sizeof(char *) * result_len); - if (!result) + if (result == NULL) return NULL; memset(result, 0, sizeof(char *) * result_len); @@ -4552,7 +4558,7 @@ static char **merge_ocihook_env(char **oldenvs, size_t env_len, size_t *merge_en } env_buf_len = ((strlen(tmpenv) + 1) + strlen(lxc_envs[j])) + 1; lxcenv_buf = malloc(env_buf_len); - if (!lxcenv_buf) { + if (lxcenv_buf == NULL) { lxc_free_array((void **)result, free); return NULL; } @@ -4759,7 +4765,7 @@ static int run_ocihook_buffer(struct oci_hook_conf *oconf, const char *inmsg) } conf = malloc(sizeof(struct wait_conf)); - if (!conf) { + if (conf == NULL) { SYSERROR("Failed to malloc."); goto on_error; } diff --git a/src/lxc/confile.c b/src/lxc/confile.c index 8a082d5..a75e023 100644 --- a/src/lxc/confile.c +++ b/src/lxc/confile.c @@ -2318,13 +2318,13 @@ static int set_config_populate_device(const char *key, const char *value, /* allocate list element */ dev_list = malloc(sizeof(*dev_list)); - if (!dev_list) + if (dev_list == NULL) goto on_error; lxc_list_init(dev_list); dev_elem = malloc(sizeof(*dev_elem)); - if (!dev_elem) + if (dev_elem == NULL) goto on_error; memset(dev_elem, 0, sizeof(*dev_elem)); @@ -2362,7 +2362,7 @@ static int set_config_rootfs_masked_paths(const char *key, const char *value, return lxc_clear_rootfs_masked_paths(lxc_conf); list_item = malloc(sizeof(*list_item)); - if (!list_item) + if (list_item == NULL) goto on_error; list_item->elem = safe_strdup(value); @@ -2387,7 +2387,7 @@ static int set_config_rootfs_ro_paths(const char *key, const char *value, return lxc_clear_rootfs_ro_paths(lxc_conf); list_item = malloc(sizeof(*list_item)); - if (!list_item) + if (list_item == NULL) goto on_error; list_item->elem = safe_strdup(value); diff --git a/src/lxc/json/container_start_generate_config.c b/src/lxc/json/container_start_generate_config.c index 5ec8311..3748973 100644 --- a/src/lxc/json/container_start_generate_config.c +++ b/src/lxc/json/container_start_generate_config.c @@ -41,6 +41,11 @@ container_start_generate_config *make_container_start_generate_config(yajl_val t if (tmp != NULL && YAJL_GET_ARRAY(tmp) != NULL && YAJL_GET_ARRAY(tmp)->len > 0) { size_t i; ret->additional_gids_len = YAJL_GET_ARRAY(tmp)->len; + if (YAJL_GET_ARRAY(tmp)->len > SIZE_MAX / sizeof(*ret->additional_gids) - 1) { + *err = safe_strdup("invalid additional gids size"); + free_container_start_generate_config(ret); + return NULL; + } ret->additional_gids = safe_malloc((YAJL_GET_ARRAY(tmp)->len + 1) * sizeof(*ret->additional_gids)); for (i = 0; i < YAJL_GET_ARRAY(tmp)->len; i++) { yajl_val val = YAJL_GET_ARRAY(tmp)->values[i]; @@ -233,8 +238,13 @@ char *container_start_generate_config_generate_json(container_start_generate_con goto free_out; } + if (gen_len == SIZE_MAX) { + *err = safe_strdup("Invalid buffer length"); + goto free_out; + } + json_buf = safe_malloc(gen_len + 1); - (void)memcpy(json_buf, gen_buf, gen_len); + (void)memcpy(json_buf, gen_buf, gen_len); json_buf[gen_len] = '\0'; free_out: diff --git a/src/lxc/json/defs.c b/src/lxc/json/defs.c index 8a052a8..4bf569a 100644 --- a/src/lxc/json/defs.c +++ b/src/lxc/json/defs.c @@ -24,6 +24,10 @@ defs_hook *make_defs_hook(yajl_val tree, struct parser_context *ctx, parser_erro if (tmp != NULL && YAJL_GET_ARRAY(tmp) != NULL && YAJL_GET_ARRAY(tmp)->len > 0) { size_t i; ret->args_len = YAJL_GET_ARRAY(tmp)->len; + if (YAJL_GET_ARRAY(tmp)->len > SIZE_MAX / sizeof(*ret->args) - 1) { + free_defs_hook(ret); + return NULL; + } ret->args = safe_malloc((YAJL_GET_ARRAY(tmp)->len + 1) * sizeof(*ret->args)); for (i = 0; i < YAJL_GET_ARRAY(tmp)->len; i++) { yajl_val val = YAJL_GET_ARRAY(tmp)->values[i]; @@ -39,6 +43,10 @@ defs_hook *make_defs_hook(yajl_val tree, struct parser_context *ctx, parser_erro if (tmp != NULL && YAJL_GET_ARRAY(tmp) != NULL && YAJL_GET_ARRAY(tmp)->len > 0) { size_t i; ret->env_len = YAJL_GET_ARRAY(tmp)->len; + if (YAJL_GET_ARRAY(tmp)->len > SIZE_MAX / sizeof(*ret->env) - 1) { + free_defs_hook(ret); + return NULL; + } ret->env = safe_malloc((YAJL_GET_ARRAY(tmp)->len + 1) * sizeof(*ret->env)); for (i = 0; i < YAJL_GET_ARRAY(tmp)->len; i++) { yajl_val val = YAJL_GET_ARRAY(tmp)->values[i]; diff --git a/src/lxc/json/json_common.c b/src/lxc/json/json_common.c index 5da1e6d..ed2fe83 100755 --- a/src/lxc/json/json_common.c +++ b/src/lxc/json/json_common.c @@ -445,6 +445,9 @@ json_map_int_int *make_json_map_int_int(yajl_val src, struct parser_context *ctx if (src != NULL && YAJL_GET_OBJECT(src) != NULL) { size_t i; size_t len = YAJL_GET_OBJECT(src)->len; + if (len > SIZE_MAX / sizeof(int) - 1) { + return NULL; + } ret = safe_malloc(sizeof(*ret)); ret->len = len; ret->keys = safe_malloc((len + 1) * sizeof(int)); @@ -505,16 +508,8 @@ int append_json_map_int_int(json_map_int_int *map, int key, int val) { vals = safe_malloc(len * sizeof(int)); if (map->len) { - if (memcpy(keys, map->keys, map->len * sizeof(int)) != NULL) { - free(keys); - free(vals); - return -1; - } - if (memcpy(vals, map->values, map->len * sizeof(int)) != NULL) { - free(keys); - free(vals); - return -1; - } + (void)memcpy(keys, map->keys, map->len * sizeof(int)); + (void)memcpy(vals, map->values, map->len * sizeof(int)); } free(map->keys); map->keys = keys; @@ -591,6 +586,9 @@ json_map_int_bool *make_json_map_int_bool(yajl_val src, struct parser_context *c if (src != NULL && YAJL_GET_OBJECT(src) != NULL) { size_t i; size_t len = YAJL_GET_OBJECT(src)->len; + if (len > SIZE_MAX / sizeof(int) - 1) { + return NULL; + } ret = safe_malloc(sizeof(*ret)); ret->len = len; ret->keys = safe_malloc((len + 1) * sizeof(int)); @@ -646,16 +644,8 @@ int append_json_map_int_bool(json_map_int_bool *map, int key, bool val) { vals = safe_malloc(len * sizeof(bool)); if (map->len) { - if (memcpy(keys, map->keys, map->len * sizeof(int)) != NULL) { - free(keys); - free(vals); - return -1; - } - if (memcpy(vals, map->values, map->len * sizeof(bool)) != NULL) { - free(keys); - free(vals); - return -1; - } + (void)memcpy(keys, map->keys, map->len * sizeof(int)); + (void)memcpy(vals, map->values, map->len * sizeof(bool)); } free(map->keys); map->keys = keys; @@ -733,6 +723,9 @@ json_map_int_string *make_json_map_int_string(yajl_val src, struct parser_contex if (src != NULL && YAJL_GET_OBJECT(src) != NULL) { size_t i; size_t len = YAJL_GET_OBJECT(src)->len; + if (len > SIZE_MAX / sizeof(char *) - 1) { + return NULL; + } ret = safe_malloc(sizeof(*ret)); ret->len = len; ret->keys = safe_malloc((len + 1) * sizeof(int)); @@ -786,16 +779,8 @@ int append_json_map_int_string(json_map_int_string *map, int key, const char *va vals = safe_malloc(len * sizeof(char *)); if (map->len) { - if (memcpy(keys, map->keys, map->len * sizeof(int)) != NULL) { - free(keys); - free(vals); - return -1; - } - if (memcpy(vals, map->values, map->len * sizeof(char *)) != NULL) { - free(keys); - free(vals); - return -1; - } + (void)memcpy(keys, map->keys, map->len * sizeof(int)); + (void)memcpy(vals, map->values, map->len * sizeof(char *)); } free(map->keys); map->keys = keys; @@ -864,6 +849,9 @@ json_map_string_int *make_json_map_string_int(yajl_val src, struct parser_contex if (src != NULL && YAJL_GET_OBJECT(src) != NULL) { size_t i; size_t len = YAJL_GET_OBJECT(src)->len; + if (len > SIZE_MAX / sizeof(char *) - 1) { + return NULL; + } ret = safe_malloc(sizeof(*ret)); ret->len = len; ret->keys = safe_malloc((len + 1) * sizeof(char *)); @@ -913,16 +901,8 @@ int append_json_map_string_int(json_map_string_int *map, const char *key, int va vals = safe_malloc(len * sizeof(int)); if (map->len) { - if (memcpy(keys, map->keys, map->len * sizeof(char *)) != NULL) { - free(keys); - free(vals); - return -1; - } - if (memcpy(vals, map->values, map->len * sizeof(int)) != NULL) { - free(keys); - free(vals); - return -1; - } + (void)memcpy(keys, map->keys, map->len * sizeof(char *)); + (void)memcpy(vals, map->values, map->len * sizeof(int)); } free(map->keys); map->keys = keys; @@ -991,6 +971,9 @@ json_map_string_bool *make_json_map_string_bool(yajl_val src, struct parser_cont if (src != NULL && YAJL_GET_OBJECT(src) != NULL) { size_t i; size_t len = YAJL_GET_OBJECT(src)->len; + if (len > SIZE_MAX / sizeof(char *) - 1) { + return NULL; + } ret = safe_malloc(sizeof(*ret)); ret->len = len; ret->keys = safe_malloc((len + 1) * sizeof(char *)); @@ -1017,6 +1000,7 @@ json_map_string_bool *make_json_map_string_bool(yajl_val src, struct parser_cont } return ret; } + int append_json_map_string_bool(json_map_string_bool *map, const char *key, bool val) { size_t len; char **keys = NULL; @@ -1035,16 +1019,8 @@ int append_json_map_string_bool(json_map_string_bool *map, const char *key, bool vals = safe_malloc(len * sizeof(bool)); if (map->len) { - if (memcpy(keys, map->keys, map->len * sizeof(char *)) != NULL) { - free(keys); - free(vals); - return -1; - } - if (memcpy(vals, map->values, map->len * sizeof(bool)) != NULL) { - free(keys); - free(vals); - return -1; - } + (void)memcpy(keys, map->keys, map->len * sizeof(char *)); + (void)memcpy(vals, map->values, map->len * sizeof(bool)); } free(map->keys); map->keys = keys; @@ -1114,6 +1090,9 @@ json_map_string_string *make_json_map_string_string(yajl_val src, struct parser_ if (src != NULL && YAJL_GET_OBJECT(src) != NULL) { size_t i; size_t len = YAJL_GET_OBJECT(src)->len; + if (len > SIZE_MAX / sizeof(char *) - 1) { + return NULL; + } ret = safe_malloc(sizeof(*ret)); ret->len = len; ret->keys = safe_malloc((len + 1) * sizeof(char *)); @@ -1149,9 +1128,9 @@ int append_json_map_string_string(json_map_string_string *map, const char *key, for (i = 0; i < map->len; i++) { if (strcmp(map->keys[i], key) == 0) { - free(map->values[i]); - map->values[i] = safe_strdup(val ? val : ""); - return 0; + free(map->values[i]); + map->values[i] = safe_strdup(val ? val : ""); + return 0; } } @@ -1164,16 +1143,8 @@ int append_json_map_string_string(json_map_string_string *map, const char *key, vals = safe_malloc(len * sizeof(char *)); if (map->len) { - if (memcpy(keys, map->keys, map->len * sizeof(char *)) != NULL) { - free(keys); - free(vals); - return -1; - } - if (memcpy(vals, map->values, map->len * sizeof(char *)) != NULL) { - free(keys); - free(vals); - return -1; - } + (void)memcpy(keys, map->keys, map->len * sizeof(char *)); + (void)memcpy(vals, map->values, map->len * sizeof(char *)); } free(map->keys); map->keys = keys; diff --git a/src/lxc/json/logger_json_file.c b/src/lxc/json/logger_json_file.c index 842d35b..6abeef4 100644 --- a/src/lxc/json/logger_json_file.c +++ b/src/lxc/json/logger_json_file.c @@ -230,8 +230,12 @@ char *logger_json_file_generate_json(logger_json_file *ptr, struct parser_contex goto free_out; } + if (gen_len == SIZE_MAX) { + *err = safe_strdup("Invalid buffer length"); + goto free_out; + } json_buf = safe_malloc(gen_len + 1); - memcpy(json_buf, gen_buf, gen_len); + (void)memcpy(json_buf, gen_buf, gen_len); json_buf[gen_len] = '\0'; free_out: diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index 3b2c5af..5a72483 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -928,6 +928,9 @@ static char **use_init_args(char **init_argv, size_t init_args) do { argv = malloc(sizeof(char *)); + if (argv == NULL) { + return NULL; + } } while (!argv); argv[0] = NULL; @@ -3378,9 +3381,12 @@ static bool set_oci_hook_config_filename(struct lxc_container *c) return false; /* $lxc_path + "/" + c->name + "/" + "config" + '\0' */ + if (strlen(c->config_path) + strlen(c->name) > SIZE_MAX - strlen(OCI_HOOK_JSON_FILE_NAME) - 3) + return false; len = strlen(c->config_path) + strlen(c->name) + strlen(OCI_HOOK_JSON_FILE_NAME) + 3; + newpath = malloc(len); - if (!newpath) + if (newpath == NULL) return false; ret = snprintf(newpath, len, "%s/%s/%s", c->config_path, c->name, OCI_HOOK_JSON_FILE_NAME); @@ -5276,11 +5282,11 @@ static int set_start_extral_configs(struct lxc_container *c) if (lconf == NULL) { c->lxc_conf = malloc(sizeof(struct lxc_conf)); - lconf = c->lxc_conf; - if (lconf == NULL) { + if (c->lxc_conf == NULL) { fprintf(stderr, "Out of memory\n"); return -1; } + lconf = c->lxc_conf; } if (snprintf(fpath, PATH_MAX, "%s/%s/%s", c->config_path, c->name, START_GENERATE_CONFIG) < 0) { fprintf(stderr, "Sprintf config path failed\n"); diff --git a/src/lxc/path.c b/src/lxc/path.c index c545887..df285f5 100644 --- a/src/lxc/path.c +++ b/src/lxc/path.c @@ -58,7 +58,7 @@ char *preserve_trailing_dot_or_separator(const char *cleanedpath, len = strlen(cleanedpath) + 3; respath = malloc(len); - if (!respath) { + if (respath == NULL) { ERROR("Out of memory"); return NULL; } @@ -90,12 +90,16 @@ bool filepath_split(const char *path, char **dir, char **base) size_t len; len = strlen(path); + if (len >= PATH_MAX) { + ERROR("Invalid path"); + return false; + } i = len - 1; while (i >= 0 && path[i] != '/') i--; *dir = malloc(i + 2); - if (!*dir) { + if (*dir == NULL) { ERROR("Out of memory"); return false; } diff --git a/src/lxc/start.c b/src/lxc/start.c index d6c706e..00d478c 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -2383,7 +2383,7 @@ static int create_start_timeout_thread(struct lxc_conf *conf, unsigned int start } timeout_conf = malloc(sizeof(struct start_timeout_conf)); - if (!timeout_conf) { + if (timeout_conf == NULL) { ERROR("Failed to malloc start timeout conf"); ret = -1; goto out; @@ -2657,7 +2657,7 @@ static struct lxc_handler *lxc_init_clean_handler(char *name, char *lxcpath, str struct lxc_handler *handler; handler = malloc(sizeof(*handler)); - if (!handler) + if (handler == NULL) return NULL; memset(handler, 0, sizeof(*handler)); @@ -2707,7 +2707,7 @@ static struct lxc_handler *lxc_init_pids_handler(char *name, char *lxcpath, stru struct lxc_handler *handler; handler = malloc(sizeof(*handler)); - if (!handler) + if (handler == NULL) return NULL; memset(handler, 0, sizeof(*handler)); diff --git a/src/lxc/terminal.c b/src/lxc/terminal.c index b9b65c6..1fee651 100644 --- a/src/lxc/terminal.c +++ b/src/lxc/terminal.c @@ -1330,7 +1330,7 @@ static int lxc_terminal_set_fifo(struct lxc_terminal *console, const char *in, c } fifo_elem = malloc(sizeof(*fifo_elem)); - if (!fifo_elem) { + if (fifo_elem == NULL) { if (fifofd_in >= 0) close(fifofd_in); if (fifofd_out >= 0) diff --git a/src/lxc/utils.c b/src/lxc/utils.c index e6e8905..656df4a 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -1945,9 +1945,9 @@ static proc_t *lxc_stat2proc(const char *S) *tmp = '\0'; /* replace trailing ')' with NUL */ P = malloc(sizeof(proc_t)); - if (!P) + if (P == NULL) return NULL; - memset(P, 0x00, sizeof(proc_t)); + (void)memset(P, 0x00, sizeof(proc_t)); /* parse these two strings separately, skipping the leading "(". */ num = sscanf(S, "%d (%15c", &P->pid, P->cmd); /* comm[16] in kernel */ -- 1.8.3.1