From 48a1957a4b0a23fce5abebe29dccb40fb37ba683 Mon Sep 17 00:00:00 2001 From: HowJMay Date: Tue, 7 Jul 2020 22:30:50 +0800 Subject: [PATCH] fix(coverity): Fix security issues coverity found Read CA files until EOF, and avoid buffer overflow. --- accelerator/config.c | 58 +++++++++++++++++++-------- accelerator/core/serializer/ser_mam.c | 10 ++--- common/debug.c | 6 +-- utils/cache/backend_redis.c | 2 +- utils/cpuinfo.h | 1 + 5 files changed, 52 insertions(+), 25 deletions(-) diff --git a/accelerator/config.c b/accelerator/config.c index cdd8a70c..e6224da2 100644 --- a/accelerator/config.c +++ b/accelerator/config.c @@ -16,7 +16,6 @@ #include "yaml.h" #define CONFIG_LOGGER "config" -#define CA_PEM_LEN 1208 #define DEFAULT_CA_PEM \ "-----BEGIN CERTIFICATE-----\r\n" \ "MIIDQTCCAimgAwIBAgITBmyfz5m/jAo54vB4ikPmljZbyjANBgkqhkiG9w0BAQsF\r\n" \ @@ -62,6 +61,42 @@ static int get_conf_key(char const* const key) { return 0; } +#define CA_BUFFER_SIZE 2048 +static char* read_ca_pem(char* ca_pem_path) { + FILE* file = NULL; + if ((file = fopen(ca_pem_path, "r")) == NULL) { + ta_log_error("%s\n", ta_error_to_string(SC_CONF_FOPEN_ERROR)); + goto done; + } + + char* ca_pem = (char*)malloc(sizeof(char) * (CA_BUFFER_SIZE)); + if (!ca_pem) { + ta_log_error("%s\n", ta_error_to_string(SC_OOM)); + goto done; + } + + char c; + int i = 0, buffer_nums = 1; + while ((c = fgetc(file)) != EOF) { + ca_pem[i++] = c; + if (!(i < CA_BUFFER_SIZE)) { + ca_pem = realloc(ca_pem, sizeof(char) * buffer_nums * CA_BUFFER_SIZE); + buffer_nums++; + } + } + ca_pem[i] = '\0'; + + if (feof(file)) { + ta_log_debug("Read the End Of File.\n"); + } else { + ta_log_error("Read file error\n"); + } + +done: + fclose(file); + return ca_pem; +} + status_t cli_core_set(ta_core_t* const core, int key, char* const value) { if (value == NULL && (key != CACHE && key != PROXY_API && key != QUIET && key != NO_GTTA)) { ta_log_error("%s\n", "SC_NULL"); @@ -71,7 +106,6 @@ status_t cli_core_set(ta_core_t* const core, int key, char* const value) { iota_config_t* const iota_conf = &core->iota_conf; ta_cache_t* const cache = &core->cache; iota_client_service_t* const iota_service = &core->iota_service; - FILE* file = NULL; char* conf_file = core->conf_file; #ifdef DB_ENABLE db_client_service_t* const db_service = &core->db_service; @@ -115,7 +149,7 @@ status_t cli_core_set(ta_core_t* const core, int key, char* const value) { for (char* p = strtok(value, ","); p && idx < MAX_NODE_LIST_ELEMENTS; p = strtok(NULL, ","), idx++) { ta_conf->iota_host_list[idx] = p; } - strncpy(iota_service->http.host, ta_conf->iota_host_list[0], HOST_MAX_LEN); + strncpy(iota_service->http.host, ta_conf->iota_host_list[0], HOST_MAX_LEN - 1); break; case NODE_PORT_CLI: idx = 0; @@ -131,15 +165,7 @@ status_t cli_core_set(ta_core_t* const core, int key, char* const value) { break; case CA_PEM: - if ((file = fopen(value, "r")) == NULL) { - /* The specified configuration file does not exist */ - ta_log_error("%s\n", ta_error_to_string(SC_CONF_FOPEN_ERROR)); - return SC_CONF_FOPEN_ERROR; - } - char* temp_ca_pem = (char*)malloc(sizeof(char) * (CA_PEM_LEN + 1)); - fread(temp_ca_pem, CA_PEM_LEN, 1, file); - iota_service->http.ca_pem = temp_ca_pem; - fclose(file); + iota_service->http.ca_pem = read_ca_pem(value); break; case HEALTH_TRACK_PERIOD: @@ -257,12 +283,12 @@ status_t cli_core_set(ta_core_t* const core, int key, char* const value) { status_t ta_set_iota_client_service(iota_client_service_t* service, char const* host, uint16_t port, char const* const ca_pem) { strncpy(service->http.path, "/", CONTENT_TYPE_MAX_LEN); - strncpy(service->http.content_type, "application/json", CONTENT_TYPE_MAX_LEN); - strncpy(service->http.accept, "application/json", CONTENT_TYPE_MAX_LEN); - strncpy(service->http.host, host, HOST_MAX_LEN); + strncpy(service->http.content_type, "application/json", CONTENT_TYPE_MAX_LEN - 1); + strncpy(service->http.accept, "application/json", CONTENT_TYPE_MAX_LEN - 1); + strncpy(service->http.host, host, HOST_MAX_LEN - 1); service->http.port = port; service->http.api_version = 1; - service->http.ca_pem = ca_pem ? ca_pem : DEFAULT_CA_PEM; + service->http.ca_pem = ca_pem; service->serializer_type = SR_JSON; init_json_serializer(&service->serializer); diff --git a/accelerator/core/serializer/ser_mam.c b/accelerator/core/serializer/ser_mam.c index 2ad353cb..292b7320 100644 --- a/accelerator/core/serializer/ser_mam.c +++ b/accelerator/core/serializer/ser_mam.c @@ -158,7 +158,7 @@ static status_t recv_mam_message_mam_v1_req_deserialize(cJSON const* const json_ if (json_value == NULL) { ret = SC_CCLIENT_JSON_KEY; ta_log_error("%s\n", ta_error_to_string(ret)); - return ret; + goto done; } if (cJSON_IsString(json_value) && (json_value->valuestring != NULL) && (strlen(json_value->valuestring) == NUM_TRYTES_HASH)) { @@ -167,7 +167,7 @@ static status_t recv_mam_message_mam_v1_req_deserialize(cJSON const* const json_ } else { ret = SC_CCLIENT_JSON_PARSE; ta_log_error("%s\n", ta_error_to_string(ret)); - return ret; + goto done; } goto set_data_id; } @@ -177,7 +177,7 @@ static status_t recv_mam_message_mam_v1_req_deserialize(cJSON const* const json_ if (json_value == NULL) { ret = SC_CCLIENT_JSON_KEY; ta_log_error("%s\n", ta_error_to_string(ret)); - return ret; + goto done; } if (cJSON_IsString(json_value) && (json_value->valuestring != NULL && (strlen(json_value->valuestring) == NUM_TRYTES_HASH))) { @@ -186,7 +186,7 @@ static status_t recv_mam_message_mam_v1_req_deserialize(cJSON const* const json_ } else { ret = SC_CCLIENT_JSON_PARSE; ta_log_error("%s\n", ta_error_to_string(ret)); - return ret; + goto done; } } @@ -204,7 +204,7 @@ static status_t recv_mam_message_mam_v1_req_deserialize(cJSON const* const json_ } else { ret = SC_CCLIENT_JSON_PARSE; ta_log_error("%s\n", ta_error_to_string(ret)); - return ret; + goto done; } } diff --git a/common/debug.c b/common/debug.c index 98502dde..bec96b59 100644 --- a/common/debug.c +++ b/common/debug.c @@ -35,7 +35,7 @@ void dump_bundle(bundle_transactions_t *bundle) { void dump_transaction_obj(iota_transaction_t *tx_obj) { field_mask_t old_mask = {}; memcpy(&old_mask, &tx_obj->loaded_columns_mask, sizeof(field_mask_t)); - memset(&tx_obj->loaded_columns_mask, 0xFFFFF, sizeof(field_mask_t)); + memset(&tx_obj->loaded_columns_mask, 0xFF, sizeof(field_mask_t)); ta_log_debug("==========Transaction Object==========\n"); // address @@ -92,9 +92,9 @@ bool transaction_cmp(iota_transaction_t *tx1, iota_transaction_t *tx2) { bool equal = true; field_mask_t old_mask1 = {}, old_mask2 = {}; memcpy(&old_mask1, &tx1->loaded_columns_mask, sizeof(field_mask_t)); - memset(&tx1->loaded_columns_mask, 0xFFFFF, sizeof(field_mask_t)); + memset(&tx1->loaded_columns_mask, 0xFF, sizeof(field_mask_t)); memcpy(&old_mask2, &tx2->loaded_columns_mask, sizeof(field_mask_t)); - memset(&tx2->loaded_columns_mask, 0xFFFFF, sizeof(field_mask_t)); + memset(&tx2->loaded_columns_mask, 0xFF, sizeof(field_mask_t)); uint64_t result = 0; // address diff --git a/utils/cache/backend_redis.c b/utils/cache/backend_redis.c index 54f57115..3c8b5cc6 100644 --- a/utils/cache/backend_redis.c +++ b/utils/cache/backend_redis.c @@ -268,7 +268,7 @@ bool cache_init(pthread_rwlock_t** rwlock, bool input_state, const char* host, i cache.conn = (connection_private*)malloc(sizeof(connection_private)); CONN(cache)->rc = redisConnect(host, port); if (!CONN(cache)->rc || CONN(cache)->rc->err) { - ta_log_error("Failed to initialize redis: %s\n", CONN(cache)->rc->err); + ta_log_error("Failed to initialize redis: %d\n", CONN(cache)->rc->err); return false; } diff --git a/utils/cpuinfo.h b/utils/cpuinfo.h index dd4f84c1..292a279a 100644 --- a/utils/cpuinfo.h +++ b/utils/cpuinfo.h @@ -56,6 +56,7 @@ static inline int get_nthds_per_phys_proc() { if (fgets(nthd, sizeof(nthd), fd) == NULL) return -1; nthread = (int)strtol(nthd, NULL, 10); if (errno == ERANGE || nthread == 0) { + pclose(fd); return -1; }