From ea03ca5af8628a05bd3c8b751a913ef0704be7e0 Mon Sep 17 00:00:00 2001
From: Kyungwook Tak <k.tak@samsung.com>
Date: Thu, 3 Mar 2016 12:08:04 +0900
Subject: [PATCH] Fix unsafe buffer usage

- sprintf
- strcpy

Change-Id: I85716d6daabc149526146dfe375874a7057550a2
Signed-off-by: Kyungwook Tak <k.tak@samsung.com>
---
 src/manager/client/client-common.cpp      |  2 +-
 src/manager/dpl/db/src/sql_connection.cpp |  2 +-
 src/manager/main/socket-manager.cpp       |  2 +-
 tests/test_common.cpp                     | 28 +++++++++++++++++-----------
 tests/test_common.h                       |  1 +
 tests/test_sql.cpp                        | 12 ++++++++++++
 6 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/manager/client/client-common.cpp b/src/manager/client/client-common.cpp
index 5aca5c5..02bbbcf 100644
--- a/src/manager/client/client-common.cpp
+++ b/src/manager/client/client-common.cpp
@@ -103,7 +103,7 @@ int SockRAII::connectWrapper(int sock, const char *interface)
         return CKM_API_ERROR_INPUT_PARAM;
     }
 
-    strcpy(clientAddr.sun_path, interface);
+    strncpy(clientAddr.sun_path, interface, sizeof(clientAddr.sun_path) - 1);
     LogDebug("ClientAddr.sun_path = " << interface);
 
     int retval = TEMP_FAILURE_RETRY(::connect(sock, (struct sockaddr*)&clientAddr, SUN_LEN(&clientAddr)));
diff --git a/src/manager/dpl/db/src/sql_connection.cpp b/src/manager/dpl/db/src/sql_connection.cpp
index 9b053a0..02a12c6 100644
--- a/src/manager/dpl/db/src/sql_connection.cpp
+++ b/src/manager/dpl/db/src/sql_connection.cpp
@@ -668,7 +668,7 @@ RawBuffer rawToHexString(const RawBuffer &raw)
     RawBuffer output;
     for (auto &e: raw) {
         char result[3];
-        snprintf(result, sizeof(result), "%02X", static_cast<unsigned int>(e));
+        snprintf(result, sizeof(result), "%02X", (e & 0xff));
         output.push_back(static_cast<unsigned char>(result[0]));
         output.push_back(static_cast<unsigned char>(result[1]));
     }
diff --git a/src/manager/main/socket-manager.cpp b/src/manager/main/socket-manager.cpp
index a8e884f..0b49378 100644
--- a/src/manager/main/socket-manager.cpp
+++ b/src/manager/main/socket-manager.cpp
@@ -561,7 +561,7 @@ int SocketManager::CreateDomainSocketHelp(
     sockaddr_un serverAddress;
     memset(&serverAddress, 0, sizeof(serverAddress));
     serverAddress.sun_family = AF_UNIX;
-    strcpy(serverAddress.sun_path, desc.serviceHandlerPath.c_str());
+    strncpy(serverAddress.sun_path, desc.serviceHandlerPath.c_str(), sizeof(serverAddress.sun_path) - 1);
     unlink(serverAddress.sun_path);
 
     mode_t originalUmask;
diff --git a/tests/test_common.cpp b/tests/test_common.cpp
index 53e12f0..b96e913 100644
--- a/tests/test_common.cpp
+++ b/tests/test_common.cpp
@@ -26,26 +26,32 @@
 using namespace CKM;
 
 RawBuffer createDefaultPass() {
-    RawBuffer raw;
-    for(unsigned char i =0; i < RAW_PASS_SIZE; i++)
-        raw.push_back(i);
-    return raw;
+    return createPass(0, RAW_PASS_SIZE);
 }
 
-RawBuffer createBigBlob(std::size_t size) {
+RawBuffer createPass(std::size_t from, std::size_t to) {
     RawBuffer raw;
-    for(std::size_t i = 0; i < size; i++) {
+
+    for (std::size_t i = from; i < to; i++)
         raw.push_back(static_cast<unsigned char>(i));
-    }
+
     return raw;
 }
 
+RawBuffer createBigBlob(std::size_t size) {
+    return createPass(0, size);
+}
+
 //raw to hex string conversion from SqlConnection
 std::string rawToHexString(const RawBuffer &raw) {
-    std::string dump(raw.size()*2, '0');
-    for(std::size_t i = 0; i < raw.size(); i++){
-        sprintf(&dump[2*i], "%02x", raw[i]);
+    std::string dump;
+
+    for (auto &e : raw) {
+        char buf[3];
+        snprintf(buf, sizeof(buf), "%02x", (e & 0xff));
+        dump.push_back(buf[0]);
+        dump.push_back(buf[1]);
     }
+
     return dump;
 }
-
diff --git a/tests/test_common.h b/tests/test_common.h
index bf645d2..b9b70bf 100644
--- a/tests/test_common.h
+++ b/tests/test_common.h
@@ -30,6 +30,7 @@
 #endif
 
 CKM::RawBuffer createDefaultPass();
+CKM::RawBuffer createPass(std::size_t from, std::size_t to);
 CKM::RawBuffer createBigBlob(std::size_t size);
 
 const CKM::RawBuffer defaultPass = createDefaultPass();
diff --git a/tests/test_sql.cpp b/tests/test_sql.cpp
index d020fc7..a07329e 100644
--- a/tests/test_sql.cpp
+++ b/tests/test_sql.cpp
@@ -60,6 +60,18 @@ BOOST_AUTO_TEST_CASE(sqlTestConversion){
     BOOST_CHECK(pass_check == pattern);
 }
 
+BOOST_AUTO_TEST_CASE(sqlTestConversionBig){
+    /* 192 ~ 208 in hex */
+    const std::string tmppattern = "c0c1c2c3c4c5c6c7c8c9cacbcccdcecfd0";
+
+    auto pass = createPass(192, 209);
+    BOOST_REQUIRE_MESSAGE(pass.size() == 17, "Password size should be 17");
+
+    auto pass_hex = rawToHexString(pass);
+    BOOST_REQUIRE_MESSAGE(pass_hex.length() == 34, "Hexed password size should be 34");
+    BOOST_CHECK(pass_hex == tmppattern);
+}
+
 BOOST_AUTO_TEST_CASE(sqlTestSetKeyTooShort) {
     using namespace CKM::DB;
     BOOST_CHECK(unlink(encrypt_me_not) == 0 || errno == ENOENT);
-- 
2.7.4