Merge pull request #1423 from keepassxreboot/feature/fix-kdbx4-attachment-mapping

Feature/fix kdbx4 attachment mapping
This commit is contained in:
Janek Bevendorff 2018-01-25 02:03:22 +01:00 committed by GitHub
commit 998fc9890e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 111 additions and 11 deletions

View File

@ -22,7 +22,6 @@
#include "core/Group.h"
#include "core/Endian.h"
#include "crypto/CryptoHash.h"
#include "crypto/kdf/AesKdf.h"
#include "format/KeePass2RandomStream.h"
#include "format/KdbxXmlReader.h"
#include "streams/HmacBlockStream.h"
@ -34,7 +33,7 @@ Database* Kdbx4Reader::readDatabaseImpl(QIODevice* device, const QByteArray& hea
{
Q_ASSERT(m_kdbxVersion == KeePass2::FILE_VERSION_4);
m_binaryPool.clear();
m_binaryPoolInverse.clear();
if (hasError()) {
return nullptr;
@ -135,7 +134,7 @@ Database* Kdbx4Reader::readDatabaseImpl(QIODevice* device, const QByteArray& hea
Q_ASSERT(xmlDevice);
KdbxXmlReader xmlReader(KeePass2::FILE_VERSION_4, m_binaryPool);
KdbxXmlReader xmlReader(KeePass2::FILE_VERSION_4, binaryPool());
xmlReader.readDatabase(xmlDevice, m_db.data(), &randomStream);
if (xmlReader.hasError()) {
@ -273,14 +272,20 @@ bool Kdbx4Reader::readInnerHeaderField(QIODevice* device)
setProtectedStreamKey(fieldData);
break;
case KeePass2::InnerHeaderFieldID::Binary:
case KeePass2::InnerHeaderFieldID::Binary: {
if (fieldLen < 1) {
raiseError(tr("Invalid inner header binary size"));
return false;
}
m_binaryPool.insert(QString::number(m_binaryPool.size()), fieldData.mid(1));
auto data = fieldData.mid(1);
if (m_binaryPoolInverse.contains(data)) {
qWarning("Skipping duplicate binary record");
break;
}
m_binaryPoolInverse.insert(data, QString::number(m_binaryPoolInverse.size()));
break;
}
}
return true;
}
@ -417,7 +422,22 @@ QVariantMap Kdbx4Reader::readVariantMap(QIODevice* device)
return vm;
}
/**
* @return mapping from attachment keys to binary data
*/
QHash<QString, QByteArray> Kdbx4Reader::binaryPool() const
{
return m_binaryPool;
QHash<QString, QByteArray> binaryPool;
for (auto it = m_binaryPoolInverse.cbegin(); it != m_binaryPoolInverse.cend(); ++it) {
binaryPool.insert(it.value(), it.key());
}
return binaryPool;
}
/**
* @return mapping from binary data to attachment keys
*/
QHash<QByteArray, QString> Kdbx4Reader::binaryPoolInverse() const
{
return m_binaryPoolInverse;
}

View File

@ -32,6 +32,7 @@ Q_DECLARE_TR_FUNCTIONS(Kdbx4Reader)
public:
Database* readDatabaseImpl(QIODevice* device, const QByteArray& headerData,
const CompositeKey& key, bool keepDatabase) override;
QHash<QByteArray, QString> binaryPoolInverse() const;
QHash<QString, QByteArray> binaryPool() const;
protected:
@ -41,7 +42,7 @@ private:
bool readInnerHeaderField(QIODevice* device);
QVariantMap readVariantMap(QIODevice* device);
QHash<QString, QByteArray> m_binaryPool;
QHash<QByteArray, QString> m_binaryPoolInverse;
};
#endif // KEEPASSX_KDBX4READER_H

View File

@ -211,12 +211,20 @@ bool Kdbx4Writer::writeInnerHeaderField(QIODevice* device, KeePass2::InnerHeader
void Kdbx4Writer::writeAttachments(QIODevice* device, Database* db)
{
const QList<Entry*> allEntries = db->rootGroup()->entriesRecursive(true);
QSet<QByteArray> writtenAttachments;
for (Entry* entry : allEntries) {
const QList<QString> attachmentKeys = entry->attachments()->keys();
for (const QString& key : attachmentKeys) {
QByteArray data = entry->attachments()->value(key);
data.prepend("\x01");
QByteArray data("\x01");
data.append(entry->attachments()->value(key));
if (writtenAttachments.contains(data)) {
continue;
}
writeInnerHeaderField(device, KeePass2::InnerHeaderFieldID::Binary, data);
writtenAttachments.insert(data);
}
}
}

View File

@ -40,7 +40,7 @@ KdbxXmlReader::KdbxXmlReader(quint32 version)
* @param version KDBX version
* @param binaryPool binary pool
*/
KdbxXmlReader::KdbxXmlReader(quint32 version, QHash<QString, QByteArray>& binaryPool)
KdbxXmlReader::KdbxXmlReader(quint32 version, const QHash<QString, QByteArray>& binaryPool)
: m_kdbxVersion(version)
, m_binaryPool(binaryPool)
{

View File

@ -42,7 +42,7 @@ Q_DECLARE_TR_FUNCTIONS(KdbxXmlReader)
public:
explicit KdbxXmlReader(quint32 version);
explicit KdbxXmlReader(quint32 version, QHash<QString, QByteArray>& binaryPool);
explicit KdbxXmlReader(quint32 version, const QHash<QString, QByteArray>& binaryPool);
virtual ~KdbxXmlReader() = default;
virtual Database* readDatabase(const QString& filename);

View File

@ -188,3 +188,73 @@ void TestKdbx4::testFormat400Upgrade_data()
QTest::newRow("AES-KDF + Twofish") << KeePass2::KDF_AES_KDBX4 << KeePass2::CIPHER_TWOFISH << kdbx4;
QTest::newRow("AES-KDF (legacy) + Twofish") << KeePass2::KDF_AES_KDBX3 << KeePass2::CIPHER_TWOFISH << kdbx3;
}
/**
* Test for catching mapping errors with duplicate attachments.
*/
void TestKdbx4::testDuplicateAttachments()
{
QScopedPointer<Database> db(new Database());
db->setKey(CompositeKey());
db->setKdf(KeePass2::uuidToKdf(KeePass2::KDF_ARGON2));
const QByteArray attachment1("abc");
const QByteArray attachment2("def");
const QByteArray attachment3("ghi");
auto entry1 = new Entry();
entry1->setGroup(db->rootGroup());
entry1->setUuid(Uuid("aaaaaaaaaaaaaaaa"));
entry1->attachments()->set("a", attachment1);
auto entry2 = new Entry();
entry2->setGroup(db->rootGroup());
entry2->setUuid(Uuid("bbbbbbbbbbbbbbbb"));
entry2->attachments()->set("b1", attachment1);
entry2->beginUpdate();
entry2->attachments()->set("b2", attachment1);
entry2->endUpdate();
entry2->beginUpdate();
entry2->attachments()->set("b3", attachment2);
entry2->endUpdate();
entry2->beginUpdate();
entry2->attachments()->set("b4", attachment2);
entry2->endUpdate();
auto entry3 = new Entry();
entry3->setGroup(db->rootGroup());
entry3->setUuid(Uuid("cccccccccccccccc"));
entry3->attachments()->set("c1", attachment2);
entry3->attachments()->set("c2", attachment2);
entry3->attachments()->set("c3", attachment3);
QBuffer buffer;
buffer.open(QBuffer::ReadWrite);
KeePass2Writer writer;
writer.writeDatabase(&buffer, db.data());
if (writer.hasError()) {
QFAIL(qPrintable(QString("Error while writing database: %1").arg(writer.errorString())));
}
buffer.seek(0);
KeePass2Reader reader;
db.reset(reader.readDatabase(&buffer, CompositeKey()));
if (reader.hasError()) {
QFAIL(qPrintable(QString("Error while reading database: %1").arg(reader.errorString())));
}
QCOMPARE(db->rootGroup()->entries()[0]->attachments()->value("a"), attachment1);
QCOMPARE(db->rootGroup()->entries()[1]->attachments()->value("b1"), attachment1);
QCOMPARE(db->rootGroup()->entries()[1]->attachments()->value("b2"), attachment1);
QCOMPARE(db->rootGroup()->entries()[1]->attachments()->value("b3"), attachment2);
QCOMPARE(db->rootGroup()->entries()[1]->attachments()->value("b4"), attachment2);
QCOMPARE(db->rootGroup()->entries()[1]->historyItems()[0]->attachments()->value("b1"), attachment1);
QCOMPARE(db->rootGroup()->entries()[1]->historyItems()[1]->attachments()->value("b2"), attachment1);
QCOMPARE(db->rootGroup()->entries()[1]->historyItems()[2]->attachments()->value("b3"), attachment2);
QCOMPARE(db->rootGroup()->entries()[2]->attachments()->value("c1"), attachment2);
QCOMPARE(db->rootGroup()->entries()[2]->attachments()->value("c2"), attachment2);
QCOMPARE(db->rootGroup()->entries()[2]->attachments()->value("c3"), attachment3);
}

View File

@ -28,6 +28,7 @@ private slots:
void testFormat400();
void testFormat400Upgrade();
void testFormat400Upgrade_data();
void testDuplicateAttachments();
protected:
void initTestCaseImpl() override;