Skip to content

Commit

Permalink
Fix Coverity warnings
Browse files Browse the repository at this point in the history 
IB-7930

Signed-off-by: Raul Metsma <raul@metsma.ee>
  • Loading branch information
@metsma
metsma committed Aug 8, 2024
1 parent f6bfa60 commit b62ada8
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 45 deletions.
2 changes: 1 addition & 1 deletion src/Conf.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ ConfV4* ConfV4::instance() { return dynamic_cast<ConfV4*>(Conf::instance()); }
vector<X509Cert> ConfV4::verifyServiceCerts() const
{
if(X509Cert cert = verifyServiceCert())
return { cert };
return { std::move(cert) };
return {};
}

Expand Down
1 change: 0 additions & 1 deletion src/Container.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ void digidoc::initialize(const string &appInfo, const string &userAgent, initCal
LIBXML_TEST_VERSION
xmlLineNumbersDefaultValue = 1;
xmlLoadExtDtdDefaultValue = XML_DETECT_IDS | XML_COMPLETE_ATTRS;
xmlSubstituteEntitiesDefault(1);
xmlIndentTreeOutput = 1;
if(xmlSecInit() < 0)
THROW("Error during initialisation of xmlsec.");
Expand Down
5 changes: 3 additions & 2 deletions src/XMLDocument.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ struct XMLDocument: public unique_xml_t<decltype(xmlFreeDoc)>, public XMLNode
}

XMLDocument(std::string_view path, const XMLName &n = {}) noexcept
: XMLDocument(path.empty() ? nullptr : xmlParseFile(path.data()), n)
: XMLDocument(path.empty() ? nullptr : xmlReadFile(path.data(), nullptr, XML_PARSE_NOENT|XML_PARSE_NONET), n)

Check failure

Code scanning / CodeQL

XML external entity expansion Critical

This
XML parser
Loading
is not configured to prevent an XML external entity (XXE) attack.
{}

static XMLDocument openStream(std::istream &is, const XMLName &name = {}, bool hugeFile = false)
Expand All @@ -334,8 +334,9 @@ struct XMLDocument: public unique_xml_t<decltype(xmlFreeDoc)>, public XMLNode
return is->good() || is->eof() ? int(is->gcount()) : -1;
}, nullptr, &is, XML_CHAR_ENCODING_NONE), xmlFreeParserCtxt);
ctxt->linenumbers = 1;
ctxt->options |= XML_PARSE_NOENT|XML_PARSE_NONET;
if(hugeFile)
ctxt->options = XML_PARSE_HUGE;
ctxt->options |= XML_PARSE_HUGE;
auto result = xmlParseDocument(ctxt.get());
if(result != 0 || !ctxt->wellFormed)
THROW("%s", ctxt->lastError.message);
Expand Down
2 changes: 1 addition & 1 deletion src/crypto/Connect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ Connect::Result Connect::exec(initializer_list<pair<string_view,string_view>> he
if(!r.isRedirect() || recursive > 3)
return r;
string location = r.headers.find("Location") == r.headers.cend() ? r.headers["location"] : r.headers["Location"];
string url = location.find("://") != string::npos ? location : baseurl + location;
string url = location.find("://") != string::npos ? std::move(location) : baseurl + location;
Connect c(url, method, timeout);
c.recursive = recursive + 1;
return c.exec(headers);
Expand Down
47 changes: 27 additions & 20 deletions src/crypto/X509Crypto.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <algorithm>
#include <array>
#include <charconv>
#include <unordered_map>

using namespace digidoc;
using namespace std;
Expand Down Expand Up @@ -103,40 +104,47 @@ int X509Crypto::compareIssuerToString(string_view name) const
"UID", "userId"
};

for(size_t old = 0, pos = name.find(','); ; pos = name.find(',', old))
bool escape = false;
string_view key;
std::unordered_map<string_view,string_view> data;
for(size_t i = 0, pos = 0; i < name.size(); ++i)
{
if(pos == string::npos)
pos = name.size();
if(pos < old)
break;
if(name[pos-1] == '\\')
if(escape)
escape = false;
else if(char chr = name[i]; chr == '\\')
escape = true;
else if(chr == '=' && key.empty())
{
old = pos + 1;
continue;
key = name.substr(pos, i - pos);
pos += key.size() + 1;
}
else if(auto last = (i + 1) == name.size(); last || chr == ',')
{
auto value = name.substr(pos, last ? string_view::npos : i - pos);
data[key] = value;
key = {};
pos += value.size() + 1;
}
}

auto nameitem = name.substr(old, pos - old);
old = pos + 1;

if(pos = nameitem.find('=');
pos == string::npos || pos == 0 || nameitem[pos-1] == '\\')
continue;

auto obj = find(list.cbegin(), list.cend(), nameitem.substr(0, pos));
X509_NAME *issuer = X509_get_issuer_name(cert.handle());
for(const auto &[key, val]: data)
{
auto obj = find(list.cbegin(), list.cend(), key);
if(obj == list.cend())
continue;

if(*obj == "STREET"sv)
obj++;
ASN1_OBJECT *obja = OBJ_txt2obj(*obj, 0);
if(!obja)
continue;
return -1;

static const string_view escape = " #+,;<=>\\";
string value(nameitem.substr(pos+1, pos-old));
string value(val);
static const errc ok{};
uint8_t result{};
for(string::size_type pos = value.find('\\'); pos < value.size(); pos = value.find('\\', ++pos))
for(size_t pos = value.find('\\'); pos < value.size(); pos = value.find('\\', ++pos))
{
if(auto data = next(value.data(), pos + 1); from_chars(data, next(data, 2), result, 16).ec == ok)
{
Expand All @@ -148,7 +156,6 @@ int X509Crypto::compareIssuerToString(string_view name) const
}

bool found = false;
X509_NAME *issuer = X509_get_issuer_name(cert.handle());
for(int i = 0; i < X509_NAME_entry_count(issuer); ++i)
{
X509_NAME_ENTRY *entb = X509_NAME_get_entry(issuer, i);
Expand Down
39 changes: 19 additions & 20 deletions src/digidoc-tool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,7 @@ static ostream &operator<<(ostream &os, Signature::Validator::Status status)

static ostream &endl(ostream &os)
{
os.put('\n');
return os;
return os.put('\n');
}
}

Expand Down Expand Up @@ -384,14 +383,14 @@ ToolConfig::ToolConfig(int argc, char *argv[])
{
for(int i = 2; i < argc; i++)
{
string arg(toUTF8(argv[i]));
string_view arg(argv[i]);
if(arg.find("--profile=") == 0)
profile = arg.substr(10);
else if(arg.find("--file=") == 0)
{
string arg2(i+1 < argc ? toUTF8(argv[i+1]) : string());
string_view arg2(i+1 < argc ? argv[i+1] : string_view());
files.emplace(arg.substr(7),
arg2.find("--mime=") == 0 ? arg2.substr(7) : "application/octet-stream");
arg2.find("--mime=") == 0 ? toUTF8(arg2.substr(7)) : "application/octet-stream");
}
#ifdef _WIN32
else if(arg == "--cng") cng = true;
Expand All @@ -402,23 +401,23 @@ ToolConfig::ToolConfig(int argc, char *argv[])
{
cng = false;
if(arg.find('=') != string::npos)
pkcs11 = arg.substr(arg.find('=') + 1);
pkcs11 = toUTF8(arg.substr(arg.find('=') + 1));
}
else if(arg.find("--pkcs12=") == 0)
{
cng = false;
pkcs12 = arg.substr(9);
pkcs12 = toUTF8(arg.substr(9));
}
else if(arg == "--dontValidate") dontValidate = true;
else if(arg == "--XAdESEN") XAdESEN = true;
else if(arg.find("--pin=") == 0) pin = arg.substr(6);
else if(arg.find("--cert=") == 0) cert = arg.substr(7);
else if(arg.find("--city=") == 0) city = arg.substr(7);
else if(arg.find("--street=") == 0) street = arg.substr(9);
else if(arg.find("--state=") == 0) state = arg.substr(8);
else if(arg.find("--postalCode=") == 0) postalCode = arg.substr(13);
else if(arg.find("--country=") == 0) country = arg.substr(10);
else if(arg.find("--role=") == 0) roles.push_back(arg.substr(7));
else if(arg.find("--cert=") == 0) cert = toUTF8(arg.substr(7));
else if(arg.find("--city=") == 0) city = toUTF8(arg.substr(7));
else if(arg.find("--street=") == 0) street = toUTF8(arg.substr(9));
else if(arg.find("--state=") == 0) state = toUTF8(arg.substr(8));
else if(arg.find("--postalCode=") == 0) postalCode = toUTF8(arg.substr(13));
else if(arg.find("--country=") == 0) country = toUTF8(arg.substr(10));
else if(arg.find("--role=") == 0) roles.push_back(toUTF8(arg.substr(7)));
else if(arg == "--sha224") uri = URI_SHA224;
else if(arg == "--sha256") uri = URI_SHA256;
else if(arg == "--sha384") uri = URI_SHA384;
Expand All @@ -435,13 +434,13 @@ ToolConfig::ToolConfig(int argc, char *argv[])
else if(arg == "--rsapss") rsaPss = true;
else if(arg.find("--tsurl") == 0) tsurl = arg.substr(8);
else if(arg.find("--tslurl=") == 0) tslurl = arg.substr(9);
else if(arg.find("--tslcert=") == 0) tslcerts = vector<X509Cert>{ X509Cert(arg.substr(10)) };
else if(arg.find("--tslcert=") == 0) tslcerts = vector<X509Cert>{ X509Cert(toUTF8(arg.substr(10))) };
else if(arg == "--TSLAllowExpired") expired = true;
else if(arg == "--dontsign") doSign = false;
else if(arg == "--nocolor") RED = GREEN = YELLOW = RESET = {};
else if(arg.find("--loglevel=") == 0) _logLevel = stoi(arg.substr(11));
else if(arg.find("--logfile=") == 0) _logFile = arg.substr(10);
else path = arg;
else if(arg.find("--loglevel=") == 0) _logLevel = atoi(arg.substr(11).data());
else if(arg.find("--logfile=") == 0) _logFile = toUTF8(arg.substr(10));
else path = toUTF8(arg);
}
}

Expand Down Expand Up @@ -917,7 +916,7 @@ static int tslcmd(int /*argc*/, char* /*argv*/[])
{
int returnCode = EXIT_SUCCESS;
string cache = CONF(TSLCache);
TSL t(cache + "/" + File::fileName(CONF(TSLUrl)));
TSL t(File::path(cache, File::fileName(CONF(TSLUrl))));
cout << "TSL: " << t.url() << endl
<< " Type: " << t.type() << endl
<< " Territory: " << t.territory() << endl
Expand Down Expand Up @@ -953,7 +952,7 @@ static int tslcmd(int /*argc*/, char* /*argv*/[])
cout << " TSL: missing" << endl;
continue;
}
TSL tp(std::move(path));
TSL tp(path);
cout << " TSL: " << p.location << endl
<< " Type: " << tp.type() << endl
<< " Territory: " << tp.territory() << endl
Expand Down

0 comments on commit b62ada8

Please sign in to comment.