Improve and secure attachment handling (fixes #2400).

Externally opened attachments are now lifecycle-managed properly.

The temporary files are created with stricter permissions and entirely
random names (except for the file extension) to prevent meta data leakage.

When the database is closed, the files are overwritten with random
data and are also more reliably deleted than before.

Changes to the temporary files are monitored and the user is asked
if they want to save the changes back to the database (fixes #3130).

KeePassXC does not keep a lock on any of the temporary files, resolving
long-standing issues with applications such as Adobe Acrobat on Windows
(fixes #5950, fixes #5839).

Internally, attachments are copied less. The EntryAttachmentsWidget
now only references EntryAttachments instead of owning a separate copy
(which used to not be cleared properly under certain circumstances).

src/core/EntryAttachments.cpp

@@ -18,14 +18,25 @@
 #include "EntryAttachments.h"
 
 #include "core/Global.h"
+#include "crypto/Random.h"
 
+#include <QDesktopServices>
+#include <QDir>
+#include <QProcessEnvironment>
 #include <QSet>
+#include <QTemporaryFile>
+#include <QUrl>
 
 EntryAttachments::EntryAttachments(QObject* parent)
     : ModifiableObject(parent)
 {
 }
 
+EntryAttachments::~EntryAttachments()
+{
+    clear();
+}
+
 QList<QString> EntryAttachments::keys() const
 {
     return m_attachments.keys();
@@ -82,6 +93,10 @@ void EntryAttachments::remove(const QString& key)
 
     m_attachments.remove(key);
 
+    if (m_openedAttachments.contains(key)) {
+        disconnectAndEraseExternalFile(m_openedAttachments.value(key));
+    }
+
     emit removed(key);
     emitModified();
 }
@@ -92,20 +107,17 @@ void EntryAttachments::remove(const QStringList& keys)
         return;
     }
 
+    bool emitStatus = modifiedSignalEnabled();
+    setEmitModified(false);
+
     bool isModified = false;
     for (const QString& key : keys) {
-        if (!m_attachments.contains(key)) {
-            Q_ASSERT_X(
-                false, "EntryAttachments::remove", qPrintable(QString("Can't find attachment for key %1").arg(key)));
-            continue;
-        }
-
-        isModified = true;
-        emit aboutToBeRemoved(key);
-        m_attachments.remove(key);
-        emit removed(key);
+        isModified |= m_attachments.contains(key);
+        remove(key);
     }
 
+    setEmitModified(emitStatus);
+
     if (isModified) {
         emitModified();
     }
@@ -133,15 +145,47 @@ void EntryAttachments::clear()
 
     m_attachments.clear();
 
+    const auto externalPath = m_openedAttachments.values();
+    for (auto& path : externalPath) {
+        disconnectAndEraseExternalFile(path);
+    }
+
     emit reset();
     emitModified();
 }
 
+void EntryAttachments::disconnectAndEraseExternalFile(const QString& path)
+{
+    if (m_openedAttachmentsInverse.contains(path)) {
+        m_attachmentFileWatchers.value(path)->stop();
+        m_attachmentFileWatchers.remove(path);
+
+        m_openedAttachments.remove(m_openedAttachmentsInverse.value(path));
+        m_openedAttachmentsInverse.remove(path);
+    }
+
+    QFile f(path);
+    if (f.open(QFile::ReadWrite)) {
+        qint64 blocks = f.size() / 128 + 1;
+        for (qint64 i = 0; i < blocks; ++i) {
+            f.write(randomGen()->randomArray(128));
+        }
+        f.close();
+    }
+    f.remove();
+}
+
 void EntryAttachments::copyDataFrom(const EntryAttachments* other)
 {
     if (*this != *other) {
         emit aboutToBeReset();
 
+        // Reset all externally opened files
+        const auto externalPath = m_openedAttachments.values();
+        for (auto& path : externalPath) {
+            disconnectAndEraseExternalFile(path);
+        }
+
         m_attachments = other->m_attachments;
 
         emit reset();
@@ -167,3 +211,54 @@ int EntryAttachments::attachmentsSize() const
     }
     return size;
 }
+
+bool EntryAttachments::openAttachment(const QString& key, QString* errorMessage)
+{
+    if (!m_openedAttachments.contains(key)) {
+        const QByteArray attachmentData = value(key);
+        auto ext = key.contains(".") ? "." + key.split(".").last() : "";
+
+#ifdef KEEPASSXC_DIST_SNAP
+        const QString tmpFileTemplate =
+            QString("%1/XXXXXXXXXXXX%2").arg(QProcessEnvironment::systemEnvironment().value("SNAP_USER_DATA"), ext);
+#else
+        const QString tmpFileTemplate = QDir::temp().absoluteFilePath(QString("XXXXXXXXXXXX").append(ext));
+#endif
+
+        QTemporaryFile tmpFile(tmpFileTemplate);
+
+        const bool saveOk = tmpFile.open() && tmpFile.setPermissions(QFile::ReadOwner | QFile::WriteOwner)
+                            && tmpFile.write(attachmentData) == attachmentData.size() && tmpFile.flush();
+
+        if (!saveOk && errorMessage) {
+            *errorMessage = tr("%1 - %2").arg(key, tmpFile.errorString());
+            return false;
+        }
+
+        tmpFile.close();
+        tmpFile.setAutoRemove(false);
+        m_openedAttachments.insert(key, tmpFile.fileName());
+        m_openedAttachmentsInverse.insert(tmpFile.fileName(), key);
+
+        auto watcher = QSharedPointer<FileWatcher>::create();
+        watcher->start(tmpFile.fileName(), 5);
+        connect(watcher.data(), &FileWatcher::fileChanged, this, &EntryAttachments::attachmentFileModified);
+        m_attachmentFileWatchers.insert(tmpFile.fileName(), watcher);
+    }
+
+    const bool openOk = QDesktopServices::openUrl(QUrl::fromLocalFile(m_openedAttachments.value(key)));
+    if (!openOk && errorMessage) {
+        *errorMessage = tr("Cannot open file \"%1\"").arg(key);
+        return false;
+    }
+
+    return true;
+}
+
+void EntryAttachments::attachmentFileModified(const QString& path)
+{
+    auto it = m_openedAttachmentsInverse.find(path);
+    if (it != m_openedAttachmentsInverse.end()) {
+        emit valueModifiedExternally(it.value(), path);
+    }
+}