From 9d8d0d2a5564aa54e260ef0a0c8e10a31039b64b Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 17 Jul 2017 14:59:06 +0200 Subject: [PATCH] Fix defects reported by SVACE Change-Id: Ia890a846836d2c7cf9657a889b304ec1e0171ead --- src/manager/client-capi/ckmc-manager.cpp | 27 +++++---- src/manager/client-capi/ckmc-type.cpp | 14 +++-- src/manager/dpl/core/include/dpl/exception.h | 67 ++++++++-------------- .../initial-values/initial-value-loader.cpp | 6 +- src/manager/main/generic-socket-manager.h | 1 + src/manager/main/socket-manager.cpp | 6 +- src/manager/service/file-lock.cpp | 2 +- 7 files changed, 60 insertions(+), 63 deletions(-) diff --git a/src/manager/client-capi/ckmc-manager.cpp b/src/manager/client-capi/ckmc-manager.cpp index 9356a4e..7eb4c9e 100644 --- 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 private_key_uptr( + NULL, ckmc_key_free); + std::unique_ptr 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(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 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 8ac6a2b..d7b212c 100644 --- a/src/manager/client-capi/ckmc-type.cpp +++ b/src/manager/client-capi/ckmc-type.cpp @@ -36,6 +36,7 @@ #include #include #include +#include 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(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 --- a/src/manager/dpl/core/include/dpl/exception.h +++ b/src/manager/dpl/core/include/dpl/exception.h @@ -29,6 +29,7 @@ #include #include #include +#include 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() ? "" : 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() ? "" : 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() ? "" : 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 --- a/src/manager/initial-values/initial-value-loader.cpp +++ b/src/manager/initial-values/initial-value-loader.cpp @@ -21,9 +21,12 @@ */ #include +#include + #include #include #include +#include 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 --- 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 --- 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(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 e7e4032..48ee743 100644 --- a/src/manager/service/file-lock.cpp +++ b/src/manager/service/file-lock.cpp @@ -50,7 +50,7 @@ std::runtime_error io_exception(const Args &... args) 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) throw io_exception("Cannot open lock file. Errno: ", GetErrnoString()); -- 2.7.4