Skip to content

Commit

Permalink
Fix CodeQL quality tasks (open-eid#1151)
Browse files Browse the repository at this point in the history
IB-7628

Signed-off-by: Raul Metsma <[email protected]>
  • Loading branch information
metsma authored Feb 9, 2023
1 parent 8423a83 commit 20bd6ba
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 65 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ jobs:
patterns: |
-**/*autogen*/**
-**/common/qtsingleapplication/**
-**:cpp/loop-variable-changed
-**:cpp/poorly-documented-function
input: sarif-results/cpp.sarif
output: sarif-results/cpp.sarif
Expand Down
13 changes: 5 additions & 8 deletions client/Diagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@
#include <QtCore/QTextStream>
#include <QtNetwork/QSslCertificate>

Diagnostics::Diagnostics() = default;

void Diagnostics::generalInfo(QTextStream &s) const
void Diagnostics::generalInfo(QTextStream &s)
{
s << "<b>" << tr("Arguments:") << "</b> " << qApp->arguments().join(' ') << "<br />"
<< "<b>" << tr("Library paths:") << "</b> " << QCoreApplication::libraryPaths().join(';') << "<br />"
Expand Down Expand Up @@ -77,11 +75,10 @@ void Diagnostics::generalInfo(QTextStream &s) const
QJsonObject metainf = qApp->conf()->object().value(QStringLiteral("META-INF")).toObject();
for(QJsonObject::const_iterator i = metainf.constBegin(), end = metainf.constEnd(); i != end; ++i)
{
switch(i.value().type())
{
case QJsonValue::Double: s << "<br />" << i.key() << ": " << i.value().toInt(); break;
default: s << "<br />" << i.key() << ": " << i.value().toString(); break;
}
if(i.value().type() == QJsonValue::Double)
s << "<br />" << i.key() << ": " << i.value().toInt();
else
s << "<br />" << i.key() << ": " << i.value().toString();
}
s << "<br /><br />";
#endif
Expand Down
4 changes: 1 addition & 3 deletions client/Diagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,13 @@ class Diagnostics: public QObject, public QRunnable
{
Q_OBJECT
public:
Diagnostics();

void run() override;

signals:
void update( const QString &data );

private:
void generalInfo(QTextStream &s) const;
static void generalInfo(QTextStream &s) ;
QStringList packages(const QStringList &names, bool withName = true);

#if defined(Q_OS_WIN) || defined(Q_OS_LINUX)
Expand Down
24 changes: 11 additions & 13 deletions client/DigiDoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ QStringList DigiDocSignature::roles() const
{
QStringList list;
for(const std::string &role: s->signerRoles())
list << from( role ).trimmed();
list.append(from( role ).trimmed());
return list;
}

Expand All @@ -174,11 +174,9 @@ void DigiDocSignature::setLastError(const Exception &e)
QStringList causes;
Exception::ExceptionCode code = Exception::General;
DigiDoc::parseException(e, causes, code);
switch(code) {
case Exception::OCSPBeforeTimeStamp:
m_lastError = DigiDoc::tr("The timestamp added to the signature must be taken before validity confirmation."); break;
default: m_lastError = causes.join('\n');
}
m_lastError = code == Exception::OCSPBeforeTimeStamp ?
DigiDoc::tr("The timestamp added to the signature must be taken before validity confirmation.") :
causes.join('\n');
}

QString DigiDocSignature::signatureMethod() const
Expand All @@ -199,12 +197,12 @@ DigiDocSignature::SignatureStatus DigiDocSignature::status() const
return m_status;
}

QSslCertificate DigiDocSignature::toCertificate(const std::vector<unsigned char> &der) const
QSslCertificate DigiDocSignature::toCertificate(const std::vector<unsigned char> &der)
{
return QSslCertificate(QByteArray::fromRawData((const char *)der.data(), int(der.size())), QSsl::Der);
}

QDateTime DigiDocSignature::toTime(const std::string &time) const
QDateTime DigiDocSignature::toTime(const std::string &time)
{
if(time.empty())
return {};
Expand Down Expand Up @@ -313,7 +311,7 @@ bool SDocumentModel::addFile(const QString &file, const QString &mime)

void SDocumentModel::addTempReference(const QString &file)
{
doc->m_tempFiles << file;
doc->m_tempFiles.append(file);
}

QString SDocumentModel::data(int row) const
Expand Down Expand Up @@ -351,13 +349,13 @@ void SDocumentModel::open(int row)
QFileInfo f(save(row, path));
if( !f.exists() )
return;
doc->m_tempFiles << f.absoluteFilePath();
doc->m_tempFiles.append(f.absoluteFilePath());
#if defined(Q_OS_WIN)
::SetFileAttributesW(f.absoluteFilePath().toStdWString().c_str(), FILE_ATTRIBUTE_READONLY);
#else
QFile::setPermissions(f.absoluteFilePath(), QFile::Permissions(QFile::Permission::ReadOwner));
#endif
if(!doc->fileName().endsWith(".pdf", Qt::CaseInsensitive) && FileDialog::isSignedPDF(f.absoluteFilePath()))
if(!doc->fileName().endsWith(QStringLiteral(".pdf"), Qt::CaseInsensitive) && FileDialog::isSignedPDF(f.absoluteFilePath()))
qApp->showClient({ f.absoluteFilePath() }, false, false, true);
else
QDesktopServices::openUrl(QUrl::fromLocalFile(f.absoluteFilePath()));
Expand Down Expand Up @@ -532,7 +530,7 @@ bool DigiDoc::open( const QString &file )
f->saveAs(to(tmppath));
if(QFileInfo::exists(tmppath))
{
m_tempFiles << tmppath;
m_tempFiles.append(tmppath);
try {
parentContainer = std::exchange(b, Container::openPtr(to(tmppath)));
} catch(const Exception &) {}
Expand Down Expand Up @@ -572,7 +570,7 @@ bool DigiDoc::open( const QString &file )

void DigiDoc::parseException(const Exception &e, QStringList &causes, Exception::ExceptionCode &code)
{
causes << QStringLiteral("%1:%2 %3").arg(QFileInfo(from(e.file())).fileName()).arg(e.line()).arg(from(e.msg()));
causes.append(QStringLiteral("%1:%2 %3").arg(QFileInfo(from(e.file())).fileName()).arg(e.line()).arg(from(e.msg())));
switch( e.code() )
{
case Exception::CertificateRevoked:
Expand Down
6 changes: 3 additions & 3 deletions client/DigiDoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ class DigiDocSignature
void setLastError(const digidoc::Exception &e);
void parseException(SignatureStatus &result, const digidoc::Exception &e);
SignatureStatus validate();
QSslCertificate toCertificate(const std::vector<unsigned char> &der) const;
QDateTime toTime(const std::string &time) const;
static QSslCertificate toCertificate(const std::vector<unsigned char> &der) ;
static QDateTime toTime(const std::string &time) ;

const digidoc::Signature *s;
QString m_lastError;
Expand Down Expand Up @@ -150,7 +150,7 @@ class DigiDoc: public QObject

private:
bool isError(bool failure, const QString &msg = {}) const;
void setLastError( const QString &msg, const digidoc::Exception &e );
static void setLastError( const QString &msg, const digidoc::Exception &e );

std::unique_ptr<digidoc::Container> b;
std::unique_ptr<digidoc::Container> parentContainer;
Expand Down
1 change: 0 additions & 1 deletion client/QSmartCard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,6 @@ bool IDEMIACard::loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const
case 12: d->data[QSmartCardData::Comment2] = record; break;
case 13: d->data[QSmartCardData::Comment3] = record; break;
case 14: d->data[QSmartCardData::Comment4] = record; break;
//case 15: d->data[QSmartCardData::Comment5] = record; break;
default: break;
}
}
Expand Down
41 changes: 14 additions & 27 deletions client/SslCertificate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,12 @@
template <typename Func, typename Arg>
static QByteArray i2dDer(Func func, Arg arg)
{
QByteArray der;
if(!arg)
return der;
der.resize(func(arg, nullptr));
if(der.isEmpty())
return der;
return {};
QByteArray der(func(arg, nullptr), 0);
unsigned char *p = (unsigned char*)der.data();
if(func(arg, &p) != der.size())
der.clear();
if(der.isEmpty() || func(arg, &p) != der.size())
return {};
return der;
}

Expand Down Expand Up @@ -88,10 +85,10 @@ QString SslCertificate::subjectInfo( QSslCertificate::SubjectInfo subject ) cons

QMultiHash<SslCertificate::AuthorityInfoAccess, QString> SslCertificate::authorityInfoAccess() const
{
QMultiHash<AuthorityInfoAccess, QString> result;
auto info = SCOPE(AUTHORITY_INFO_ACCESS, extension(NID_info_access));
if(!info)
return result;
return {};
QMultiHash<AuthorityInfoAccess, QString> result;
for(int i = 0; i < sk_ACCESS_DESCRIPTION_num(info.get()); ++i)
{
ACCESS_DESCRIPTION *ad = sk_ACCESS_DESCRIPTION_value(info.get(), i);
Expand Down Expand Up @@ -127,14 +124,11 @@ QByteArray SslCertificate::authorityKeyIdentifier() const

QHash<SslCertificate::EnhancedKeyUsage,QString> SslCertificate::enhancedKeyUsage() const
{
QHash<EnhancedKeyUsage,QString> list;
auto usage = SCOPE(EXTENDED_KEY_USAGE, extension(NID_ext_key_usage));
if( !usage )
{
list[All] = tr("All application policies");
return list;
}
if(!usage)
return { {All, tr("All application policies")} };

QHash<EnhancedKeyUsage,QString> list;
for(int i = 0; i < sk_ASN1_OBJECT_num(usage.get()); ++i)
{
ASN1_OBJECT *obj = sk_ASN1_OBJECT_value(usage.get(), i);
Expand Down Expand Up @@ -194,10 +188,10 @@ Qt::HANDLE SslCertificate::extension( int nid ) const

QHash<SslCertificate::KeyUsage,QString> SslCertificate::keyUsage() const
{
QHash<KeyUsage,QString> list;
auto keyusage = SCOPE(ASN1_BIT_STRING, extension(NID_key_usage));
if(!keyusage)
return list;
return {};
QHash<KeyUsage,QString> list;
for( int n = 0; n < 9; ++n )
{
if(!ASN1_BIT_STRING_get_bit(keyusage.get(), n))
Expand Down Expand Up @@ -234,10 +228,10 @@ QString SslCertificate::personalCode() const
QStringList SslCertificate::policies() const
{
auto cp = SCOPE(CERTIFICATEPOLICIES, extension(NID_certificate_policies));
QStringList list;
if( !cp )
return list;
return {};

QStringList list;
for(int i = 0; i < sk_POLICYINFO_num(cp.get()); ++i)
{
POLICYINFO *pi = sk_POLICYINFO_value(cp.get(), i);
Expand Down Expand Up @@ -289,8 +283,7 @@ QString SslCertificate::toString( const QString &format ) const
QRegularExpression r(QStringLiteral("[a-zA-Z]+"));
QString ret = format;
QRegularExpressionMatch match;
int pos = 0;
while((match = r.match(ret, pos)).hasMatch()) {
for(int pos = 0; (match = r.match(ret, pos)).hasMatch(); ) {
QString cap = match.captured();
QString si = cap == QStringLiteral("serialNumber") ? personalCode() : subjectInfo(cap.toLatin1());
ret.replace(match.capturedStart(), cap.size(), si);
Expand Down Expand Up @@ -402,12 +395,6 @@ SslCertificate::Validity SslCertificate::validateOnline() const
auto basic = SCOPE(OCSP_BASICRESP, OCSP_response_get1_basic(resp.get()));
if(!basic)
return Unknown;
//OCSP_TRUSTOTHER - enables OCSP_NOVERIFY
//OCSP_NOSIGS - does not verify ocsp signatures
//OCSP_NOVERIFY - ignores signer(responder) cert verification, requires store otherwise crashes
//OCSP_NOCHECKS - cancel futurer responder issuer checks and trust bits
//OCSP_NOEXPLICIT - returns 0 by mistake
//all checks enabled fails trust bit check, cant use OCSP_NOEXPLICIT instead using OCSP_NOCHECKS
if(OCSP_basic_verify(basic.get(), nullptr, nullptr, OCSP_NOVERIFY) <= 0)
return Unknown;
int status = -1;
Expand Down
6 changes: 2 additions & 4 deletions client/SslCertificate.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ class SslCertificate: public QSslCertificate
public:
enum EnhancedKeyUsage
{
EnhancedKeyUsageNone = -1,
All = 0,
All,
ClientAuth,
ServerAuth,
EmailProtect,
Expand All @@ -43,8 +42,7 @@ class SslCertificate: public QSslCertificate
};
enum KeyUsage
{
KeyUsageNone = -1,
DigitalSignature = 0,
DigitalSignature,
NonRepudiation,
KeyEncipherment,
DataEncipherment,
Expand Down
6 changes: 3 additions & 3 deletions client/TSLDownload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ int main(int argc, char *argv[])
{
QNetworkReply *rt = m.get(QNetworkRequest(QUrl(url)));
QEventLoop e;
QObject::connect(rt, &QNetworkReply::finished, [&](){
QObject::connect(rt, &QNetworkReply::finished, rt, [&](){
QFile t(QStringLiteral("%1/%2.xml").arg(path, territory));
if(t.open(QFile::WriteOnly))
t.write(rt->readAll());
Expand All @@ -81,8 +81,8 @@ int main(int argc, char *argv[])
w.writeStartElement(QStringLiteral("qresource"));
w.writeAttribute(QStringLiteral("prefix"), QStringLiteral("TSL"));
w.writeTextElement(QStringLiteral("file"), r->request().url().fileName());
for(const QString &territory: territories)
w.writeTextElement(QStringLiteral("file"), territory + ".xml");
for(const QString &t: territories)
w.writeTextElement(QStringLiteral("file"), t + ".xml");
w.writeEndElement();
w.writeEndElement();

Expand Down
3 changes: 1 addition & 2 deletions client/dialogs/SettingsDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -582,8 +582,7 @@ QSslCertificate SettingsDialog::selectCert(const QString &label, const QString &

void SettingsDialog::selectLanguage()
{
const QList<QAbstractButton*> list = ui->langGroup->buttons();
for(QAbstractButton *button: list)
for(QAbstractButton *button: ui->langGroup->buttons())
button->setChecked(button->property("lang").toString() == Common::language());
}

Expand Down
1 change: 0 additions & 1 deletion client/widgets/SignatureItem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ SignatureItem::~SignatureItem()
void SignatureItem::init()
{
const SslCertificate cert = ui->signature.cert();
DigiDocSignature::SignatureStatus signatureValidity = ui->signature.status();

ui->serial.clear();
ui->status.clear();
Expand Down

0 comments on commit 20bd6ba

Please sign in to comment.