Fix defects reported by SVACE

Change-Id: Ia890a846836d2c7cf9657a889b304ec1e0171ead

diff --git a/src/manager/client-capi/ckmc-manager.cpp b/src/manager/client-capi/ckmc-manager.cpp
index 4702170..2aa3c48 100644 (file)
--- a/src/manager/client-capi/ckmc-manager.cpp
+++ b/src/manager/client-capi/ckmc-manager.cpp
@@ -409,6 +409,10 @@ int ckmc_get_pkcs12(const char *alias, const char *key_password,
        int ret;
        CKM::PKCS12ShPtr pkcs;
        auto mgr = CKM::Manager::create();
+       std::unique_ptr<ckmc_key_s, decltype(&ckmc_key_free)> private_key_uptr(
+                       NULL, ckmc_key_free);
+       std::unique_ptr<ckmc_cert_s, decltype(&ckmc_cert_free)> cert_uptr(
+                       NULL, ckmc_cert_free);
 
        if ((ret = mgr->getPKCS12(alias, _tostring(key_password),
                                                          _tostring(cert_password), pkcs)) != CKM_API_SUCCESS)
@@ -417,10 +421,10 @@ int ckmc_get_pkcs12(const char *alias, const char *key_password,
        if (!pkcs)
                return CKMC_ERROR_BAD_RESPONSE;
 
-       ckmc_key_s *private_key = nullptr;
        auto pkcsKey = pkcs->getKey();
 
        if (pkcsKey) {
+               ckmc_key_s *private_key = nullptr;
                auto buffer = pkcsKey->getDER();
                ckmc_key_type_e keyType = static_cast<ckmc_key_type_e>(pkcsKey->getType());
                ret = ckmc_key_new(buffer.data(), buffer.size(), keyType, nullptr,
@@ -428,29 +432,32 @@ int ckmc_get_pkcs12(const char *alias, const char *key_password,
 
                if (ret != CKMC_ERROR_NONE)
                        return ret;
+
+               private_key_uptr.reset(private_key);
        }
 
-       ckmc_cert_s *cert = nullptr;
        auto pkcsCert = pkcs->getCertificate();
 
        if (pkcsCert) {
+               ckmc_cert_s *cert = nullptr;
                CKM::RawBuffer buffer = pkcsCert->getDER();
                ret = ckmc_cert_new(buffer.data(), buffer.size(), CKMC_FORM_DER, &cert);
 
                if (ret != CKMC_ERROR_NONE) {
-                       ckmc_key_free(private_key);
                        return ret;
                }
+               cert_uptr.reset(cert);
        }
 
-       auto ca_cert_list = _toNewCkmCertList(pkcs->getCaCertificateShPtrVector());
-
-       ret = ckmc_pkcs12_new(private_key, cert, ca_cert_list, pkcs12);
+       std::unique_ptr<ckmc_cert_list_s, decltype(&ckmc_cert_list_free)> cert_list_uptr(
+                       _toNewCkmCertList(pkcs->getCaCertificateShPtrVector()),
+                       ckmc_cert_list_free);
 
-       if (ret != CKMC_ERROR_NONE) {
-               ckmc_key_free(private_key);
-               ckmc_cert_free(cert);
-               ckmc_cert_list_all_free(ca_cert_list);
+       ret = ckmc_pkcs12_new(private_key_uptr.get(), cert_uptr.get(), cert_list_uptr.get(), pkcs12);
+       if (ret == CKMC_ERROR_NONE) {
+               private_key_uptr.release();
+               cert_uptr.release();
+               cert_list_uptr.release();
        }
 
        return ret;

diff --git a/src/manager/client-capi/ckmc-type.cpp b/src/manager/client-capi/ckmc-type.cpp
index 1820aa9..926cbd4 100644 (file)
--- a/src/manager/client-capi/ckmc-type.cpp
+++ b/src/manager/client-capi/ckmc-type.cpp
@@ -36,6 +36,7 @@
 #include <fstream>
 #include <crypto-init.h>
 #include <dpl/log/log.h>
+#include <dpl/errno_string.h>
 
 namespace {
 
@@ -119,16 +120,12 @@ int ckmc_key_new(unsigned char *raw_key, size_t key_size,
        pkey->key_type = key_type;
 
        if (password != NULL) {
-               pkey->password = reinterpret_cast<char *>(malloc(strlen(password) + 1));
-
+               pkey->password = strdup(password);
                if (pkey->password == NULL) {
                        free(pkey->raw_key);
                        free(pkey);
                        return CKMC_ERROR_OUT_OF_MEMORY;
                }
-
-               memset(pkey->password, 0, strlen(password) + 1);
-               strncpy(pkey->password, password, strlen(password));
        } else {
                pkey->password = NULL;
        }
@@ -241,7 +238,12 @@ int ckmc_load_cert_from_file(const char *file_path, ckmc_cert_s **cert)
        X509 *pcert = NULL;
 
        if (!(pcert = d2i_X509_fp(fp, NULL))) {
-               fseek(fp, 0, SEEK_SET);
+               if (-1 == fseek(fp, 0, SEEK_SET)) {
+                       LogError("fseek() failed: " <<  CKM::GetErrnoString(errno));
+                       fclose(fp);
+                       return CKMC_ERROR_UNKNOWN;
+               }
+
                pcert = PEM_read_X509(fp, NULL, NULL, NULL);
        }
 

diff --git a/src/manager/dpl/core/include/dpl/exception.h b/src/manager/dpl/core/include/dpl/exception.h
index 538076b..efaafee 100644 (file)
--- a/src/manager/dpl/core/include/dpl/exception.h
+++ b/src/manager/dpl/core/include/dpl/exception.h
@@ -29,6 +29,7 @@
 #include <cstdlib>
 #include <sstream>
 #include <symbol-visibility.h>
+#include <iostream>
 
 namespace CKM {
 COMMON_API void LogUnhandledException(const std::string &str);
@@ -78,6 +79,25 @@ private:
                }
        }
 
+       void InternalDump(std::ostream& os) const
+       {
+               if (m_reason != NULL)
+                       m_reason->InternalDump(os);
+
+               const char *file = strchr(m_path.c_str(), '/');
+
+               if (file == NULL)
+                       file = m_path.c_str();
+               else
+                       ++file;
+
+               os << "\033[0;36m" << "[" << file << ":" << m_line << "]" <<
+                         "\033[m " << m_function << "() " <<
+                         "\033[4;35m" << m_className <<
+                         "\033[m: " << (m_message.empty() ? "<EMPTY>" : m_message) <<
+                         "\033[m" << "\n";
+       }
+
        Exception *m_reason;
        std::string m_path;
        std::string m_function;
@@ -200,53 +220,14 @@ public:
 
        void Dump() const
        {
-               // Show reason first
-               if (m_reason != NULL)
-                       m_reason->Dump();
-
-               // Afterward, dump exception
-               const char *file = strchr(m_path.c_str(), '/');
-
-               if (file == NULL)
-                       file = m_path.c_str();
-               else
-                       ++file;
-
-               printf("\033[0;36m[%s:%i]\033[m %s() \033[4;35m%s\033[m: %s\033[m\n",
-                          file, m_line,
-                          m_function.c_str(),
-                          m_className.c_str(),
-                          m_message.empty() ? "<EMPTY>" : m_message.c_str());
+               InternalDump(std::cout);
        }
 
        std::string DumpToString() const
        {
-               std::string ret;
-
-               if (m_reason != NULL)
-                       ret = m_reason->DumpToString();
-
-               const char *file = strchr(m_path.c_str(), '/');
-
-               if (file == NULL)
-                       file = m_path.c_str();
-               else
-                       ++file;
-
-               char buf[1024];
-               snprintf(buf,
-                                sizeof(buf),
-                                "\033[0;36m[%s:%i]\033[m %s() \033[4;35m%s\033[m: %s\033[m\n",
-                                file,
-                                m_line,
-                                m_function.c_str(),
-                                m_className.c_str(),
-                                m_message.empty() ? "<EMPTY>" : m_message.c_str());
-
-               buf[sizeof(buf) - 1] = '\n';
-               ret += buf;
-
-               return ret;
+               std::ostringstream oss;
+               InternalDump(oss);
+               return oss.str();
        }
 
        Exception *GetReason() const

diff --git a/src/manager/initial-values/initial-value-loader.cpp b/src/manager/initial-values/initial-value-loader.cpp
index 4cdfb2d..d2c9408 100644 (file)
--- a/src/manager/initial-values/initial-value-loader.cpp
+++ b/src/manager/initial-values/initial-value-loader.cpp
@@ -21,9 +21,12 @@
  */
 #include <initial-value-loader.h>
 
+#include <unistd.h>
+
 #include <ckm-logic.h>
 #include <for-each-file.h>
 #include <InitialValuesFile.h>
+#include <dpl/errno_string.h>
 
 namespace {
 const char *const INIT_VALUES_XSD          = RO_DATA_DIR "/initial_values.xsd";
@@ -64,7 +67,8 @@ void LoadFiles(CKMLogic &logic)
                                                 rc);
                        }
 
-                       unlink(file.c_str());
+                       if (-1 == unlink(file.c_str()))
+                               LogError("Unlink() failed for " << file << ":" << CKM::GetErrnoString(errno));
                }
        } catch (...) {
                LogError("The implementation of exception handling in xml parser is broken!");

diff --git a/src/manager/main/generic-socket-manager.h b/src/manager/main/generic-socket-manager.h
index bc06bac..90c9156 100644 (file)
--- a/src/manager/main/generic-socket-manager.h
+++ b/src/manager/main/generic-socket-manager.h
@@ -137,6 +137,7 @@ protected:
 
 struct GenericSocketManager {
        virtual void MainLoop() = 0;
+       virtual void MainLoopStop() = 0;
        virtual void RegisterSocketService(GenericSocketService *ptr) = 0;
        virtual void CynaraSocket(int oldFd, int newFd, bool isRW) = 0;
        virtual void Close(ConnectionID connectionID) = 0;

diff --git a/src/manager/main/socket-manager.cpp b/src/manager/main/socket-manager.cpp
index 6a4a45d..ee5a0bf 100644 (file)
--- a/src/manager/main/socket-manager.cpp
+++ b/src/manager/main/socket-manager.cpp
@@ -131,7 +131,7 @@ struct SignalService : public GenericSocketService {
 
                if (siginfo->ssi_signo == SIGTERM) {
                        LogInfo("Got signal: SIGTERM");
-                       dynamic_cast<SocketManager *>(m_serviceManager)->MainLoopStop();
+                       m_serviceManager->MainLoopStop();
                        return;
                }
 
@@ -589,7 +589,9 @@ int SocketManager::CreateDomainSocketHelp(
        serverAddress.sun_family = AF_UNIX;
        strncpy(serverAddress.sun_path, desc.serviceHandlerPath.c_str(),
                        sizeof(serverAddress.sun_path) - 1);
-       unlink(serverAddress.sun_path);
+       if (-1 == unlink(serverAddress.sun_path))
+               LogError("Unlink failed for " << serverAddress.sun_path << ": " <<
+                                CKM::GetErrnoString(errno));
 
        mode_t originalUmask;
        originalUmask = umask(0);

diff --git a/src/manager/service/file-lock.cpp b/src/manager/service/file-lock.cpp
index 1c5f107..ff037f4 100644 (file)
--- a/src/manager/service/file-lock.cpp
+++ b/src/manager/service/file-lock.cpp
@@ -39,7 +39,7 @@ namespace CKM {
 FileLock::FileLock(const char *const file)
 {
        // Open lock file
-       m_lockFd = TEMP_FAILURE_RETRY(creat(file, 0644));
+       m_lockFd = TEMP_FAILURE_RETRY(creat(file, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH));
 
        if (m_lockFd == -1)
                ThrowErr(Exc::FileSystemFailed,