From: Igor Kotrasinski <i.kotrasinsk@partner.samsung.com>
Date: Fri, 24 Nov 2017 13:48:30 +0000 (+0100)
Subject: Rework get_data_name for buffer overflows
X-Git-Tag: submit/tizen/20171205.070457~5
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b10d277f3586fd119bb06dfc000fc5854d8e1715;p=platform%2Fcore%2Fsecurity%2Ftef-simulator.git

Rework get_data_name for buffer overflows

Change-Id: I8142537acfeb81d1a00bbc4cdc3222f83b493ae8
Signed-off-by: Igor Kotrasinski <i.kotrasinsk@partner.samsung.com>
---

diff --git a/ssflib/dep/swdss/include/secure_file.h b/ssflib/dep/swdss/include/secure_file.h
index 183a1c4..835b02f 100644
--- a/ssflib/dep/swdss/include/secure_file.h
+++ b/ssflib/dep/swdss/include/secure_file.h
@@ -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:
diff --git a/ssflib/dep/swdss/source/secure_file.cpp b/ssflib/dep/swdss/source/secure_file.cpp
index 4bf131f..0ab45f6 100644
--- a/ssflib/dep/swdss/source/secure_file.cpp
+++ b/ssflib/dep/swdss/source/secure_file.cpp
@@ -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