Fix coverity warnings

Signed-off-by: Raul Metsma <[email protected]>

Author: metsma

.github/workflows/build.yml

@@ -149,9 +149,10 @@ jobs:
         java-version: '17'
     - name: Build
       run: |
+        $swig = (Get-Item "$env:LOCALAPPDATA\Microsoft\WinGet\Links\swig.exe").Target
         cmake -A ${{ matrix.platform }} -S . -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo `
           "-DCMAKE_TOOLCHAIN_FILE=${{ env.VCPKG_ROOT }}/scripts/buildsystems/vcpkg.cmake" `
-          -DSWIG_EXECUTABLE=(Get-Item $env:LOCALAPPDATA\Microsoft\WinGet\Links\swig.exe).Target `
+          "-DSWIG_EXECUTABLE=$swig" `
           -DVCPKG_TARGET_TRIPLET=${{ env.VCPKG_DEFAULT_TRIPLET }} `
           -DVCPKG_MANIFEST_FEATURES=tests `
           -DCMAKE_INSTALL_LIBDIR=bin

cdoc/CDoc2Reader.cpp

@@ -298,7 +298,11 @@ CDoc2Reader::getFMK(std::vector<uint8_t>& fmk, unsigned int lock_idx)
                 LOG_ERROR("Cannot fetch share {}", i);
                 return result;
             }
-            Crypto::xor_data(kek, kek, share.share);
+            if (Crypto::xor_data(kek, kek, share.share) != libcdoc::OK) {
+                setLastError("Failed to derive kek");
+                LOG_ERROR("Failed to derive kek");
+                return libcdoc::CRYPTO_ERROR;
+            }
         }
         LOG_INFO("Fetched all shares");
     } else {
@@ -651,7 +655,7 @@ CDoc2Reader::CDoc2Reader(libcdoc::DataSource *src, bool take_ownership)
                     std::string url = cshare->server_base_url()->str();
                     std::string str = url + "," + id;
                     LOG_DBG("Keyshare: {}", str);
-                    strs.push_back(str);
+                    strs.push_back(std::move(str));
                 }
                 std::string urls = join(strs, ";");
                 LOG_DBG("Keyshare urls: {}", urls);

cdoc/Crypto.cpp

@@ -521,9 +521,11 @@ Crypto::EVP_PKEY_ptr
 Crypto::genECKey(EVP_PKEY *params)
 {
 	EVP_PKEY *key = nullptr;
-    auto ctx = make_unique_ptr<EVP_PKEY_CTX_free>(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");
+    if(auto ctx = make_unique_ptr<EVP_PKEY_CTX_free>(EVP_PKEY_CTX_new(params, nullptr));
+        ctx &&
+        !SSL_FAILED(EVP_PKEY_keygen_init(ctx.get()), "EVP_PKEY_keygen_init") &&
+        !SSL_FAILED(EVP_PKEY_keygen(ctx.get(), &key), "EVP_PKEY_keygen"))
+        return EVP_PKEY_ptr(nullptr, EVP_PKEY_free);
     return EVP_PKEY_ptr(key, EVP_PKEY_free);
 }
 

cdoc/KeyShares.cpp

@@ -65,15 +65,17 @@ namespace libcdoc {
 /* Helper for JWT signing */
 struct JWTSigner {
     Signer *parent;
-    result_t *result;
+    result_t *result {};
 
     JWTSigner(Signer *_parent) : parent(_parent) {}
     std::string sign(const std::string& data, std::error_code& ec) const {
         LOG_DBG("Sign JWT: {}", data);
         std::vector<uint8_t> digest(32);
         SHA256((uint8_t *) data.c_str(), data.size(), digest.data());
         std::vector<uint8_t> dst;
-        *result = parent->signDigest(dst, digest);
+        auto rv = parent->signDigest(dst, digest);
+        if (result)
+            *result = rv;
         return std::string((const char *) dst.data(), dst.size());
     }
     void verify(const std::string& data, const std::string& signature, std::error_code& ec) const {};
@@ -164,9 +166,8 @@ Signer::generateTickets(std::vector<std::string>& dst, std::vector<ShareData>& s
     // Create list of individual disclosures
     std::vector<Disclosure> disclosures;
     for (auto share : shares) {
-        Disclosure d({}, share.getURL());
+        Disclosure &d = disclosures.emplace_back(std::string{}, share.getURL());
         LOG_DBG("Disclosure for {}: {}", share.base_url, d.json);
-        disclosures.push_back(d);
     }
     // Create disclosure of the whole list
     Disclosure aud("aud", disclosures);

cdoc/NetworkBackend.cpp

@@ -483,7 +483,7 @@ libcdoc::NetworkBackend::fetchShare(ShareInfo& share, const std::string& url, co
     std::vector<uint8_t> shareval = fromBase64(share64);
     shareval.resize(32);
     LOG_DBG("Share: {}", toHex(shareval));
-    share = {shareval, recipient};
+    share = {std::move(shareval), std::move(recipient)};
     return OK;
 }
 

cdoc/Utils.h

@@ -72,7 +72,7 @@ split(const std::string &s, char delim = ':') {
 }
 
 static std::string
-join(const std::vector<std::string> parts, const std::string_view sep)
+join(const std::vector<std::string> &parts, const std::string_view sep)
 {
 	std::string result;
 	for (auto& part : parts) {

cdoc/XmlWriter.cpp

@@ -20,6 +20,8 @@
 
 #include "Io.h"
 
+#include "utils/memory.h"
+
 #include <libxml/xmlwriter.h>
 
 using namespace libcdoc;
@@ -28,8 +30,8 @@ typedef const xmlChar *pcxmlChar;
 
 struct XMLWriter::Private
 {
-	xmlTextWriterPtr w = xmlNewTextWriter(
-		xmlOutputBufferCreateIO(xmlOutputWriteCallback, xmlOutputCloseCallback, this, nullptr));
+    unique_ptr_t<xmlFreeTextWriter> w = make_unique_ptr<xmlFreeTextWriter>(xmlNewTextWriter(
+        xmlOutputBufferCreateIO(xmlOutputWriteCallback, xmlOutputCloseCallback, this, nullptr)));
 	std::map<std::string, int> nsmap;
 
 	libcdoc::DataConsumer* dst = nullptr;
@@ -57,7 +59,7 @@ XMLWriter::XMLWriter(libcdoc::DataConsumer* dst)
 	: d(new Private)
 {
 	d->dst = dst;
-	xmlTextWriterStartDocument(d->w, nullptr, "UTF-8", nullptr);
+    xmlTextWriterStartDocument(d->w.get(), nullptr, "UTF-8", nullptr);
 }
 
 XMLWriter::XMLWriter(const std::string& path)
@@ -74,8 +76,7 @@ XMLWriter::XMLWriter(std::vector<uint8_t>& vec)
 
 XMLWriter::~XMLWriter()
 {
-	xmlTextWriterEndDocument(d->w);
-	xmlFreeTextWriter(d->w);
+    xmlTextWriterEndDocument(d->w.get());
 	if(d->dst && d->dst_owned) delete d->dst;
 	delete d;
 }
@@ -89,12 +90,12 @@ int64_t XMLWriter::writeStartElement(const NS &ns, const std::string &name, cons
 		pos->second++;
 	else
 		pos = d->nsmap.insert({ns.prefix, 1}).first;
-    if(xmlTextWriterStartElementNS(d->w, ns.prefix.empty() ? nullptr : pcxmlChar(ns.prefix.c_str()),
+    if(xmlTextWriterStartElementNS(d->w.get(), ns.prefix.empty() ? nullptr : pcxmlChar(ns.prefix.c_str()),
         pcxmlChar(name.c_str()), pos->second > 1 ? nullptr : pcxmlChar(ns.ns.c_str())) == -1)
         return IO_ERROR;
 	for(auto i = attr.cbegin(), end = attr.cend(); i != end; ++i)
 	{
-        if(xmlTextWriterWriteAttribute(d->w, pcxmlChar(i->first.c_str()), pcxmlChar(i->second.c_str())) == -1)
+        if(xmlTextWriterWriteAttribute(d->w.get(), pcxmlChar(i->first.c_str()), pcxmlChar(i->second.c_str())) == -1)
             return IO_ERROR;
 	}
     return OK;
@@ -104,7 +105,7 @@ int64_t XMLWriter::writeEndElement(const NS &ns)
 {
     if(!d->w)
         return WRONG_ARGUMENTS;
-    if(xmlTextWriterEndElement(d->w) == -1)
+    if(xmlTextWriterEndElement(d->w.get()) == -1)
         return IO_ERROR;
     if(std::map<std::string, int>::iterator pos = d->nsmap.find(ns.prefix);
         pos != d->nsmap.cend())
@@ -134,7 +135,7 @@ int64_t XMLWriter::writeBase64Element(const NS &ns, const std::string &name, con
 {
     if(auto rv = writeStartElement(ns, name, attr); rv != OK)
         return rv;
-    if(xmlTextWriterWriteBase64(d->w, reinterpret_cast<const char*>(data.data()), 0, data.size()) == -1)
+    if(xmlTextWriterWriteBase64(d->w.get(), reinterpret_cast<const char*>(data.data()), 0, data.size()) == -1)
         return IO_ERROR;
     return writeEndElement(ns);
 }
@@ -143,7 +144,7 @@ int64_t XMLWriter::writeTextElement(const NS &ns, const std::string &name, const
 {
     if(auto rv = writeStartElement(ns, name, attr); rv != OK)
         return rv;
-    if(xmlTextWriterWriteString(d->w, pcxmlChar(data.c_str())) == -1)
+    if(xmlTextWriterWriteString(d->w.get(), pcxmlChar(data.c_str())) == -1)
         return IO_ERROR;
     return writeEndElement(ns);
 }

cdoc/utils/memory.h

@@ -42,6 +42,9 @@ struct free_argument<R (*)(T *)>
 template <class T>
 using unique_free_t = std::unique_ptr<T, void(*)(T*)>;
 
+template <auto D>
+using unique_ptr_t = std::unique_ptr<typename free_argument<decltype(D)>::type, free_deleter<D>>;
+
 template<class T, typename D>
 [[nodiscard]]
 constexpr std::unique_ptr<T, D> make_unique_ptr(T *p, D d) noexcept
@@ -60,8 +63,7 @@ template<auto D>
 [[nodiscard]]
 constexpr auto make_unique_ptr(nullptr_t) noexcept
 {
-    using T = typename free_argument<decltype(D)>::type;
-    return std::unique_ptr<T, free_deleter<D>>(nullptr);
+    return unique_ptr_t<D>(nullptr);
 }
 
 template<auto D, class P>