From 43c72359c928ca41862809d1c2942220ea492a62 Mon Sep 17 00:00:00 2001 From: Igor Kotrasinski Date: Mon, 23 Oct 2017 09:55:27 +0200 Subject: [PATCH] SVACE: HEAP_LEAK, correct size in strncpy Change-Id: Ib53c48c7f6ab9aaa1e00ea407bbcad329aa38a3a --- ssflib/dep/cryptocore/source/base/cc_bignum.c | 1 + ssflib/dep/swdss/source/file_op.cpp | 8 ++------ ssflib/dep/swdss/source/secure_file.cpp | 26 +++++++++++++++----------- ssflib/dep/swdss/source/ss_api.cpp | 6 ++---- ssflib/src/ssf_storage.cpp | 5 ++--- 5 files changed, 22 insertions(+), 24 deletions(-) diff --git a/ssflib/dep/cryptocore/source/base/cc_bignum.c b/ssflib/dep/cryptocore/source/base/cc_bignum.c index b1561ab..5dc6043 100644 --- a/ssflib/dep/cryptocore/source/base/cc_bignum.c +++ b/ssflib/dep/cryptocore/source/base/cc_bignum.c @@ -3107,6 +3107,7 @@ cc_u8 * SDRM_BN2STRFOUR(cc_u32 *numberBits, SDRM_BIG_NUM *BN_Src) num = SDRM_BN_Init(BN_Src->Size); if( num == NULL)//fix prevent cid = 89093 by guoxing.xu { + free(num); free(strDestTemp); SDRM_BN_FREE(d); return NULL; diff --git a/ssflib/dep/swdss/source/file_op.cpp b/ssflib/dep/swdss/source/file_op.cpp index 38d135d..eafe680 100644 --- a/ssflib/dep/swdss/source/file_op.cpp +++ b/ssflib/dep/swdss/source/file_op.cpp @@ -230,14 +230,10 @@ bool file_op::is_file_exists(const char* file) { } void file_op::get_base_path(const char* filename, char* base_path) { - char tmp[256] = {0}; - memcpy(tmp, filename, strlen(filename)); - for (int i = strlen(filename) - 1; i >= 0; --i) { - tmp[i] = 0; - if ('/' == filename[i]) { - memcpy(base_path, tmp, strlen(tmp)); + memcpy(base_path, filename, i); + base_path[i] = '\0'; break; } } diff --git a/ssflib/dep/swdss/source/secure_file.cpp b/ssflib/dep/swdss/source/secure_file.cpp index 3fdc18f..9a828f1 100644 --- a/ssflib/dep/swdss/source/secure_file.cpp +++ b/ssflib/dep/swdss/source/secure_file.cpp @@ -29,6 +29,8 @@ #endif +#define MAX_FILENAME_LEN 256 + // this is RNG SEED for mask static const CBT_UINT32 RNG_SEED = 0xa3e59cf2; // this is RNG SEED for mask @@ -383,9 +385,8 @@ int secure_file::initialize(const ss_credential_s * cred, const char* data_name, m_data_name[0] = '\0'; if (NULL != data_name) { - int data_name_len = strlen(data_name); - memcpy(m_data_name, data_name, data_name_len); - m_data_name[data_name_len] = '\0'; + strncpy(m_data_name, data_name, sizeof(m_data_name)); + m_data_name[sizeof(m_data_name)-1] = '\0'; } m_options = options; @@ -942,7 +943,7 @@ int secure_file::serialize_data(unsigned char** buffer, unsigned int& ret_size) { #ifdef _SECOS_SIM_ *buffer = (unsigned char*)OsaMalloc(m_write_data_size); - if (NULL == buffer) { + if (NULL == *buffer) { //SLOGE("fail to alloc memory for data."); return SS_RET_MALLOC_FAILED; } @@ -1094,7 +1095,7 @@ int secure_file::write_temp_store(unsigned char* data, unsigned int size) { int secure_file::write_persistent_store(unsigned char* data, unsigned int size) { #ifdef _SECOS_SIM_ - char filename[256] = {0}; + char filename[MAX_FILENAME_LEN] = {0}; get_data_name(filename, false); int iRet = file_op::write_file(filename, data, size); OsaFree(data); @@ -1144,7 +1145,7 @@ int secure_file::read_temp_store(unsigned char** buffer, int secure_file::read_persistent_store(unsigned char** buffer, unsigned int& ret_size) { #ifdef _SECOS_SIM_ - char filename[256] = {0}; + char filename[MAX_FILENAME_LEN] = {0}; get_data_name(filename, false); return file_op::read_file(filename, buffer, ret_size); #else @@ -1191,7 +1192,7 @@ int secure_file::read_persistent_store(unsigned char** buffer, int secure_file::remove_persistent_store(bool is_dir) { #ifdef _SECOS_SIM_ - char filename[256] = {0}; + char filename[MAX_FILENAME_LEN] = {0}; get_data_name(filename, is_dir); int iret = SS_RET_SUCCESS; if (is_dir) { @@ -1447,6 +1448,7 @@ int secure_file::write(unsigned char* buffer, unsigned int buf_size, unsigned char* data = NULL; unsigned int size = 0; if (SS_RET_SUCCESS != (ret = serialize_data(&data, size))) { + free(data); return ret; } @@ -1722,20 +1724,22 @@ int secure_file::clear_storage() { #ifdef _SECOS_SIM_ #define SS_CRED_LEN 36 void secure_file::get_data_name(char* data_name, bool is_dir) { - uint8_t* ptr = (uint8_t*)data_name; - memcpy(ptr, SWD_SS_ROOT, strlen(SWD_SS_ROOT)); + char* ptr = data_name; + strncpy(ptr, SWD_SS_ROOT, MAX_FILENAME_LEN - (ptr-data_name)); ptr += strlen(SWD_SS_ROOT); // first 4 bytes for directory. //byte_to_hex(ptr, (uint8_t*)m_full_path, 4); - memcpy(ptr, m_full_path, SS_CRED_LEN); + strncpy(ptr, m_full_path, SS_CRED_LEN); + ptr[SS_CRED_LEN] = '\0'; if (is_dir) { return; } // next 8 bytes for filename - memcpy(ptr, m_full_path, strlen(m_full_path)); + strncpy(ptr, m_full_path, MAX_FILENAME_LEN - (ptr-data_name)); + data_name[MAX_FILENAME_LEN - 1] = '\0'; //memset(ptr, '/', 1); //ptr += 1; //memcpy(ptr,m_full_path+SS_CRED_LEN,) diff --git a/ssflib/dep/swdss/source/ss_api.cpp b/ssflib/dep/swdss/source/ss_api.cpp index 5619d11..b6ada9f 100644 --- a/ssflib/dep/swdss/source/ss_api.cpp +++ b/ssflib/dep/swdss/source/ss_api.cpp @@ -34,16 +34,14 @@ int ss_set_credential(ss_credential_s * cred, const char* uuid, return SS_RET_INVALID_PARAM; } - int uuid_size = strlen(uuid); - int mn_size = strlen(module_name); + int uuid_size = strnlen(uuid, SS_MAX_UUID_LEN) + 1; + int mn_size = strnlen(module_name, SS_MAX_MODULE_NAME_LEN) + 1; if (uuid_size > SS_MAX_UUID_LEN || mn_size > SS_MAX_MODULE_NAME_LEN) { SLOGE("[%s] length of uuid or module name error.\n", __FUNCTION__); return SS_RET_INVALID_PARAM; } - memset(cred->uuid, '\0', SS_MAX_UUID_LEN); - memset(cred->module_name, '\0', SS_MAX_MODULE_NAME_LEN); strncpy(cred->uuid, uuid, uuid_size); strncpy(cred->module_name, module_name, mn_size); diff --git a/ssflib/src/ssf_storage.cpp b/ssflib/src/ssf_storage.cpp index 33a494e..cfe01da 100644 --- a/ssflib/src/ssf_storage.cpp +++ b/ssflib/src/ssf_storage.cpp @@ -790,9 +790,8 @@ int init_po_info_file(po_info_file* pi_file) { char uuid[64] = {0}; convert_TA_UUID(uuid, tmp_uuid); ss_set_credential(&pi_file->cred, uuid, PO_INTERNAL_MODULE_NAME, 1, 0); - uint32_t fn_sz = strlen(PI_FILE_NAME); - memcpy(pi_file->filename, PI_FILE_NAME, fn_sz); - pi_file->filename[fn_sz] = '\0'; + strncpy(pi_file->filename, PI_FILE_NAME, sizeof(pi_file->filename)); + pi_file->filename[sizeof(pi_file->filename)-1] = '\0'; pi_file->b_inited = 1; return 0; } -- 2.7.4