From ccfd7a065c821b3075144f497ddff6b4c80598f1 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sat, 6 Jan 2018 17:06:51 +0100 Subject: Fix coding style and GUI test --- src/crypto/kdf/Argon2Kdf.cpp | 8 +- src/crypto/kdf/Argon2Kdf.h | 2 +- src/format/Kdbx3Reader.cpp | 83 ++++---- src/format/Kdbx3Reader.h | 1 + src/format/Kdbx3Writer.cpp | 32 ++- src/format/Kdbx3Writer.h | 1 + src/format/Kdbx3XmlReader.cpp | 1 + src/format/Kdbx3XmlReader.h | 1 + src/format/Kdbx3XmlWriter.cpp | 19 +- src/format/Kdbx3XmlWriter.h | 1 + src/format/Kdbx4Reader.cpp | 89 ++++----- src/format/Kdbx4Reader.h | 2 +- src/format/Kdbx4Writer.cpp | 102 +++++----- src/format/Kdbx4Writer.h | 2 +- src/format/Kdbx4XmlReader.cpp | 355 ++++++++++++++++++++-------------- src/format/Kdbx4XmlReader.h | 6 +- src/format/Kdbx4XmlWriter.cpp | 2 +- src/format/Kdbx4XmlWriter.h | 2 +- src/format/KeePass2.cpp | 17 +- src/format/KeePass2.h | 4 +- src/format/KeePass2RandomStream.cpp | 4 +- src/format/KeePass2Reader.cpp | 2 +- src/format/KeePass2Writer.h | 2 + src/gui/DatabaseSettingsWidget.cpp | 4 +- src/streams/HmacBlockStream.cpp | 34 ++-- src/streams/SymmetricCipherStream.cpp | 3 +- tests/TestKeePass2RandomStream.cpp | 2 +- tests/gui/TestGui.cpp | 4 +- 28 files changed, 416 insertions(+), 369 deletions(-) diff --git a/src/crypto/kdf/Argon2Kdf.cpp b/src/crypto/kdf/Argon2Kdf.cpp index 12e9135af..154f2ffe4 100644 --- a/src/crypto/kdf/Argon2Kdf.cpp +++ b/src/crypto/kdf/Argon2Kdf.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2017 KeePassXC Team + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -33,7 +33,7 @@ Argon2Kdf::Argon2Kdf() : Kdf::Kdf(KeePass2::KDF_ARGON2) , m_version(0x13) - , m_memory(1<<16) + , m_memory(1 << 16) , m_parallelism(2) { m_rounds = 1; @@ -63,7 +63,7 @@ quint64 Argon2Kdf::memory() const bool Argon2Kdf::setMemory(quint64 kibibytes) { // MIN=8KB; MAX=2,147,483,648KB - if (kibibytes >= 8 && kibibytes < (1ULL<<32)) { + if (kibibytes >= 8 && kibibytes < (1ULL << 32)) { m_memory = kibibytes; return true; } @@ -79,7 +79,7 @@ quint32 Argon2Kdf::parallelism() const bool Argon2Kdf::setParallelism(quint32 threads) { // MIN=1; MAX=16,777,215 - if (threads >= 1 && threads < (1<<24)) { + if (threads >= 1 && threads < (1 << 24)) { m_parallelism = threads; return true; } diff --git a/src/crypto/kdf/Argon2Kdf.h b/src/crypto/kdf/Argon2Kdf.h index 345ca279c..fe62b2953 100644 --- a/src/crypto/kdf/Argon2Kdf.h +++ b/src/crypto/kdf/Argon2Kdf.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2017 KeePassXC Team + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/src/format/Kdbx3Reader.cpp b/src/format/Kdbx3Reader.cpp index 3187442be..5fb72a23e 100644 --- a/src/format/Kdbx3Reader.cpp +++ b/src/format/Kdbx3Reader.cpp @@ -1,4 +1,5 @@ /* + * Copyright (C) 2017 KeePassXC Team * Copyright (C) 2010 Felix Geyer * * This program is free software: you can redistribute it and/or modify @@ -161,7 +162,7 @@ Database* Kdbx3Reader::readDatabase(QIODevice* device, const CompositeKey& key, xmlDevice = ioCompressor.data(); } - KeePass2RandomStream randomStream(KeePass2::Salsa20); + KeePass2RandomStream randomStream(KeePass2::ProtectedStreamAlgo::Salsa20); if (!randomStream.init(m_protectedStreamKey)) { raiseError(randomStream.errorString()); return nullptr; @@ -208,10 +209,10 @@ bool Kdbx3Reader::readHeaderField() raiseError("Invalid header id size"); return false; } - quint8 fieldID = fieldIDArray.at(0); + char fieldID = fieldIDArray.at(0); bool ok; - quint16 fieldLen = Endian::readSizedInt(m_headerStream, KeePass2::BYTEORDER, &ok); + auto fieldLen = Endian::readSizedInt(m_headerStream, KeePass2::BYTEORDER, &ok); if (!ok) { raiseError("Invalid header field length"); return false; @@ -226,44 +227,44 @@ bool Kdbx3Reader::readHeaderField() } } - switch (fieldID) { - case KeePass2::EndOfHeader: + switch (static_cast(fieldID)) { + case KeePass2::HeaderFieldID::EndOfHeader: m_headerEnd = true; break; - case KeePass2::CipherID: + case KeePass2::HeaderFieldID::CipherID: setCipher(fieldData); break; - case KeePass2::CompressionFlags: + case KeePass2::HeaderFieldID::CompressionFlags: setCompressionFlags(fieldData); break; - case KeePass2::MasterSeed: + case KeePass2::HeaderFieldID::MasterSeed: setMasterSeed(fieldData); break; - case KeePass2::TransformSeed: + case KeePass2::HeaderFieldID::TransformSeed: setTransformSeed(fieldData); break; - case KeePass2::TransformRounds: + case KeePass2::HeaderFieldID::TransformRounds: setTransformRounds(fieldData); break; - case KeePass2::EncryptionIV: + case KeePass2::HeaderFieldID::EncryptionIV: setEncryptionIV(fieldData); break; - case KeePass2::ProtectedStreamKey: + case KeePass2::HeaderFieldID::ProtectedStreamKey: setProtectedStreamKey(fieldData); break; - case KeePass2::StreamStartBytes: + case KeePass2::HeaderFieldID::StreamStartBytes: setStreamStartBytes(fieldData); break; - case KeePass2::InnerRandomStreamID: + case KeePass2::HeaderFieldID::InnerRandomStreamID: setInnerRandomStreamID(fieldData); break; @@ -279,39 +280,40 @@ void Kdbx3Reader::setCipher(const QByteArray& data) { if (data.size() != Uuid::Length) { raiseError("Invalid cipher uuid length"); - } else { - Uuid uuid(data); + return; + } - if (SymmetricCipher::cipherToAlgorithm(uuid) == SymmetricCipher::InvalidAlgorithm) { - raiseError("Unsupported cipher"); - } else { - m_db->setCipher(uuid); - } + Uuid uuid(data); + + if (SymmetricCipher::cipherToAlgorithm(uuid) == SymmetricCipher::InvalidAlgorithm) { + raiseError("Unsupported cipher"); + return; } + m_db->setCipher(uuid); } void Kdbx3Reader::setCompressionFlags(const QByteArray& data) { if (data.size() != 4) { raiseError("Invalid compression flags length"); - } else { - quint32 id = Endian::bytesToSizedInt(data, KeePass2::BYTEORDER); + return; + } + auto id = Endian::bytesToSizedInt(data, KeePass2::BYTEORDER); - if (id > Database::CompressionAlgorithmMax) { - raiseError("Unsupported compression algorithm"); - } else { - m_db->setCompressionAlgo(static_cast(id)); - } + if (id > Database::CompressionAlgorithmMax) { + raiseError("Unsupported compression algorithm"); + return; } + m_db->setCompressionAlgo(static_cast(id)); } void Kdbx3Reader::setMasterSeed(const QByteArray& data) { if (data.size() != 32) { raiseError("Invalid master seed size"); - } else { - m_masterSeed = data; + return; } + m_masterSeed = data; } void Kdbx3Reader::setTransformSeed(const QByteArray& data) @@ -355,22 +357,23 @@ void Kdbx3Reader::setStreamStartBytes(const QByteArray& data) { if (data.size() != 32) { raiseError("Invalid start bytes size"); - } else { - m_streamStartBytes = data; + return; } + m_streamStartBytes = data; } void Kdbx3Reader::setInnerRandomStreamID(const QByteArray& data) { if (data.size() != 4) { raiseError("Invalid random stream id size"); - } else { - quint32 id = Endian::bytesToSizedInt(data, KeePass2::BYTEORDER); - KeePass2::ProtectedStreamAlgo irsAlgo = KeePass2::idToProtectedStreamAlgo(id); - if (irsAlgo == KeePass2::InvalidProtectedStreamAlgo || irsAlgo == KeePass2::ArcFourVariant) { - raiseError("Invalid inner random stream cipher"); - } else { - m_irsAlgo = irsAlgo; - } + return; + } + quint32 id = Endian::bytesToSizedInt(data, KeePass2::BYTEORDER); + KeePass2::ProtectedStreamAlgo irsAlgo = KeePass2::idToProtectedStreamAlgo(id); + if (irsAlgo == KeePass2::ProtectedStreamAlgo::InvalidProtectedStreamAlgo || + irsAlgo == KeePass2::ProtectedStreamAlgo::ArcFourVariant) { + raiseError("Invalid inner random stream cipher"); + return; } + m_irsAlgo = irsAlgo; } diff --git a/src/format/Kdbx3Reader.h b/src/format/Kdbx3Reader.h index 24a1ef100..024189eda 100644 --- a/src/format/Kdbx3Reader.h +++ b/src/format/Kdbx3Reader.h @@ -1,4 +1,5 @@ /* + * Copyright (C) 2017 KeePassXC Team * Copyright (C) 2010 Felix Geyer * * This program is free software: you can redistribute it and/or modify diff --git a/src/format/Kdbx3Writer.cpp b/src/format/Kdbx3Writer.cpp index 2fedf273c..ca02fb7ac 100644 --- a/src/format/Kdbx3Writer.cpp +++ b/src/format/Kdbx3Writer.cpp @@ -1,4 +1,5 @@ /* + * Copyright (C) 2017 KeePassXC Team * Copyright (C) 2010 Felix Geyer * * This program is free software: you can redistribute it and/or modify @@ -32,9 +33,6 @@ #include "streams/QtIOCompressor" #include "streams/SymmetricCipherStream.h" -#define CHECK_RETURN(x) if (!(x)) return; -#define CHECK_RETURN_FALSE(x) if (!(x)) return false; - Kdbx3Writer::Kdbx3Writer() : m_device(0) { @@ -51,7 +49,7 @@ bool Kdbx3Writer::writeDatabase(QIODevice* device, Database* db) QByteArray startBytes = randomGen()->randomArray(32); QByteArray endOfHeader = "\r\n\r\n"; - if (db->challengeMasterSeed(masterSeed) == false) { + if (!db->challengeMasterSeed(masterSeed)) { raiseError(tr("Unable to issue challenge-response.")); return false; } @@ -76,23 +74,23 @@ bool Kdbx3Writer::writeDatabase(QIODevice* device, Database* db) CHECK_RETURN_FALSE(writeData(Endian::sizedIntToBytes(KeePass2::SIGNATURE_2, KeePass2::BYTEORDER))); CHECK_RETURN_FALSE(writeData(Endian::sizedIntToBytes(KeePass2::FILE_VERSION_3, KeePass2::BYTEORDER))); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::CipherID, db->cipher().toByteArray())); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::CompressionFlags, + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::CipherID, db->cipher().toByteArray())); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::CompressionFlags, Endian::sizedIntToBytes(db->compressionAlgo(), KeePass2::BYTEORDER))); auto kdf = db->kdf(); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::MasterSeed, masterSeed)); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::TransformSeed, kdf->seed())); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::TransformRounds, + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::MasterSeed, masterSeed)); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::TransformSeed, kdf->seed())); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::TransformRounds, Endian::sizedIntToBytes(kdf->rounds(), KeePass2::BYTEORDER))); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::EncryptionIV, encryptionIV)); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::ProtectedStreamKey, protectedStreamKey)); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::StreamStartBytes, startBytes)); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::InnerRandomStreamID, - Endian::sizedIntToBytes(KeePass2::Salsa20, + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::EncryptionIV, encryptionIV)); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::ProtectedStreamKey, protectedStreamKey)); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::StreamStartBytes, startBytes)); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::InnerRandomStreamID, + Endian::sizedIntToBytes(static_cast(KeePass2::ProtectedStreamAlgo::Salsa20), KeePass2::BYTEORDER))); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::EndOfHeader, endOfHeader)); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::EndOfHeader, endOfHeader)); header.close(); m_device = device; @@ -130,7 +128,7 @@ bool Kdbx3Writer::writeDatabase(QIODevice* device, Database* db) m_device = ioCompressor.data(); } - KeePass2RandomStream randomStream(KeePass2::Salsa20); + KeePass2RandomStream randomStream(KeePass2::ProtectedStreamAlgo::Salsa20); if (!randomStream.init(protectedStreamKey)) { raiseError(randomStream.errorString()); return false; @@ -175,7 +173,7 @@ bool Kdbx3Writer::writeHeaderField(KeePass2::HeaderFieldID fieldId, const QByteA Q_ASSERT(data.size() <= 65535); QByteArray fieldIdArr; - fieldIdArr[0] = fieldId; + fieldIdArr[0] = static_cast(fieldId); CHECK_RETURN_FALSE(writeData(fieldIdArr)); CHECK_RETURN_FALSE(writeData(Endian::sizedIntToBytes(static_cast(data.size()), KeePass2::BYTEORDER))); diff --git a/src/format/Kdbx3Writer.h b/src/format/Kdbx3Writer.h index f3368eebb..1b9fe7563 100644 --- a/src/format/Kdbx3Writer.h +++ b/src/format/Kdbx3Writer.h @@ -1,4 +1,5 @@ /* + * Copyright (C) 2017 KeePassXC Team * Copyright (C) 2010 Felix Geyer * * This program is free software: you can redistribute it and/or modify diff --git a/src/format/Kdbx3XmlReader.cpp b/src/format/Kdbx3XmlReader.cpp index 3a4bdbd9d..18dd0914d 100644 --- a/src/format/Kdbx3XmlReader.cpp +++ b/src/format/Kdbx3XmlReader.cpp @@ -1,4 +1,5 @@ /* + * Copyright (C) 2017 KeePassXC Team * Copyright (C) 2010 Felix Geyer * * This program is free software: you can redistribute it and/or modify diff --git a/src/format/Kdbx3XmlReader.h b/src/format/Kdbx3XmlReader.h index 477eb3865..213b21c92 100644 --- a/src/format/Kdbx3XmlReader.h +++ b/src/format/Kdbx3XmlReader.h @@ -1,4 +1,5 @@ /* + * Copyright (C) 2017 KeePassXC Team * Copyright (C) 2010 Felix Geyer * * This program is free software: you can redistribute it and/or modify diff --git a/src/format/Kdbx3XmlWriter.cpp b/src/format/Kdbx3XmlWriter.cpp index 40c20e65c..bfa94a77c 100644 --- a/src/format/Kdbx3XmlWriter.cpp +++ b/src/format/Kdbx3XmlWriter.cpp @@ -1,4 +1,5 @@ /* + * Copyright (C) 2017 KeePassXC Team * Copyright (C) 2010 Felix Geyer * * This program is free software: you can redistribute it and/or modify @@ -458,11 +459,7 @@ void Kdbx3XmlWriter::writeNumber(const QString& qualifiedName, int number) void Kdbx3XmlWriter::writeBool(const QString& qualifiedName, bool b) { - if (b) { - writeString(qualifiedName, "True"); - } else { - writeString(qualifiedName, "False"); - } + writeString(qualifiedName, b ? "True" : "False"); } void Kdbx3XmlWriter::writeDateTime(const QString& qualifiedName, const QDateTime& dateTime) @@ -487,20 +484,12 @@ void Kdbx3XmlWriter::writeUuid(const QString& qualifiedName, const Uuid& uuid) void Kdbx3XmlWriter::writeUuid(const QString& qualifiedName, const Group* group) { - if (group) { - writeUuid(qualifiedName, group->uuid()); - } else { - writeUuid(qualifiedName, Uuid()); - } + writeUuid(qualifiedName, group ? group->uuid() : Uuid()); } void Kdbx3XmlWriter::writeUuid(const QString& qualifiedName, const Entry* entry) { - if (entry) { - writeUuid(qualifiedName, entry->uuid()); - } else { - writeUuid(qualifiedName, Uuid()); - } + writeUuid(qualifiedName, entry ? entry->uuid() : Uuid()); } void Kdbx3XmlWriter::writeBinary(const QString& qualifiedName, const QByteArray& ba) diff --git a/src/format/Kdbx3XmlWriter.h b/src/format/Kdbx3XmlWriter.h index 2bb130566..6eaf32f35 100644 --- a/src/format/Kdbx3XmlWriter.h +++ b/src/format/Kdbx3XmlWriter.h @@ -1,4 +1,5 @@ /* + * Copyright (C) 2017 KeePassXC Team * Copyright (C) 2010 Felix Geyer * * This program is free software: you can redistribute it and/or modify diff --git a/src/format/Kdbx4Reader.cpp b/src/format/Kdbx4Reader.cpp index 0a69cbf2d..5bac5d005 100644 --- a/src/format/Kdbx4Reader.cpp +++ b/src/format/Kdbx4Reader.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010 Felix Geyer + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -72,8 +72,7 @@ Database* Kdbx4Reader::readDatabase(QIODevice* device, const CompositeKey& key, "This is a one-way migration. You won't be able to open the imported " "database with the old KeePassX 0.4 version.")); return nullptr; - } - else if (!ok || signature2 != KeePass2::SIGNATURE_2) { + } else if (!ok || signature2 != KeePass2::SIGNATURE_2) { raiseError(tr("Not a KeePass database.")); return nullptr; } @@ -203,9 +202,7 @@ Database* Kdbx4Reader::readDatabase(QIODevice* device, const CompositeKey& key, if (keepDatabase) { return db.take(); } - else { - return nullptr; - } + return nullptr; } return db.take(); @@ -218,10 +215,10 @@ bool Kdbx4Reader::readHeaderField(QIODevice* device) raiseError("Invalid header id size"); return false; } - quint8 fieldID = fieldIDArray.at(0); + char fieldID = fieldIDArray.at(0); bool ok; - quint32 fieldLen = Endian::readSizedInt(device, KeePass2::BYTEORDER, &ok); + auto fieldLen = Endian::readSizedInt(device, KeePass2::BYTEORDER, &ok); if (!ok) { raiseError("Invalid header field length"); return false; @@ -236,27 +233,27 @@ bool Kdbx4Reader::readHeaderField(QIODevice* device) } } - switch (fieldID) { - case KeePass2::EndOfHeader: + switch (static_cast(fieldID)) { + case KeePass2::HeaderFieldID::EndOfHeader: return false; - case KeePass2::CipherID: + case KeePass2::HeaderFieldID::CipherID: setCipher(fieldData); break; - case KeePass2::CompressionFlags: + case KeePass2::HeaderFieldID::CompressionFlags: setCompressionFlags(fieldData); break; - case KeePass2::MasterSeed: + case KeePass2::HeaderFieldID::MasterSeed: setMasterSeed(fieldData); break; - case KeePass2::EncryptionIV: + case KeePass2::HeaderFieldID::EncryptionIV: setEncryptionIV(fieldData); break; - case KeePass2::KdfParameters: { + case KeePass2::HeaderFieldID::KdfParameters: { QBuffer bufIoDevice(&fieldData); if (!bufIoDevice.open(QIODevice::ReadOnly)) { raiseError("Failed to open buffer for KDF parameters in header"); @@ -264,7 +261,7 @@ bool Kdbx4Reader::readHeaderField(QIODevice* device) } QVariantMap kdfParams = readVariantMap(&bufIoDevice); QSharedPointer kdf = KeePass2::kdfFromParameters(kdfParams); - if (kdf == nullptr) { + if (!kdf) { raiseError("Invalid KDF parameters"); return false; } @@ -272,15 +269,15 @@ bool Kdbx4Reader::readHeaderField(QIODevice* device) break; } - case KeePass2::PublicCustomData: + case KeePass2::HeaderFieldID::PublicCustomData: m_db->setPublicCustomData(fieldData); break; - case KeePass2::ProtectedStreamKey: - case KeePass2::TransformRounds: - case KeePass2::TransformSeed: - case KeePass2::StreamStartBytes: - case KeePass2::InnerRandomStreamID: + case KeePass2::HeaderFieldID::ProtectedStreamKey: + case KeePass2::HeaderFieldID::TransformRounds: + case KeePass2::HeaderFieldID::TransformSeed: + case KeePass2::HeaderFieldID::StreamStartBytes: + case KeePass2::HeaderFieldID::InnerRandomStreamID: raiseError("Legacy header fields found in KDBX4 file."); return false; @@ -456,39 +453,39 @@ void Kdbx4Reader::setCipher(const QByteArray& data) { if (data.size() != Uuid::Length) { raiseError("Invalid cipher uuid length"); - } else { - Uuid uuid(data); + return; + } + Uuid uuid(data); - if (SymmetricCipher::cipherToAlgorithm(uuid) == SymmetricCipher::InvalidAlgorithm) { - raiseError("Unsupported cipher"); - } else { - m_db->setCipher(uuid); - } + if (SymmetricCipher::cipherToAlgorithm(uuid) == SymmetricCipher::InvalidAlgorithm) { + raiseError("Unsupported cipher"); + return; } + m_db->setCipher(uuid); } void Kdbx4Reader::setCompressionFlags(const QByteArray& data) { if (data.size() != 4) { raiseError("Invalid compression flags length"); - } else { - quint32 id = Endian::bytesToSizedInt(data, KeePass2::BYTEORDER); + return; + } + auto id = Endian::bytesToSizedInt(data, KeePass2::BYTEORDER); - if (id > Database::CompressionAlgorithmMax) { - raiseError("Unsupported compression algorithm"); - } else { - m_db->setCompressionAlgo(static_cast(id)); - } + if (id > Database::CompressionAlgorithmMax) { + raiseError("Unsupported compression algorithm"); + return; } + m_db->setCompressionAlgo(static_cast(id)); } void Kdbx4Reader::setMasterSeed(const QByteArray& data) { if (data.size() != 32) { raiseError("Invalid master seed size"); - } else { - m_masterSeed = data; + return; } + m_masterSeed = data; } void Kdbx4Reader::setEncryptionIV(const QByteArray& data) @@ -505,15 +502,15 @@ void Kdbx4Reader::setInnerRandomStreamID(const QByteArray& data) { if (data.size() != 4) { raiseError("Invalid random stream id size"); - } else { - quint32 id = Endian::bytesToSizedInt(data, KeePass2::BYTEORDER); - KeePass2::ProtectedStreamAlgo irsAlgo = KeePass2::idToProtectedStreamAlgo(id); - if (irsAlgo == KeePass2::InvalidProtectedStreamAlgo || irsAlgo == KeePass2::ArcFourVariant) { - raiseError("Invalid inner random stream cipher"); - } else { - m_irsAlgo = irsAlgo; - } + return; + } + auto id = Endian::bytesToSizedInt(data, KeePass2::BYTEORDER); + KeePass2::ProtectedStreamAlgo irsAlgo = KeePass2::idToProtectedStreamAlgo(id); + if (irsAlgo == KeePass2::ProtectedStreamAlgo::InvalidProtectedStreamAlgo || irsAlgo == KeePass2::ProtectedStreamAlgo::ArcFourVariant) { + raiseError("Invalid inner random stream cipher"); + return; } + m_irsAlgo = irsAlgo; } QHash Kdbx4Reader::binaryPool() diff --git a/src/format/Kdbx4Reader.h b/src/format/Kdbx4Reader.h index 0375209c4..9d8c531ef 100644 --- a/src/format/Kdbx4Reader.h +++ b/src/format/Kdbx4Reader.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010 Felix Geyer + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/src/format/Kdbx4Writer.cpp b/src/format/Kdbx4Writer.cpp index 49d04c853..8b503c73a 100644 --- a/src/format/Kdbx4Writer.cpp +++ b/src/format/Kdbx4Writer.cpp @@ -1,5 +1,4 @@ /* - * Copyright (C) 2010 Felix Geyer * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify @@ -34,8 +33,6 @@ #include "streams/QtIOCompressor" #include "streams/SymmetricCipherStream.h" -#define CHECK_RETURN_FALSE(x) if (!(x)) return false; - Kdbx4Writer::Kdbx4Writer() : m_device(nullptr) { @@ -63,7 +60,7 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db) QByteArray startBytes; QByteArray endOfHeader = "\r\n\r\n"; - if (db->challengeMasterSeed(masterSeed) == false) { + if (!db->challengeMasterSeed(masterSeed)) { raiseError(tr("Unable to issue challenge-response.")); return false; } @@ -88,12 +85,12 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db) CHECK_RETURN_FALSE(writeData(Endian::sizedIntToBytes(KeePass2::SIGNATURE_1, KeePass2::BYTEORDER))); CHECK_RETURN_FALSE(writeData(Endian::sizedIntToBytes(KeePass2::SIGNATURE_2, KeePass2::BYTEORDER))); CHECK_RETURN_FALSE(writeData(Endian::sizedIntToBytes(KeePass2::FILE_VERSION_4, KeePass2::BYTEORDER))); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::CipherID, db->cipher().toByteArray())); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::CompressionFlags, + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::CipherID, db->cipher().toByteArray())); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::CompressionFlags, Endian::sizedIntToBytes(static_cast(db->compressionAlgo()), KeePass2::BYTEORDER))); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::MasterSeed, masterSeed)); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::EncryptionIV, encryptionIV)); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::MasterSeed, masterSeed)); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::EncryptionIV, encryptionIV)); // Convert current Kdf to basic parameters QVariantMap kdfParams = KeePass2::kdfToParameters(db->kdf()); @@ -104,12 +101,12 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db) return false; } QByteArray publicCustomData = db->publicCustomData(); - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::KdfParameters, kdfParamBytes)); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::KdfParameters, kdfParamBytes)); if (!publicCustomData.isEmpty()) { - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::PublicCustomData, publicCustomData)); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::PublicCustomData, publicCustomData)); } - CHECK_RETURN_FALSE(writeHeaderField(KeePass2::EndOfHeader, endOfHeader)); + CHECK_RETURN_FALSE(writeHeaderField(KeePass2::HeaderFieldID::EndOfHeader, endOfHeader)); header.close(); m_device = device; headerData = header.data(); @@ -161,7 +158,7 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db) QHash idMap; CHECK_RETURN_FALSE(writeInnerHeaderField(KeePass2::InnerHeaderFieldID::InnerRandomStreamID, - Endian::sizedIntToBytes(static_cast(KeePass2::ChaCha20), + Endian::sizedIntToBytes(static_cast(KeePass2::ProtectedStreamAlgo::ChaCha20), KeePass2::BYTEORDER))); CHECK_RETURN_FALSE(writeInnerHeaderField(KeePass2::InnerHeaderFieldID::InnerRandomStreamKey, protectedStreamKey)); @@ -180,7 +177,7 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db) } CHECK_RETURN_FALSE(writeInnerHeaderField(KeePass2::InnerHeaderFieldID::End, QByteArray())); - KeePass2RandomStream randomStream(KeePass2::ChaCha20); + KeePass2RandomStream randomStream(KeePass2::ProtectedStreamAlgo::ChaCha20); if (!randomStream.init(protectedStreamKey)) { raiseError(randomStream.errorString()); return false; @@ -217,15 +214,13 @@ bool Kdbx4Writer::writeData(const QByteArray& data) raiseError(m_device->errorString()); return false; } - else { - return true; - } + return true; } bool Kdbx4Writer::writeHeaderField(KeePass2::HeaderFieldID fieldId, const QByteArray& data) { QByteArray fieldIdArr; - fieldIdArr[0] = fieldId; + fieldIdArr[0] = static_cast(fieldId); CHECK_RETURN_FALSE(writeData(fieldIdArr)); CHECK_RETURN_FALSE(writeData(Endian::sizedIntToBytes(static_cast(data.size()), KeePass2::BYTEORDER))); CHECK_RETURN_FALSE(writeData(data)); @@ -264,47 +259,46 @@ bool Kdbx4Writer::serializeVariantMap(const QVariantMap& p, QByteArray& o) bool ok; QList keys = p.keys(); - for (int i = 0; i < keys.size(); ++i) { - QString k = keys.at(i); + for (const auto& k : keys) { KeePass2::VariantMapFieldType fieldType; QByteArray data; QVariant v = p.value(k); switch (static_cast(v.type())) { - case QMetaType::Type::Int: - fieldType = KeePass2::VariantMapFieldType::Int32; - data = Endian::sizedIntToBytes(v.toInt(&ok), KeePass2::BYTEORDER); - CHECK_RETURN_FALSE(ok); - break; - case QMetaType::Type::UInt: - fieldType = KeePass2::VariantMapFieldType::UInt32; - data = Endian::sizedIntToBytes(v.toUInt(&ok), KeePass2::BYTEORDER); - CHECK_RETURN_FALSE(ok); - break; - case QMetaType::Type::LongLong: - fieldType = KeePass2::VariantMapFieldType::Int64; - data = Endian::sizedIntToBytes(v.toLongLong(&ok), KeePass2::BYTEORDER); - CHECK_RETURN_FALSE(ok); - break; - case QMetaType::Type::ULongLong: - fieldType = KeePass2::VariantMapFieldType::UInt64; - data = Endian::sizedIntToBytes(v.toULongLong(&ok), KeePass2::BYTEORDER); - CHECK_RETURN_FALSE(ok); - break; - case QMetaType::Type::QString: - fieldType = KeePass2::VariantMapFieldType::String; - data = v.toString().toUtf8(); - break; - case QMetaType::Type::Bool: - fieldType = KeePass2::VariantMapFieldType::Bool; - data = QByteArray(1, (v.toBool() ? '\1' : '\0')); - break; - case QMetaType::Type::QByteArray: - fieldType = KeePass2::VariantMapFieldType::ByteArray; - data = v.toByteArray(); - break; - default: - qWarning("Unknown object type %d in QVariantMap", v.type()); - return false; + case QMetaType::Type::Int: + fieldType = KeePass2::VariantMapFieldType::Int32; + data = Endian::sizedIntToBytes(v.toInt(&ok), KeePass2::BYTEORDER); + CHECK_RETURN_FALSE(ok); + break; + case QMetaType::Type::UInt: + fieldType = KeePass2::VariantMapFieldType::UInt32; + data = Endian::sizedIntToBytes(v.toUInt(&ok), KeePass2::BYTEORDER); + CHECK_RETURN_FALSE(ok); + break; + case QMetaType::Type::LongLong: + fieldType = KeePass2::VariantMapFieldType::Int64; + data = Endian::sizedIntToBytes(v.toLongLong(&ok), KeePass2::BYTEORDER); + CHECK_RETURN_FALSE(ok); + break; + case QMetaType::Type::ULongLong: + fieldType = KeePass2::VariantMapFieldType::UInt64; + data = Endian::sizedIntToBytes(v.toULongLong(&ok), KeePass2::BYTEORDER); + CHECK_RETURN_FALSE(ok); + break; + case QMetaType::Type::QString: + fieldType = KeePass2::VariantMapFieldType::String; + data = v.toString().toUtf8(); + break; + case QMetaType::Type::Bool: + fieldType = KeePass2::VariantMapFieldType::Bool; + data = QByteArray(1, static_cast(v.toBool() ? '\1' : '\0')); + break; + case QMetaType::Type::QByteArray: + fieldType = KeePass2::VariantMapFieldType::ByteArray; + data = v.toByteArray(); + break; + default: + qWarning("Unknown object type %d in QVariantMap", v.type()); + return false; } QByteArray typeBytes; typeBytes[0] = static_cast(fieldType); diff --git a/src/format/Kdbx4Writer.h b/src/format/Kdbx4Writer.h index 4e703324d..7bb20e0f1 100644 --- a/src/format/Kdbx4Writer.h +++ b/src/format/Kdbx4Writer.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010 Felix Geyer + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/src/format/Kdbx4XmlReader.cpp b/src/format/Kdbx4XmlReader.cpp index 10dfe6475..9f6ce02c3 100644 --- a/src/format/Kdbx4XmlReader.cpp +++ b/src/format/Kdbx4XmlReader.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010 Felix Geyer + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -19,12 +19,9 @@ #include #include -#include #include "core/Endian.h" -#include "core/Database.h" #include "core/DatabaseIcons.h" -#include "core/Group.h" #include "core/Metadata.h" #include "core/Tools.h" #include "format/KeePass2RandomStream.h" @@ -68,30 +65,32 @@ void Kdbx4XmlReader::readDatabase(QIODevice* device, Database* db, KeePass2Rando m_randomStream = randomStream; m_headerHash.clear(); - m_tmpParent = new Group(); + m_tmpParent.reset(new Group()); bool rootGroupParsed = false; - if (!m_xml.hasError() && m_xml.readNextStartElement()) { - if (m_xml.name() == "KeePassFile") { - rootGroupParsed = parseKeePassFile(); - } + if (m_xml.hasError()) { + raiseError(QString("XML parsing failure: %1").arg(m_xml.error())); + return; + } + + if (m_xml.readNextStartElement() && m_xml.name() == "KeePassFile") { + rootGroupParsed = parseKeePassFile(); } - if (!m_xml.hasError() && !rootGroupParsed) { + if (!rootGroupParsed) { raiseError("No root group"); + return; } - if (!m_xml.hasError()) { - if (!m_tmpParent->children().isEmpty()) { - qWarning("Kdbx4XmlReader::readDatabase: found %d invalid group reference(s)", - m_tmpParent->children().size()); - } + if (!m_tmpParent->children().isEmpty()) { + qWarning("Kdbx4XmlReader::readDatabase: found %d invalid group reference(s)", + m_tmpParent->children().size()); + } - if (!m_tmpParent->entries().isEmpty()) { - qWarning("Kdbx4XmlReader::readDatabase: found %d invalid entry reference(s)", - m_tmpParent->children().size()); - } + if (!m_tmpParent->entries().isEmpty()) { + qWarning("Kdbx4XmlReader::readDatabase: found %d invalid entry reference(s)", + m_tmpParent->children().size()); } const QSet poolKeys = m_binaryPool.keys().toSet(); @@ -100,13 +99,11 @@ void Kdbx4XmlReader::readDatabase(QIODevice* device, Database* db, KeePass2Rando const QSet unusedKeys = poolKeys - entryKeys; if (!unmappedKeys.isEmpty()) { - raiseError("Unmapped keys left."); + qWarning("Unmapped keys left."); } - if (!m_xml.hasError()) { - for (const QString& key : unusedKeys) { - qWarning("Kdbx4XmlReader::readDatabase: found unused key \"%s\"", qPrintable(key)); - } + for (const QString& key : unusedKeys) { + qWarning("Kdbx4XmlReader::readDatabase: found unused key \"%s\"", qPrintable(key)); } QHash >::const_iterator i; @@ -131,13 +128,11 @@ void Kdbx4XmlReader::readDatabase(QIODevice* device, Database* db, KeePass2Rando histEntry->setUpdateTimeinfo(true); } } - - delete m_tmpParent; } Database* Kdbx4XmlReader::readDatabase(QIODevice* device) { - Database* db = new Database(); + auto db = new Database(); readDatabase(device, db); return db; } @@ -158,14 +153,16 @@ QString Kdbx4XmlReader::errorString() { if (m_error) { return m_errorStr; - } else if (m_xml.hasError()) { + } + + if (m_xml.hasError()) { return QString("XML error:\n%1\nLine %2, column %3") .arg(m_xml.errorString()) .arg(m_xml.lineNumber()) .arg(m_xml.columnNumber()); - } else { - return QString(); } + + return QString(); } void Kdbx4XmlReader::raiseError(const QString& errorMessage) @@ -189,17 +186,21 @@ bool Kdbx4XmlReader::parseKeePassFile() while (!m_xml.hasError() && m_xml.readNextStartElement()) { if (m_xml.name() == "Meta") { parseMeta(); - } else if (m_xml.name() == "Root") { + continue; + } + + if (m_xml.name() == "Root") { if (rootElementFound) { rootParsedSuccessfully = false; - raiseError("Multiple root elements"); + qWarning("Multiple root elements"); } else { rootParsedSuccessfully = parseRoot(); rootElementFound = true; } - } else { - skipCurrentElement(); + continue; } + + skipCurrentElement(); } return rootParsedSuccessfully; @@ -259,14 +260,14 @@ void Kdbx4XmlReader::parseMeta() if (value >= -1) { m_meta->setHistoryMaxItems(value); } else { - raiseError("HistoryMaxItems invalid number"); + qWarning("HistoryMaxItems invalid number"); } } else if (m_xml.name() == "HistoryMaxSize") { int value = readNumber(); if (value >= -1) { m_meta->setHistoryMaxSize(value); } else { - raiseError("HistoryMaxSize invalid number"); + qWarning("HistoryMaxSize invalid number"); } } else if (m_xml.name() == "Binaries") { parseBinaries(); @@ -337,9 +338,10 @@ void Kdbx4XmlReader::parseIcon() if (uuidSet && iconSet) { m_meta->addCustomIcon(uuid, icon); - } else { - raiseError("Missing icon uuid or data"); + return; } + + raiseError("Missing icon uuid or data"); } void Kdbx4XmlReader::parseBinaries() @@ -347,27 +349,28 @@ void Kdbx4XmlReader::parseBinaries() Q_ASSERT(m_xml.isStartElement() && m_xml.name() == "Binaries"); while (!m_xml.hasError() && m_xml.readNextStartElement()) { - if (m_xml.name() == "Binary") { - QXmlStreamAttributes attr = m_xml.attributes(); + if (m_xml.name() != "Binary") { + skipCurrentElement(); + continue; + } - QString id = attr.value("ID").toString(); + QXmlStreamAttributes attr = m_xml.attributes(); - QByteArray data; - if (attr.value("Compressed").compare(QLatin1String("True"), Qt::CaseInsensitive) == 0) { - data = readCompressedBinary(); - } else { - data = readBinary(); - } + QString id = attr.value("ID").toString(); - if (m_binaryPool.contains(id)) { - qWarning("Kdbx4XmlReader::parseBinaries: overwriting binary item \"%s\"", - qPrintable(id)); - } - - m_binaryPool.insert(id, data); + QByteArray data; + if (attr.value("Compressed").compare(QLatin1String("True"), Qt::CaseInsensitive) == 0) { + data = readCompressedBinary(); } else { - skipCurrentElement(); + data = readBinary(); } + + if (m_binaryPool.contains(id)) { + qWarning("Kdbx4XmlReader::parseBinaries: overwriting binary item \"%s\"", + qPrintable(id)); + } + + m_binaryPool.insert(id, data); } } @@ -378,9 +381,9 @@ void Kdbx4XmlReader::parseCustomData() while (!m_xml.hasError() && m_xml.readNextStartElement()) { if (m_xml.name() == "Item") { parseCustomDataItem(); - } else { - skipCurrentElement(); + continue; } + skipCurrentElement(); } } @@ -407,9 +410,10 @@ void Kdbx4XmlReader::parseCustomDataItem() if (keySet && valueSet) { m_meta->addCustomField(key, value); - } else { - raiseError("Missing custom data key or value"); + return; } + + raiseError("Missing custom data key or value"); } bool Kdbx4XmlReader::parseRoot() @@ -450,7 +454,7 @@ Group* Kdbx4XmlReader::parseGroup() { Q_ASSERT(m_xml.isStartElement() && m_xml.name() == "Group"); - Group* group = new Group(); + auto group = new Group(); group->setUpdateTimeinfo(false); QList children; QList entries; @@ -466,11 +470,17 @@ Group* Kdbx4XmlReader::parseGroup() } else { group->setUuid(uuid); } - } else if (m_xml.name() == "Name") { + continue; + } + if (m_xml.name() == "Name") { group->setName(readString()); - } else if (m_xml.name() == "Notes") { + continue; + } + if (m_xml.name() == "Notes") { group->setNotes(readString()); - } else if (m_xml.name() == "IconID") { + continue; + } + if (m_xml.name() == "IconID") { int iconId = readNumber(); if (iconId < 0) { if (m_strictMode) { @@ -483,18 +493,28 @@ Group* Kdbx4XmlReader::parseGroup() } group->setIcon(iconId); - } else if (m_xml.name() == "CustomIconUUID") { + continue; + } + if (m_xml.name() == "CustomIconUUID") { Uuid uuid = readUuid(); if (!uuid.isNull()) { group->setIcon(uuid); } - } else if (m_xml.name() == "Times") { + continue; + } + if (m_xml.name() == "Times") { group->setTimeInfo(parseTimes()); - } else if (m_xml.name() == "IsExpanded") { + continue; + } + if (m_xml.name() == "IsExpanded") { group->setExpanded(readBool()); - } else if (m_xml.name() == "DefaultAutoTypeSequence") { + continue; + } + if (m_xml.name() == "DefaultAutoTypeSequence") { group->setDefaultAutoTypeSequence(readString()); - } else if (m_xml.name() == "EnableAutoType") { + continue; + } + if (m_xml.name() == "EnableAutoType") { QString str = readString(); if (str.compare("null", Qt::CaseInsensitive) == 0) { @@ -506,7 +526,9 @@ Group* Kdbx4XmlReader::parseGroup() } else { raiseError("Invalid EnableAutoType value"); } - } else if (m_xml.name() == "EnableSearching") { + continue; + } + if (m_xml.name() == "EnableSearching") { QString str = readString(); if (str.compare("null", Qt::CaseInsensitive) == 0) { @@ -518,21 +540,28 @@ Group* Kdbx4XmlReader::parseGroup() } else { raiseError("Invalid EnableSearching value"); } - } else if (m_xml.name() == "LastTopVisibleEntry") { + continue; + } + if (m_xml.name() == "LastTopVisibleEntry") { group->setLastTopVisibleEntry(getEntry(readUuid())); - } else if (m_xml.name() == "Group") { + continue; + } + if (m_xml.name() == "Group") { Group* newGroup = parseGroup(); if (newGroup) { children.append(newGroup); } - } else if (m_xml.name() == "Entry") { + continue; + } + if (m_xml.name() == "Entry") { Entry* newEntry = parseEntry(false); if (newEntry) { entries.append(newEntry); } - } else { - skipCurrentElement(); + continue; } + + skipCurrentElement(); } if (group->uuid().isNull() && !m_strictMode) { @@ -577,7 +606,7 @@ void Kdbx4XmlReader::parseDeletedObject() { Q_ASSERT(m_xml.isStartElement() && m_xml.name() == "DeletedObject"); - DeletedObject delObj; + DeletedObject delObj{}; while (!m_xml.hasError() && m_xml.readNextStartElement()) { if (m_xml.name() == "UUID") { @@ -586,19 +615,24 @@ void Kdbx4XmlReader::parseDeletedObject() if (m_strictMode) { raiseError("Null DeleteObject uuid"); } - } else { - delObj.uuid = uuid; + continue; } - } else if (m_xml.name() == "DeletionTime") { + delObj.uuid = uuid; + continue; + } + if (m_xml.name() == "DeletionTime") { delObj.deletionTime = readDateTime(); - } else { - skipCurrentElement(); + continue; } + skipCurrentElement(); } if (!delObj.uuid.isNull() && !delObj.deletionTime.isNull()) { m_db->addDeletedObject(delObj); - } else if (m_strictMode) { + return; + } + + if (m_strictMode) { raiseError("Missing DeletedObject uuid or time"); } } @@ -607,7 +641,7 @@ Entry* Kdbx4XmlReader::parseEntry(bool history) { Q_ASSERT(m_xml.isStartElement() && m_xml.name() == "Entry"); - Entry* entry = new Entry(); + auto entry = new Entry(); entry->setUpdateTimeinfo(false); QList historyItems; QList binaryRefs; @@ -624,7 +658,9 @@ Entry* Kdbx4XmlReader::parseEntry(bool history) } else { entry->setUuid(uuid); } - } else if (m_xml.name() == "IconID") { + continue; + } + if (m_xml.name() == "IconID") { int iconId = readNumber(); if (iconId < 0) { if (m_strictMode) { @@ -633,39 +669,59 @@ Entry* Kdbx4XmlReader::parseEntry(bool history) iconId = 0; } entry->setIcon(iconId); - } else if (m_xml.name() == "CustomIconUUID") { + continue; + } + if (m_xml.name() == "CustomIconUUID") { Uuid uuid = readUuid(); if (!uuid.isNull()) { entry->setIcon(uuid); } - } else if (m_xml.name() == "ForegroundColor") { + continue; + }if (m_xml.name() == "ForegroundColor") { entry->setForegroundColor(readColor()); - } else if (m_xml.name() == "BackgroundColor") { + continue; + } + if (m_xml.name() == "BackgroundColor") { entry->setBackgroundColor(readColor()); - } else if (m_xml.name() == "OverrideURL") { + continue; + } + if (m_xml.name() == "OverrideURL") { entry->setOverrideUrl(readString()); - } else if (m_xml.name() == "Tags") { + continue; + } + if (m_xml.name() == "Tags") { entry->setTags(readString()); - } else if (m_xml.name() == "Times") { + continue; + } + if (m_xml.name() == "Times") { entry->setTimeInfo(parseTimes()); - } else if (m_xml.name() == "String") { + continue; + } + if (m_xml.name() == "String") { parseEntryString(entry); - } else if (m_xml.name() == "Binary") { + continue; + } + if (m_xml.name() == "Binary") { QPair ref = parseEntryBinary(entry); if (!ref.first.isNull() && !ref.second.isNull()) { binaryRefs.append(ref); } - } else if (m_xml.name() == "AutoType") { + continue; + } + if (m_xml.name() == "AutoType") { parseAutoType(entry); - } else if (m_xml.name() == "History") { + continue; + } + if (m_xml.name() == "History") { if (history) { raiseError("History element in history entry"); } else { historyItems = parseEntryHistory(); } - } else { - skipCurrentElement(); + continue; } + + skipCurrentElement(); } if (entry->uuid().isNull() && !m_strictMode) { @@ -720,7 +776,10 @@ void Kdbx4XmlReader::parseEntryString(Entry* entry) if (m_xml.name() == "Key") { key = readString(); keySet = true; - } else if (m_xml.name() == "Value") { + continue; + } + + if (m_xml.name() == "Value") { QXmlStreamAttributes attr = m_xml.attributes(); value = readString(); @@ -740,26 +799,29 @@ void Kdbx4XmlReader::parseEntryString(Entry* entry) } } else { raiseError("Unable to decrypt entry string"); + continue; } } protect = isProtected || protectInMemory; valueSet = true; - } else { - skipCurrentElement(); + continue; } + + skipCurrentElement(); } if (keySet && valueSet) { // the default attributes are always there so additionally check if it's empty if (entry->attributes()->hasKey(key) && !entry->attributes()->value(key).isEmpty()) { raiseError("Duplicate custom attribute found"); - } else { - entry->attributes()->set(key, value, protect); + return; } - } else { - raiseError("Entry string key or value missing"); + entry->attributes()->set(key, value, protect); + return; } + + raiseError("Entry string key or value missing"); } QPair Kdbx4XmlReader::parseEntryBinary(Entry* entry) @@ -777,7 +839,9 @@ QPair Kdbx4XmlReader::parseEntryBinary(Entry* entry) if (m_xml.name() == "Key") { key = readString(); keySet = true; - } else if (m_xml.name() == "Value") { + continue; + } + if (m_xml.name() == "Value") { QXmlStreamAttributes attr = m_xml.attributes(); if (attr.hasAttribute("Ref")) { @@ -797,9 +861,9 @@ QPair Kdbx4XmlReader::parseEntryBinary(Entry* entry) } valueSet = true; - } else { - skipCurrentElement(); + continue; } + skipCurrentElement(); } if (keySet && valueSet) { @@ -856,9 +920,9 @@ void Kdbx4XmlReader::parseAutoTypeAssoc(Entry* entry) if (windowSet && sequenceSet) { entry->autoTypeAssociations()->add(assoc); - } else { - raiseError("Auto-type association window or sequence missing"); + return; } + raiseError("Auto-type association window or sequence missing"); } QList Kdbx4XmlReader::parseEntryHistory() @@ -917,14 +981,15 @@ bool Kdbx4XmlReader::readBool() if (str.compare("True", Qt::CaseInsensitive) == 0) { return true; - } else if (str.compare("False", Qt::CaseInsensitive) == 0) { - return false; - } else if (str.length() == 0) { + } + if (str.compare("False", Qt::CaseInsensitive) == 0) { return false; - } else { - raiseError("Invalid bool value"); + } + if (str.length() == 0) { return false; } + raiseError("Invalid bool value"); + return false; } QDateTime Kdbx4XmlReader::readDateTime() @@ -936,18 +1001,18 @@ QDateTime Kdbx4XmlReader::readDateTime() QByteArray secsBytes = QByteArray::fromBase64(str.toUtf8()).leftJustified(8, '\0', true).left(8); qint64 secs = Endian::bytesToSizedInt(secsBytes, KeePass2::BYTEORDER); return QDateTime(QDate(1, 1, 1), QTime(0, 0, 0, 0), Qt::UTC).addSecs(secs); - } else { - QDateTime dt = QDateTime::fromString(str, Qt::ISODate); - if (dt.isValid()) { - return dt; - } else { - if (m_strictMode) { - raiseError("Invalid date time value"); - } + } - return QDateTime::currentDateTimeUtc(); - } + QDateTime dt = QDateTime::fromString(str, Qt::ISODate); + if (dt.isValid()) { + return dt; } + + if (m_strictMode) { + raiseError("Invalid date time value"); + } + + return QDateTime::currentDateTimeUtc(); } QColor Kdbx4XmlReader::readColor() @@ -955,26 +1020,26 @@ QColor Kdbx4XmlReader::readColor() QString colorStr = readString(); if (colorStr.isEmpty()) { - return QColor(); + return {}; } if (colorStr.length() != 7 || colorStr[0] != '#') { if (m_strictMode) { raiseError("Invalid color value"); } - return QColor(); + return {}; } QColor color; - for (int i = 0; i <= 2; i++) { - QString rgbPartStr = colorStr.mid(1 + 2*i, 2); + for (int i = 0; i <= 2; ++i) { + QString rgbPartStr = colorStr.mid(1 + 2 * i, 2); bool ok; int rgbPart = rgbPartStr.toInt(&ok, 16); if (!ok || rgbPart > 255) { if (m_strictMode) { raiseError("Invalid color rgb part"); } - return QColor(); + return {}; } if (i == 0) { @@ -1003,15 +1068,15 @@ Uuid Kdbx4XmlReader::readUuid() { QByteArray uuidBin = readBinary(); if (uuidBin.isEmpty()) { - return Uuid(); - } else if (uuidBin.length() != Uuid::Length) { + return {}; + } + if (uuidBin.length() != Uuid::Length) { if (m_strictMode) { raiseError("Invalid uuid value"); } - return Uuid(); - } else { - return Uuid(uuidBin); + return {}; } + return Uuid(uuidBin); } QByteArray Kdbx4XmlReader::readBinary() @@ -1045,14 +1110,14 @@ Group* Kdbx4XmlReader::getGroup(const Uuid& uuid) if (m_groups.contains(uuid)) { return m_groups.value(uuid); - } else { - Group* group = new Group(); - group->setUpdateTimeinfo(false); - group->setUuid(uuid); - group->setParent(m_tmpParent); - m_groups.insert(uuid, group); - return group; } + + auto group = new Group(); + group->setUpdateTimeinfo(false); + group->setUuid(uuid); + group->setParent(m_tmpParent.data()); + m_groups.insert(uuid, group); + return group; } Entry* Kdbx4XmlReader::getEntry(const Uuid& uuid) @@ -1063,14 +1128,14 @@ Entry* Kdbx4XmlReader::getEntry(const Uuid& uuid) if (m_entries.contains(uuid)) { return m_entries.value(uuid); - } else { - Entry* entry = new Entry(); - entry->setUpdateTimeinfo(false); - entry->setUuid(uuid); - entry->setGroup(m_tmpParent); - m_entries.insert(uuid, entry); - return entry; } + + auto entry = new Entry(); + entry->setUpdateTimeinfo(false); + entry->setUuid(uuid); + entry->setGroup(m_tmpParent.data()); + m_entries.insert(uuid, entry); + return entry; } void Kdbx4XmlReader::skipCurrentElement() diff --git a/src/format/Kdbx4XmlReader.h b/src/format/Kdbx4XmlReader.h index 6a0a6d4f4..229ac7425 100644 --- a/src/format/Kdbx4XmlReader.h +++ b/src/format/Kdbx4XmlReader.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010 Felix Geyer + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -27,10 +27,10 @@ #include "core/TimeInfo.h" #include "core/Uuid.h" +#include "core/Group.h" class Database; class Entry; -class Group; class KeePass2RandomStream; class Metadata; @@ -88,7 +88,7 @@ private: KeePass2RandomStream* m_randomStream; Database* m_db; Metadata* m_meta; - Group* m_tmpParent; + QScopedPointer m_tmpParent; QHash m_groups; QHash m_entries; QHash m_binaryPool; diff --git a/src/format/Kdbx4XmlWriter.cpp b/src/format/Kdbx4XmlWriter.cpp index 374744563..5c99186ca 100644 --- a/src/format/Kdbx4XmlWriter.cpp +++ b/src/format/Kdbx4XmlWriter.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010 Felix Geyer + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/src/format/Kdbx4XmlWriter.h b/src/format/Kdbx4XmlWriter.h index 79f27c98b..17d872580 100644 --- a/src/format/Kdbx4XmlWriter.h +++ b/src/format/Kdbx4XmlWriter.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010 Felix Geyer + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/src/format/KeePass2.cpp b/src/format/KeePass2.cpp index f89e828a1..88286a051 100644 --- a/src/format/KeePass2.cpp +++ b/src/format/KeePass2.cpp @@ -89,7 +89,8 @@ QSharedPointer KeePass2::uuidToKdf(const Uuid& uuid) { if (uuid == KDF_AES) { return QSharedPointer::create(); - } else if (uuid == KDF_ARGON2) { + } + if (uuid == KDF_ARGON2) { return QSharedPointer::create(); } @@ -100,13 +101,13 @@ QSharedPointer KeePass2::uuidToKdf(const Uuid& uuid) KeePass2::ProtectedStreamAlgo KeePass2::idToProtectedStreamAlgo(quint32 id) { switch (id) { - case static_cast(KeePass2::ArcFourVariant): - return KeePass2::ArcFourVariant; - case static_cast(KeePass2::Salsa20): - return KeePass2::Salsa20; - case static_cast(KeePass2::ChaCha20): - return KeePass2::ChaCha20; + case static_cast(KeePass2::ProtectedStreamAlgo::ArcFourVariant): + return KeePass2::ProtectedStreamAlgo::ArcFourVariant; + case static_cast(KeePass2::ProtectedStreamAlgo::Salsa20): + return KeePass2::ProtectedStreamAlgo::Salsa20; + case static_cast(KeePass2::ProtectedStreamAlgo::ChaCha20): + return KeePass2::ProtectedStreamAlgo::ChaCha20; default: - return KeePass2::InvalidProtectedStreamAlgo; + return KeePass2::ProtectedStreamAlgo::InvalidProtectedStreamAlgo; } } diff --git a/src/format/KeePass2.h b/src/format/KeePass2.h index cdc594f5a..f7fa0d397 100644 --- a/src/format/KeePass2.h +++ b/src/format/KeePass2.h @@ -65,7 +65,7 @@ namespace KeePass2 extern const QList> CIPHERS; extern const QList> KDFS; - enum HeaderFieldID + enum class HeaderFieldID { EndOfHeader = 0, Comment = 1, @@ -90,7 +90,7 @@ namespace KeePass2 Binary = 3 }; - enum ProtectedStreamAlgo + enum class ProtectedStreamAlgo { ArcFourVariant = 1, Salsa20 = 2, diff --git a/src/format/KeePass2RandomStream.cpp b/src/format/KeePass2RandomStream.cpp index 9e3d4cbce..26824b7e5 100644 --- a/src/format/KeePass2RandomStream.cpp +++ b/src/format/KeePass2RandomStream.cpp @@ -123,9 +123,9 @@ bool KeePass2RandomStream::loadBlock() SymmetricCipher::Algorithm KeePass2RandomStream::mapAlgo(KeePass2::ProtectedStreamAlgo algo) { switch (algo) { - case KeePass2::ChaCha20: + case KeePass2::ProtectedStreamAlgo::ChaCha20: return SymmetricCipher::ChaCha20; - case KeePass2::Salsa20: + case KeePass2::ProtectedStreamAlgo::Salsa20: return SymmetricCipher::Salsa20; default: return SymmetricCipher::InvalidAlgorithm; diff --git a/src/format/KeePass2Reader.cpp b/src/format/KeePass2Reader.cpp index 0a04c79c6..c213b4a18 100644 --- a/src/format/KeePass2Reader.cpp +++ b/src/format/KeePass2Reader.cpp @@ -30,7 +30,7 @@ BaseKeePass2Reader::BaseKeePass2Reader() : m_error(false) , m_saveXml(false) - , m_irsAlgo(KeePass2::InvalidProtectedStreamAlgo) + , m_irsAlgo(KeePass2::ProtectedStreamAlgo::InvalidProtectedStreamAlgo) { m_errorStr.clear(); m_xmlData.clear(); diff --git a/src/format/KeePass2Writer.h b/src/format/KeePass2Writer.h index 85802eb44..f6ec129f2 100644 --- a/src/format/KeePass2Writer.h +++ b/src/format/KeePass2Writer.h @@ -27,6 +27,8 @@ #include "core/Database.h" #include "format/KeePass2.h" +#define CHECK_RETURN_FALSE(x) if (!(x)) return false; + class BaseKeePass2Writer { public: diff --git a/src/gui/DatabaseSettingsWidget.cpp b/src/gui/DatabaseSettingsWidget.cpp index 0c2eba796..c59f97423 100644 --- a/src/gui/DatabaseSettingsWidget.cpp +++ b/src/gui/DatabaseSettingsWidget.cpp @@ -134,7 +134,7 @@ void DatabaseSettingsWidget::save() { // first perform safety check for KDF rounds auto kdf = KeePass2::uuidToKdf(Uuid(m_uiEncryption->kdfComboBox->currentData().toByteArray())); - if (kdf->uuid() == KeePass2::KDF_ARGON2 and m_uiEncryption->transformRoundsSpinBox->value() > 1000) { + if (kdf->uuid() == KeePass2::KDF_ARGON2 && m_uiEncryption->transformRoundsSpinBox->value() > 1000) { QMessageBox warning; warning.setIcon(QMessageBox::Warning); warning.setWindowTitle(tr("Number of rounds too high")); @@ -147,7 +147,7 @@ void DatabaseSettingsWidget::save() if (warning.clickedButton() != ok) { return; } - } else if (kdf->uuid() == KeePass2::KDF_AES and m_uiEncryption->transformRoundsSpinBox->value() < 100000) { + } else if (kdf->uuid() == KeePass2::KDF_AES && m_uiEncryption->transformRoundsSpinBox->value() < 100000) { QMessageBox warning; warning.setIcon(QMessageBox::Warning); warning.setWindowTitle(tr("Number of rounds too low")); diff --git a/src/streams/HmacBlockStream.cpp b/src/streams/HmacBlockStream.cpp index 677921a60..780db98c1 100644 --- a/src/streams/HmacBlockStream.cpp +++ b/src/streams/HmacBlockStream.cpp @@ -57,10 +57,8 @@ bool HmacBlockStream::reset() // Write final block(s) only if device is writable and we haven't // already written a final block. if (isWritable() && (!m_buffer.isEmpty() || m_blockIndex != 0)) { - if (!m_buffer.isEmpty()) { - if (!writeHashedBlock()) { - return false; - } + if (!m_buffer.isEmpty() && !writeHashedBlock()) { + return false; } // write empty final block @@ -106,15 +104,14 @@ qint64 HmacBlockStream::readData(char* data, qint64 maxSize) if (!readHashedBlock()) { if (m_error) { return -1; - } else { - return maxSize - bytesRemaining; } + return maxSize - bytesRemaining; } } - int bytesToCopy = qMin(bytesRemaining, static_cast(m_buffer.size() - m_bufferPos)); + qint64 bytesToCopy = qMin(bytesRemaining, static_cast(m_buffer.size() - m_bufferPos)); - memcpy(data + offset, m_buffer.constData() + m_bufferPos, bytesToCopy); + memcpy(data + offset, m_buffer.constData() + m_bufferPos, static_cast(bytesToCopy)); offset += bytesToCopy; m_bufferPos += bytesToCopy; @@ -142,7 +139,7 @@ bool HmacBlockStream::readHashedBlock() setErrorString("Invalid block size size."); return false; } - qint32 blockSize = Endian::bytesToSizedInt(blockSizeBytes, ByteOrder); + auto blockSize = Endian::bytesToSizedInt(blockSizeBytes, ByteOrder); if (blockSize < 0) { m_error = true; setErrorString("Invalid block size."); @@ -169,7 +166,7 @@ bool HmacBlockStream::readHashedBlock() } m_bufferPos = 0; - m_blockIndex++; + ++m_blockIndex; if (blockSize == 0) { m_eof = true; @@ -191,21 +188,18 @@ qint64 HmacBlockStream::writeData(const char* data, qint64 maxSize) qint64 offset = 0; while (bytesRemaining > 0) { - int bytesToCopy = qMin(bytesRemaining, static_cast(m_blockSize - m_buffer.size())); + qint64 bytesToCopy = qMin(bytesRemaining, static_cast(m_blockSize - m_buffer.size())); - m_buffer.append(data + offset, bytesToCopy); + m_buffer.append(data + offset, static_cast(bytesToCopy)); offset += bytesToCopy; bytesRemaining -= bytesToCopy; - if (m_buffer.size() == m_blockSize) { - if (!writeHashedBlock()) { - if (m_error) { - return -1; - } else { - return maxSize - bytesRemaining; - } + if (m_buffer.size() == m_blockSize && !writeHashedBlock()) { + if (m_error) { + return -1; } + return maxSize - bytesRemaining; } } @@ -242,7 +236,7 @@ bool HmacBlockStream::writeHashedBlock() m_buffer.clear(); } - m_blockIndex++; + ++m_blockIndex; return true; } diff --git a/src/streams/SymmetricCipherStream.cpp b/src/streams/SymmetricCipherStream.cpp index ece2642b5..78476c618 100644 --- a/src/streams/SymmetricCipherStream.cpp +++ b/src/streams/SymmetricCipherStream.cpp @@ -252,7 +252,6 @@ bool SymmetricCipherStream::writeBlock(bool lastBlock) int SymmetricCipherStream::blockSize() const { if (m_streamCipher) { return 1024; - } else { - return m_cipher->blockSize(); } + return m_cipher->blockSize(); } diff --git a/tests/TestKeePass2RandomStream.cpp b/tests/TestKeePass2RandomStream.cpp index bef7af540..53852e82d 100644 --- a/tests/TestKeePass2RandomStream.cpp +++ b/tests/TestKeePass2RandomStream.cpp @@ -58,7 +58,7 @@ void TestKeePass2RandomStream::test() } - KeePass2RandomStream randomStream(KeePass2::Salsa20); + KeePass2RandomStream randomStream(KeePass2::ProtectedStreamAlgo::Salsa20); bool ok; QVERIFY(randomStream.init(key)); QByteArray randomStreamData; diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index 449f13a06..bf237a854 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -900,11 +900,11 @@ void TestGui::testDatabaseSettings() QWidget* dbSettingsWidget = m_dbWidget->findChild("databaseSettingsWidget"); QSpinBox* transformRoundsSpinBox = dbSettingsWidget->findChild("transformRoundsSpinBox"); QVERIFY(transformRoundsSpinBox != nullptr); - transformRoundsSpinBox->setValue(100); + transformRoundsSpinBox->setValue(123456); QTest::keyClick(transformRoundsSpinBox, Qt::Key_Enter); // wait for modified timer QTRY_COMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("Save*")); - QCOMPARE(m_db->kdf()->rounds(), Q_UINT64_C(100)); + QCOMPARE(m_db->kdf()->rounds(), Q_UINT64_C(123456)); triggerAction("actionDatabaseSave"); QCOMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("Save")); -- cgit v1.2.3