Fix KDBX4 reader/writer attachment mapping error

Write duplicate attachments to the binary inner header only once and
skip duplicate entries when reading a KDBX4 file.
This fixes a an attachment mapping problem when an attachment appears
more than once in a database (which occurs frequently when editing attachment
entries and history is turned on)

src/format/Kdbx4Reader.cpp

@@ -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;
 }

src/format/Kdbx4Reader.h

@@ -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

src/format/Kdbx4Writer.cpp

@@ -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);
         }
     }
 }

src/format/KdbxXmlReader.cpp

@@ -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)
 {

src/format/KdbxXmlReader.h

@@ -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);