Use Github Actions CodeQL tests

IB-7528

Signed-off-by: Raul Metsma <raul@metsma.ee>

.github/workflows/build.yml

@@ -24,7 +24,7 @@ jobs:
       run: |
         brew update
         brew install doxygen boost xsd || brew link --overwrite xsd
-        brew unlink xerces-c python@3.10
+        brew unlink xerces-c python@3.10 python@3.11
     - name: Cache
       uses: actions/cache@v3
       id: cache
@@ -257,3 +257,48 @@ jobs:
           --form version=master \
           --form description="Github Actions CI build" \
           https://scan.coverity.com/builds?project=$PROJECTNAME
+  codeql:
+    name: Run CodeQL tests
+    if: github.repository == 'open-eid/libdigidocpp'
+    runs-on: ubuntu-20.04
+    permissions:
+      security-events: write
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v3
+      with:
+        submodules: recursive
+    - name: Install dependencies
+      run: sudo apt update -qq && sudo apt install --no-install-recommends -y cmake vim-common xsdcxx libxml-security-c-dev zlib1g-dev curl ca-certificates
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v2
+      with:
+        languages: cpp
+        queries: +security-and-quality
+    - name: Build
+      run: |
+        cmake -B build -S . \
+          -DSWIG_EXECUTABLE=NOTFOUND \
+          -DBoost_INCLUDE_DIR=NOTFOUND \
+          -DDOXYGEN_EXECUTABLE=NOTFOUND \
+          -DBUILD_TOOLS=NO
+        cmake --build build
+    - name: Perform CodeQL Analysis
+      uses: github/codeql-action/analyze@v2
+      with:
+        upload: False
+        output: sarif-results
+    - name: Filter results
+      uses: advanced-security/filter-sarif@develop
+      with:
+        patterns: |
+          -src/json.hpp
+          -src/minizip/*
+          -build/src/xml/*
+          -**:cpp/poorly-documented-function
+        input: sarif-results/cpp.sarif
+        output: sarif-results/cpp.sarif
+    - name: Upload results
+      uses: github/codeql-action/upload-sarif@v2
+      with:
+        sarif_file: sarif-results/cpp.sarif

README.md

@@ -9,7 +9,6 @@
 ## Building
 [![Build Status](https://github.com/open-eid/libdigidocpp/workflows/CI/badge.svg?branch=master)](https://github.com/open-eid/libdigidocpp/actions)
 [![Coverity Scan Build Status](https://scan.coverity.com/projects/727/badge.svg)](https://scan.coverity.com/projects/727)
-[![LGTM alerts](https://img.shields.io/lgtm/alerts/g/open-eid/libdigidocpp.svg?logo=lgtm&logoWidth=18)](https://lgtm.com/projects/g/open-eid/libdigidocpp/alerts/)
 
 ### Ubuntu, Fedora
 

src/ASiContainer.cpp

@@ -37,7 +37,7 @@ using namespace digidoc;
 using namespace digidoc::util;
 using namespace std;
 
-constexpr unsigned long MAX_MEM_FILE = 500*1024*1024;
+constexpr unsigned long MAX_MEM_FILE = 500UL*1024UL*1024UL;
 
 class ASiContainer::Private
 {
@@ -48,11 +48,11 @@ class ASiContainer::Private
     map<string, ZipSerialize::Properties> properties;
 };
 
-const string ASiContainer::ASICE_EXTENSION = "asice";
-const string ASiContainer::ASICE_EXTENSION_ABBR = "sce";
-const string ASiContainer::ASICS_EXTENSION = "asics";
-const string ASiContainer::ASICS_EXTENSION_ABBR = "scs";
-const string ASiContainer::BDOC_EXTENSION = "bdoc";
+const string_view ASiContainer::ASICE_EXTENSION = "asice";
+const string_view ASiContainer::ASICE_EXTENSION_ABBR = "sce";
+const string_view ASiContainer::ASICS_EXTENSION = "asics";
+const string_view ASiContainer::ASICS_EXTENSION_ABBR = "scs";
+const string_view ASiContainer::BDOC_EXTENSION = "bdoc";
 
 const string ASiContainer::MIMETYPE_ASIC_E = "application/vnd.etsi.asic-e+zip";
 const string ASiContainer::MIMETYPE_ASIC_S = "application/vnd.etsi.asic-s+zip";
@@ -91,8 +91,8 @@ unique_ptr<ZipSerialize> ASiContainer::load(const string &path, bool mimetypeReq
     if(mimetypeRequired && list[0] != "mimetype")
         THROW("required mimetype not found");
 
-    // ETSI TS 102 918: mimetype has to be the first in the archive;
-    if(list[0] == "mimetype")
+    // ETSI TS 102 918: mimetype has to be the first in the archive
+    if(list.front() == "mimetype")
     {
         stringstream data;
         z->extract(list.front(), data);
@@ -178,20 +178,19 @@ void ASiContainer::addDataFile(const string &path, const string &mediaType)
     if(!File::fileExists(path))
         THROW("Document file '%s' does not exist.", path.c_str());
 
-    ZipSerialize::Properties prop = { appInfo(), File::modifiedTime(path), File::fileSize(path) };
-    zproperty(File::fileName(path), prop);
+    ZipSerialize::Properties prop { appInfo(), File::modifiedTime(path), File::fileSize(path) };
+    bool useTempFile = prop.size > MAX_MEM_FILE;
+    zproperty(File::fileName(path), move(prop));
     unique_ptr<istream> is;
-    if(prop.size > MAX_MEM_FILE)
+    if(useTempFile)
     {
         is = make_unique<ifstream>(File::encodeName(path).c_str(), ifstream::binary);
     }
     else
     {
-        ifstream file(File::encodeName(path).c_str(), ifstream::binary);
         stringstream *data = new stringstream;
-        if(file)
+        if(ifstream file(File::encodeName(path).c_str(), ifstream::binary); file)
             *data << file.rdbuf();
-        file.close();
         is.reset(data);
     }
     addDataFilePrivate(move(is), fileName, mediaType);
@@ -235,7 +234,7 @@ void ASiContainer::removeDataFile(unsigned int id)
     if(!d->signatures.empty())
         THROW("Can not remove document from container which has signatures, remove all signatures before removing document.");
     if(id >= d->documents.size())
-        THROW("Incorrect document id %u, there are only %lu documents in container.", id, (unsigned long)dataFiles().size());
+        THROW("Incorrect document id %u, there are only %zu documents in container.", id, dataFiles().size());
     vector<DataFile*>::const_iterator it = (d->documents.cbegin() + id);
     delete *it;
     d->documents.erase(it);
@@ -256,16 +255,15 @@ Signature* ASiContainer::addSignature(unique_ptr<Signature> &&signature)
 void ASiContainer::removeSignature(unsigned int id)
 {
     if(id >= d->signatures.size())
-        THROW("Incorrect signature id %u, there are only %lu signatures in container.", id, (unsigned long)d->signatures.size());
+        THROW("Incorrect signature id %u, there are only %zu signatures in container.", id, d->signatures.size());
     vector<Signature*>::const_iterator it = (d->signatures.cbegin() + id);
     delete *it;
     d->signatures.erase(it);
 }
 
 void ASiContainer::deleteSignature(Signature* s)
 {
-    vector<Signature*>::const_iterator i = find(d->signatures.cbegin(), d->signatures.cend(), s);
-    if(i != d->signatures.cend())
+    if(auto i = find(d->signatures.cbegin(), d->signatures.cend(), s); i != d->signatures.cend())
         d->signatures.erase(i);
     delete s;
 }
@@ -288,9 +286,9 @@ ZipSerialize::Properties ASiContainer::zproperty(const string &file) const
     return d->properties[file] = { appInfo(), time(nullptr), 0 };
 }
 
-void ASiContainer::zproperty(const string &file, const ZipSerialize::Properties &prop)
+void ASiContainer::zproperty(const string &file, ZipSerialize::Properties &&prop)
 {
-    d->properties[file] = prop;
+    d->properties[file] = move(prop);
 }
 
 /**
@@ -310,7 +308,7 @@ string ASiContainer::readMimetype(istream &is)
        (bom[0] == 0xEF && bom[1] == 0xFF))
         THROW("Mimetype file must be UTF-8 format.");
     // does not contain UTF-8 BOM reset pos
-    if(!(bom[0] == 0xEF && bom[1] == 0xBB && bom[2] == 0xBF))
+    if(bom[0] != 0xEF || bom[1] != 0xBB || bom[2] != 0xBF)
         is.seekg(0, ios::beg);
 
     string text;

src/ASiContainer.h

@@ -38,11 +38,11 @@ namespace digidoc
     class ASiContainer: public Container
     {
       public:
-          static const std::string ASICE_EXTENSION;
-          static const std::string ASICE_EXTENSION_ABBR;
-          static const std::string ASICS_EXTENSION;
-          static const std::string ASICS_EXTENSION_ABBR;
-          static const std::string BDOC_EXTENSION;
+          static const std::string_view ASICE_EXTENSION;
+          static const std::string_view ASICE_EXTENSION_ABBR;
+          static const std::string_view ASICS_EXTENSION;
+          static const std::string_view ASICS_EXTENSION_ABBR;
+          static const std::string_view BDOC_EXTENSION;
 
           static const std::string MIMETYPE_ASIC_E;
           static const std::string MIMETYPE_ASIC_S;
@@ -70,7 +70,7 @@ namespace digidoc
           void zpath(const std::string &file);
           std::string zpath() const;
           ZipSerialize::Properties zproperty(const std::string &file) const;
-          void zproperty(const std::string &file, const ZipSerialize::Properties &prop);
+          void zproperty(const std::string &file, ZipSerialize::Properties &&prop);
 
           static std::string readMimetype(std::istream &path);
 

src/Conf.cpp

@@ -174,11 +174,7 @@ bool Conf::TSLAutoUpdate() const { return true; }
  */
 string Conf::TSLCache() const
 {
-#ifdef _WIN32
-    return File::env("APPDATA") + "\\digidocpp\\tsl\\";
-#else
-    return File::env("HOME") + "/.digidocpp/tsl/";
-#endif
+    return File::path(File::digidocppPath(), "tsl");
 }
 
 /**

src/SiVaContainer.cpp

@@ -202,7 +202,7 @@ SiVaContainer::SiVaContainer(const string &path, const string &ext, bool useHash
     {
         Exception e(EXCEPTION_PARAMS("Signature validation"));
         for(const json &error: result["requestErrors"])
-            EXCEPTION_ADD(e, error.value<string>("message", {}).c_str());
+            EXCEPTION_ADD(e, "%s", error.value<string>("message", {}).c_str());
         throw e;
     }
 
@@ -260,13 +260,13 @@ SiVaContainer::SiVaContainer(const string &path, const string &ext, bool useHash
         {
             string message = error["content"];
             if(message.find("Bad digest for DataFile") == 0 && useHashCode)
-                THROW(message.c_str());
-            s->_exceptions.emplace_back(EXCEPTION_PARAMS(message.c_str()));
+                THROW("%s", message.c_str());
+            s->_exceptions.emplace_back(EXCEPTION_PARAMS("%s", message.c_str()));
         }
         for(const json &warning: signature.value<json>("warnings", {}))
         {
             string message = warning["content"];
-            Exception ex(EXCEPTION_PARAMS(message.c_str()));
+            Exception ex(EXCEPTION_PARAMS("%s", message.c_str()));
             if(message == "X509IssuerName has none or invalid namespace: null" ||
                 message == "X509SerialNumber has none or invalid namespace: null")
                 ex.setCode(Exception::IssuerNameSpaceWarning);

src/XmlConf.cpp

@@ -127,16 +127,7 @@ XmlConf::Private::Private(const string &path, string schema)
         }
     }
 
-#ifdef _WIN32
-    USER_CONF_LOC = File::env("APPDATA");
-    if (!USER_CONF_LOC.empty())
-        USER_CONF_LOC += "\\digidocpp\\digidocpp.conf";
-#else
-    USER_CONF_LOC = File::env("HOME");
-    if (!USER_CONF_LOC.empty())
-        USER_CONF_LOC += "/.digidocpp/digidocpp.conf";
-#endif
-
+    USER_CONF_LOC = File::path(File::digidocppPath(), "digidocpp.conf");
     if(path.empty())
     {
         init(File::confPath() + "/digidocpp.conf", true);

src/crypto/TSL.cpp

@@ -193,7 +193,7 @@ vector<TSL::Service> TSL::services() const
 
 void TSL::debugException(const digidoc::Exception &e)
 {
-    Log::out(Log::DebugType, e.file().c_str(), e.line(), e.msg().c_str());
+    Log::out(Log::DebugType, e.file().c_str(), e.line(), "%s", e.msg().c_str());
     for(const Exception &ex: e.causes())
         debugException(ex);
 }
@@ -562,7 +562,6 @@ void TSL::validate(const X509Cert &cert) const
         XSECProvider prov;
         auto deleteSig = [&](DSIGSignature *s) { prov.releaseSignature(s); };
         unique_ptr<DSIGSignature, decltype(deleteSig)> sig(prov.newSignatureFromDOM(tsl->_node()->getOwnerDocument()), deleteSig);
-        //sig->setKeyInfoResolver(new XSECKeyInfoResolverDefault);
         sig->setSigningKey(OpenSSLCryptoX509(cert.handle()).clonePublicKey());
         sig->registerIdAttributeName((const XMLCh*)u"ID");
         sig->setIdByAttributeName(true);

src/crypto/X509CertStore.cpp

@@ -203,13 +203,16 @@ int X509CertStore::validate(int ok, X509_STORE_CTX *ctx, const Type &type)
     case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY:
     case X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE:
     case X509_V_ERR_CERT_UNTRUSTED:
+        break;
+    default: return ok;
+    }
+
+    X509 *x509 = X509_STORE_CTX_get0_cert(ctx);
+    for(const TSL::Service &s: *instance()->d)
     {
-        X509 *x509 = X509_STORE_CTX_get0_cert(ctx);
-        for(const TSL::Service &s: *instance()->d)
-        {
-            if(type.find(s.type) == type.cend())
-                continue;
-            if(none_of(s.certs.cbegin(), s.certs.cend(), [&](const X509Cert &issuer){
+        if(type.find(s.type) == type.cend())
+            continue;
+        if(none_of(s.certs.cbegin(), s.certs.cend(), [&](const X509Cert &issuer){
                 if(issuer == x509)
                     return true;
                 if(X509_check_issued(issuer.handle(), x509) != X509_V_OK)
@@ -220,25 +223,22 @@ int X509CertStore::validate(int ok, X509_STORE_CTX *ctx, const Type &type)
                 OpenSSLException(EXCEPTION_PARAMS("ignore")); //Clear errors
                 return false;
             }))
-                continue;
-            X509_STORE_CTX_set_ex_data(ctx, 0, const_cast<TSL::Validity*>(&s.validity[0]));
-            X509_VERIFY_PARAM *param = X509_STORE_CTX_get0_param(ctx);
-            if(!(X509_VERIFY_PARAM_get_flags(param) & X509_V_FLAG_USE_CHECK_TIME) || s.validity.empty())
-                return 1;
-            for(const TSL::Validity &v: s.validity)
+            continue;
+        X509_STORE_CTX_set_ex_data(ctx, 0, const_cast<TSL::Validity*>(&s.validity[0]));
+        X509_VERIFY_PARAM *param = X509_STORE_CTX_get0_param(ctx);
+        if(!(X509_VERIFY_PARAM_get_flags(param) & X509_V_FLAG_USE_CHECK_TIME) || s.validity.empty())
+            return 1;
+        for(const TSL::Validity &v: s.validity)
+        {
+            if(X509_VERIFY_PARAM_get_time(param) >= v.start &&
+                (v.end == 0 || X509_VERIFY_PARAM_get_time(param) <= v.end))
             {
-                if(X509_VERIFY_PARAM_get_time(param) >= v.start &&
-                    (v.end == 0 || X509_VERIFY_PARAM_get_time(param) <= v.end))
-                {
-                    X509_STORE_CTX_set_ex_data(ctx, 0, const_cast<TSL::Validity*>(&v));
-                    return 1;
-                }
+                X509_STORE_CTX_set_ex_data(ctx, 0, const_cast<TSL::Validity*>(&v));
+                return 1;
             }
         }
-        return ok;
-    }
-    default: return ok;
     }
+    return ok;
 }
 
 /**
@@ -258,13 +258,9 @@ bool X509CertStore::verify(const X509Cert &cert, bool noqscd) const
     {
         int err = X509_STORE_CTX_get_error(csc.get());
         OpenSSLException e(EXCEPTION_PARAMS("%s", X509_verify_cert_error_string(err)));
-        switch(err)
-        {
-        case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY:
+        if(err == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY)
             e.setCode(Exception::CertificateIssuerMissing);
-            throw e;
-        default: throw e;
-        }
+        throw e;
     }
 
     if(noqscd)