From d80c65d2673e22ecf9942ec219d14b98d3eb8da6 Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Sun, 16 Feb 2025 15:39:15 +0200 Subject: [PATCH] Fix memory leaks and add helper memory.h util Signed-off-by: Raul Metsma --- cdoc/CDoc1Reader.cpp | 2 -- cdoc/CDoc1Writer.cpp | 22 ++++++------- cdoc/CDoc2Reader.cpp | 2 +- cdoc/CMakeLists.txt | 9 +++--- cdoc/Crypto.cpp | 41 ++++++++++++------------ cdoc/Crypto.h | 5 +-- cdoc/utils/memory.h | 75 ++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 113 insertions(+), 43 deletions(-) create mode 100644 cdoc/utils/memory.h diff --git a/cdoc/CDoc1Reader.cpp b/cdoc/CDoc1Reader.cpp index 133fc46..d6ff13e 100644 --- a/cdoc/CDoc1Reader.cpp +++ b/cdoc/CDoc1Reader.cpp @@ -38,8 +38,6 @@ static const std::string MIME_ZLIB = "http://www.isi.edu/in-noes/iana/assignment static const std::string MIME_DDOC = "http://www.sk.ee/DigiDoc/v1.3.0/digidoc.xsd"; static const std::string MIME_DDOC_OLD = "http://www.sk.ee/DigiDoc/1.3.0/digidoc.xsd"; -#define SCOPE(TYPE, VAR, DATA) std::unique_ptr VAR(DATA, TYPE##_free) - static const std::set SUPPORTED_METHODS = { libcdoc::Crypto::AES128CBC_MTH, libcdoc::Crypto::AES192CBC_MTH, libcdoc::Crypto::AES256CBC_MTH, libcdoc::Crypto::AES128GCM_MTH, libcdoc::Crypto::AES192GCM_MTH, libcdoc::Crypto::AES256GCM_MTH }; diff --git a/cdoc/CDoc1Writer.cpp b/cdoc/CDoc1Writer.cpp index 32faf0f..fd76724 100644 --- a/cdoc/CDoc1Writer.cpp +++ b/cdoc/CDoc1Writer.cpp @@ -23,6 +23,7 @@ #include "XmlWriter.h" #include "CDoc1Writer.h" #include "ILogger.h" +#include "utils/memory.h" #if defined(_WIN32) || defined(_WIN64) #include @@ -32,9 +33,6 @@ #include - -#define SCOPE(TYPE, VAR, DATA) std::unique_ptr VAR(DATA, TYPE##_free) - using namespace libcdoc; struct FileEntry { @@ -79,7 +77,7 @@ CDoc1Writer::~CDoc1Writer() bool CDoc1Writer::Private::writeRecipient(XMLWriter *xmlw, const std::vector &recipient, const libcdoc::Crypto::Key& transportKey) { - SCOPE(X509, peerCert, libcdoc::Crypto::toX509(recipient)); + auto peerCert = make_unique_ptr(libcdoc::Crypto::toX509(recipient)); if(!peerCert) return false; std::string cn = [&]{ @@ -102,12 +100,12 @@ bool CDoc1Writer::Private::writeRecipient(XMLWriter *xmlw, const std::vectorwriteElement(Private::DENC, "EncryptedKey", {{"Recipient", cn}}, [&]{ std::vector encryptedData; - SCOPE(EVP_PKEY, peerPKey, X509_get_pubkey(peerCert.get())); - switch(EVP_PKEY_base_id(peerPKey.get())) + auto *peerPKey = X509_get0_pubkey(peerCert.get()); + switch(EVP_PKEY_base_id(peerPKey)) { case EVP_PKEY_RSA: { - SCOPE(RSA, rsa, EVP_PKEY_get1_RSA(peerPKey.get())); + auto rsa = make_unique_ptr(EVP_PKEY_get1_RSA(peerPKey)); encryptedData.resize(size_t(RSA_size(rsa.get()))); RSA_public_encrypt(int(transportKey.key.size()), transportKey.key.data(), encryptedData.data(), rsa.get(), RSA_PKCS1_PADDING); @@ -121,13 +119,13 @@ bool CDoc1Writer::Private::writeRecipient(XMLWriter *xmlw, const std::vector(EC_KEY_new_by_curve_name(curveName)); EC_KEY_generate_key(priv.get()); - SCOPE(EVP_PKEY, pkey, EVP_PKEY_new()); + auto pkey = make_unique_ptr(EVP_PKEY_new()); EVP_PKEY_set1_EC_KEY(pkey.get(), priv.get()); - std::vector sharedSecret = libcdoc::Crypto::deriveSharedSecret(pkey.get(), peerPKey.get()); + std::vector sharedSecret = libcdoc::Crypto::deriveSharedSecret(pkey.get(), peerPKey); std::string oid(50, 0); oid.resize(size_t(OBJ_obj2txt(&oid[0], int(oid.size()), OBJ_nid2obj(curveName), 1))); diff --git a/cdoc/CDoc2Reader.cpp b/cdoc/CDoc2Reader.cpp index 02483db..476029e 100644 --- a/cdoc/CDoc2Reader.cpp +++ b/cdoc/CDoc2Reader.cpp @@ -285,7 +285,7 @@ CDoc2Reader::beginDecryption(const std::vector& fmk) priv->tgs = std::make_unique(priv->_src, false, 16); libcdoc::CipherSource *csrc = new libcdoc::CipherSource(priv->tgs.get(), false, priv->cipher.get()); - priv->zsrc = std::make_unique(csrc, false); + priv->zsrc = std::make_unique(csrc, true); priv->tar = std::make_unique(priv->zsrc.get(), false); return libcdoc::OK; diff --git a/cdoc/CMakeLists.txt b/cdoc/CMakeLists.txt index 5fabdd3..347c461 100644 --- a/cdoc/CMakeLists.txt +++ b/cdoc/CMakeLists.txt @@ -57,6 +57,7 @@ add_library(cdoc ToolConf.h CDoc2.h Wrapper.h + utils/memory.h ) set_target_properties(cdoc PROPERTIES VERSION ${PROJECT_VERSION} @@ -82,7 +83,7 @@ target_link_libraries(cdoc PRIVATE OpenSSL::SSL LibXml2::LibXml2 ZLIB::ZLIB - #cdoc_ver + $<$:cdoc_ver> $ #$ ) @@ -94,6 +95,8 @@ if(BUILD_TOOLS) set_target_properties(cdoc-tool PROPERTIES INSTALL_RPATH $<$:@executable_path/../lib> ) + install(TARGETS cdoc-tool DESTINATION ${CMAKE_INSTALL_BINDIR}) + #install( FILES ${CMAKE_CURRENT_BINARY_DIR}/cdoc-tool.1 DESTINATION ${CMAKE_INSTALL_MANDIR}/man1 ) endif() if(WIN32) @@ -230,8 +233,4 @@ if(NOT ANDROID) ) endif() -if(BUILD_TOOLS) - install(TARGETS cdoc-tool DESTINATION ${CMAKE_INSTALL_BINDIR}) -endif() -#install( FILES ${CMAKE_CURRENT_BINARY_DIR}/cdoc-tool.1 DESTINATION ${CMAKE_INSTALL_MANDIR}/man1 ) #install( FILES ${CMAKE_CURRENT_BINARY_DIR}/libcdoc.pc DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig ) diff --git a/cdoc/Crypto.cpp b/cdoc/Crypto.cpp index 22cb11e..2aabece 100644 --- a/cdoc/Crypto.cpp +++ b/cdoc/Crypto.cpp @@ -36,8 +36,6 @@ #include -#define SCOPE(TYPE, VAR, DATA) std::unique_ptr VAR(DATA, TYPE##_free) - using namespace libcdoc; const std::string Crypto::SHA256_MTH = "http://www.w3.org/2001/04/xmlenc#sha256"; @@ -218,7 +216,7 @@ std::vector Crypto::concatKDF(const std::string &hashAlg, uint32_t keyD std::vector Crypto::encrypt(const std::string &method, const Key &key, const std::vector &data) { const EVP_CIPHER *c = cipher(method); - SCOPE(EVP_CIPHER_CTX, ctx, EVP_CIPHER_CTX_new()); + auto ctx = make_unique_ptr(EVP_CIPHER_CTX_new()); if (SSL_FAILED(EVP_CipherInit(ctx.get(), c, key.key.data(), key.iv.data(), 1), "EVP_CipherInit")) return {}; @@ -249,7 +247,7 @@ std::vector Crypto::encrypt(const std::string &method, const Key &key, std::vector Crypto::encrypt(EVP_PKEY *pub, int padding, const std::vector &data) { - SCOPE(EVP_PKEY_CTX, ctx, EVP_PKEY_CTX_new(pub, nullptr)); + auto ctx = make_unique_ptr(EVP_PKEY_CTX_new(pub, nullptr)); size_t size = 0; if (SSL_FAILED(EVP_PKEY_encrypt_init(ctx.get()), "EVP_PKEY_encrypt_init") || SSL_FAILED(EVP_PKEY_CTX_set_rsa_padding(ctx.get(), padding), "EVP_PKEY_CTX_set_rsa_padding") || @@ -277,7 +275,7 @@ std::vector Crypto::decrypt(const std::string &method, const std::vecto LOG_DBG("iv {}", toHex(iv)); LOG_DBG("transport {}", toHex(key)); - SCOPE(EVP_CIPHER_CTX, ctx, EVP_CIPHER_CTX_new()); + auto ctx = make_unique_ptr(EVP_CIPHER_CTX_new()); if (!ctx) { LOG_SSL_ERROR("EVP_CIPHER_CTX_new"); @@ -322,7 +320,7 @@ std::vector Crypto::decodeBase64(const uint8_t *data) return result; } result.resize(strlen((const char*)data)); - SCOPE(EVP_ENCODE_CTX, ctx, EVP_ENCODE_CTX_new()); + auto ctx = make_unique_ptr(EVP_ENCODE_CTX_new()); if (!ctx) { LOG_SSL_ERROR("EVP_ENCODE_CTX_new"); @@ -350,7 +348,7 @@ std::vector Crypto::deriveSharedSecret(EVP_PKEY *pkey, EVP_PKEY *peerPK { std::vector sharedSecret; size_t sharedSecretLen = 0; - SCOPE(EVP_PKEY_CTX, ctx, EVP_PKEY_CTX_new(pkey, nullptr)); + auto ctx = make_unique_ptr(EVP_PKEY_CTX_new(pkey, nullptr)); if (!ctx) { LOG_SSL_ERROR("EVP_PKEY_CTX_new"); @@ -396,7 +394,7 @@ uint32_t Crypto::keySize(const std::string &algo) std::vector Crypto::hkdf(const std::vector &key, const std::vector &salt, const std::vector &info, int len, int mode) { - SCOPE(EVP_PKEY_CTX, ctx, EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, nullptr)); + auto ctx = make_unique_ptr(EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, nullptr)); if (!ctx) { LOG_SSL_ERROR("EVP_PKEY_CTX_new_id"); @@ -432,26 +430,28 @@ Crypto::extract(const std::vector &key, const std::vector &sal std::vector Crypto::sign_hmac(const std::vector &key, const std::vector &data) { - EVP_PKEY *pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, nullptr, key.data(), int(key.size())); + std::vector sig; + auto pkey = make_unique_ptr(EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, nullptr, key.data(), int(key.size()))); if (!pkey) { LOG_SSL_ERROR("EVP_PKEY_new_mac_key"); - return {}; + return sig; } - size_t req = 0; - SCOPE(EVP_MD_CTX, ctx, EVP_MD_CTX_new()); + auto ctx = make_unique_ptr(EVP_MD_CTX_new()); if (!ctx) { LOG_SSL_ERROR("EVP_MD_CTX_new"); - return {}; + return sig; } - if (SSL_FAILED(EVP_DigestSignInit(ctx.get(), nullptr, EVP_sha256(), nullptr, pkey), "EVP_DigestSignInit") || + + size_t req = 0; + if (SSL_FAILED(EVP_DigestSignInit(ctx.get(), nullptr, EVP_sha256(), nullptr, pkey.get()), "EVP_DigestSignInit") || SSL_FAILED(EVP_DigestSignUpdate(ctx.get(), data.data(), data.size()), "EVP_DigestSignUpdate") || SSL_FAILED(EVP_DigestSignFinal(ctx.get(), nullptr, &req), "EVP_DigestSignFinal")) - return {}; + return sig; - std::vector sig(int(req), 0); + sig.resize(req); if(SSL_FAILED(EVP_DigestSignFinal(ctx.get(), sig.data(), &req), "EVP_DigestSignFinal")) sig.clear(); return sig; @@ -482,7 +482,7 @@ Crypto::EVP_PKEY_ptr Crypto::fromECPublicKeyDer(const std::vector &der, int curveName) { EVP_PKEY *params = nullptr; - SCOPE(EVP_PKEY_CTX, ctx, EVP_PKEY_CTX_new_id(EVP_PKEY_EC, nullptr)); + auto ctx = make_unique_ptr(EVP_PKEY_CTX_new_id(EVP_PKEY_EC, nullptr)); if (!ctx) LOG_SSL_ERROR("EVP_PKEY_CTX_new_id"); @@ -516,13 +516,12 @@ Crypto::EVP_PKEY_ptr Crypto::genECKey(EVP_PKEY *params) { EVP_PKEY *key = nullptr; - SCOPE(EVP_PKEY_CTX, ctx, EVP_PKEY_CTX_new(params, nullptr)); - SCOPE(EVP_PKEY, result, nullptr); + auto ctx = make_unique_ptr(EVP_PKEY_CTX_new(params, nullptr)); if(ctx && !SSL_FAILED(EVP_PKEY_keygen_init(ctx.get()), "EVP_PKEY_keygen_init") && !SSL_FAILED(EVP_PKEY_keygen(ctx.get(), &key), "EVP_PKEY_keygen")) - result.reset(key); - return result; + {} + return EVP_PKEY_ptr(key, EVP_PKEY_free); } std::vector diff --git a/cdoc/Crypto.h b/cdoc/Crypto.h index b607ae4..f73f0c9 100644 --- a/cdoc/Crypto.h +++ b/cdoc/Crypto.h @@ -18,7 +18,8 @@ #pragma once -#include +#include "utils/memory.h" + #include #include @@ -36,7 +37,7 @@ namespace libcdoc { class Crypto { public: - typedef typename std::unique_ptr EVP_PKEY_ptr; + using EVP_PKEY_ptr = unique_free_t; struct Cipher { struct evp_cipher_ctx_st *ctx; diff --git a/cdoc/utils/memory.h b/cdoc/utils/memory.h new file mode 100644 index 0000000..60d6532 --- /dev/null +++ b/cdoc/utils/memory.h @@ -0,0 +1,75 @@ +/* + * libcdoc + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +#pragma once + +#include + +namespace libcdoc { + +template +struct free_deleter +{ + template + void operator()(T *p) const noexcept + { + D(p); + } +}; + +template struct free_argument; +template +struct free_argument +{ + using type = T; +}; + +template +using unique_free_t = std::unique_ptr; + +template +[[nodiscard]] +constexpr std::unique_ptr make_unique_ptr(T *p, D d) noexcept +{ + return {p, std::forward(d)}; +} + +template +[[nodiscard]] +constexpr auto make_unique_ptr(T *p) noexcept +{ + return std::unique_ptr>(p); +} + +template +[[nodiscard]] +constexpr auto make_unique_ptr(nullptr_t) noexcept +{ + using T = typename free_argument::type; + return std::unique_ptr>(nullptr); +} + +template +[[nodiscard]] +constexpr auto make_unique_cast(P *p) noexcept +{ + using T = typename free_argument::type; + return make_unique_ptr(static_cast(p)); +} + +} \ No newline at end of file