Rework get_data_name for buffer overflows 71/161671/6
authorIgor Kotrasinski <i.kotrasinsk@partner.samsung.com>
Fri, 24 Nov 2017 13:48:30 +0000 (14:48 +0100)
committerIgor Kotrasinski <i.kotrasinsk@partner.samsung.com>
Tue, 28 Nov 2017 14:06:57 +0000 (15:06 +0100)
Change-Id: I8142537acfeb81d1a00bbc4cdc3222f83b493ae8
Signed-off-by: Igor Kotrasinski <i.kotrasinsk@partner.samsung.com>
ssflib/dep/swdss/include/secure_file.h
ssflib/dep/swdss/source/secure_file.cpp

index 183a1c4..835b02f 100644 (file)
@@ -255,7 +255,7 @@ private:
        int decrypt_data();
 
 #ifdef _SECOS_SIM_
-       void get_data_name(char* data_name, bool is_dir);
+       int get_data_name(char* data_name, int maxlen, bool is_dir);
 #endif
 
 private:
index 4bf131f..0ab45f6 100644 (file)
@@ -1069,7 +1069,10 @@ int secure_file::write_persistent_store(unsigned char* data,
     unsigned int size) {
 #ifdef _SECOS_SIM_
        char filename[MAX_FILENAME_LEN] = {0};
-       get_data_name(filename, false);
+       if (get_data_name(filename, MAX_FILENAME_LEN, false) == -1) {
+               OsaFree(data);
+               return SS_RET_INTERNAL_ERROR;
+       }
        int iRet = file_op::write_file(filename, data, size);
        OsaFree(data);
        return iRet;
@@ -1119,7 +1122,8 @@ int secure_file::read_persistent_store(unsigned char** buffer,
     unsigned int& ret_size) {
 #ifdef _SECOS_SIM_
        char filename[MAX_FILENAME_LEN] = {0};
-       get_data_name(filename, false);
+       if (get_data_name(filename, MAX_FILENAME_LEN, false) == -1)
+               return SS_RET_INTERNAL_ERROR;
        return file_op::read_file(filename, buffer, ret_size);
 #else
 
@@ -1166,7 +1170,9 @@ int secure_file::read_persistent_store(unsigned char** buffer,
 int secure_file::remove_persistent_store(bool is_dir) {
 #ifdef _SECOS_SIM_
        char filename[MAX_FILENAME_LEN] = {0};
-       get_data_name(filename, is_dir);
+       if (get_data_name(filename, MAX_FILENAME_LEN, is_dir) == -1)
+               return SS_RET_INTERNAL_ERROR;
+
        int iret = SS_RET_SUCCESS;
        if (is_dir) {
                iret = file_op::remove_folder(filename);
@@ -1421,7 +1427,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);
+               OsaFree(data);
                return ret;
        }
 
@@ -1696,27 +1702,29 @@ 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) {
+int secure_file::get_data_name(char* data_name, int maxlen, bool is_dir) {
        char* ptr = data_name;
-       strncpy(ptr, SWD_SS_ROOT, MAX_FILENAME_LEN - (ptr-data_name));
-       ptr += strlen(SWD_SS_ROOT);
+       int remain = maxlen;
 
-       // first 4 bytes for directory.
-       //byte_to_hex(ptr, (uint8_t*)m_full_path, 4);
-       strncpy(ptr, m_full_path, SS_CRED_LEN);
-       ptr[SS_CRED_LEN] = '\0';
+       strncpy(ptr, SWD_SS_ROOT, remain);
+       if (ptr[remain - 1] != '\0')
+               return -1;
+       ptr += strlen(ptr);
+       remain = maxlen - (ptr - data_name);
 
        if (is_dir) {
-               return;
+               if (remain > SS_CRED_LEN) {
+                       strncpy(ptr, m_full_path, SS_CRED_LEN);
+                       ptr[SS_CRED_LEN] = '\0';
+               } else {
+                       return -1;
+               }
+       } else {
+               strncpy(ptr, m_full_path, remain);
+               if (ptr[remain - 1] != '\0')
+                       return -1;
        }
-
-       // next 8 bytes for filename
-       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,)
-       //byte_to_hex(ptr, (uint8_t*)&m_full_path[4], 8);
+       return 0;
 }
 #endif