Fix issues with reloading and handling of externally modified db file

src/CMakeLists.txt

@@ -195,6 +195,7 @@ set(keepassx_SOURCES
         keys/PasswordKey.cpp
         keys/ChallengeResponseKey.cpp
         streams/HashedBlockStream.cpp
+        streams/HashingStream.cpp
         streams/HmacBlockStream.cpp
         streams/LayeredStream.cpp
         streams/qtiocompressor.cpp

src/core/Database.cpp

@@ -25,6 +25,7 @@
 #include "format/KdbxXmlReader.h"
 #include "format/KeePass2Reader.h"
 #include "format/KeePass2Writer.h"
+#include "streams/HashingStream.h"
 
 #include <QFileInfo>
 #include <QJsonObject>
@@ -63,7 +64,7 @@ Database::Database()
     });
     connect(this, &Database::modified, this, [this] { updateTagList(); });
     connect(this, &Database::databaseSaved, this, [this]() { updateCommonUsernames(); });
-    connect(m_fileWatcher, &FileWatcher::fileChanged, this, &Database::databaseFileChanged);
+    connect(m_fileWatcher, &FileWatcher::fileChanged, this, [this] { emit Database::databaseFileChanged(false); });
 
     // static uuid map
     s_uuidMap.insert(m_uuid, this);
@@ -151,6 +152,20 @@ bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey>
 
     setEmitModified(false);
 
+    // update the hash of the first block
+    m_fileBlockHash.clear();
+    auto fileBlockData = dbFile.peek(kFileBlockToHashSizeBytes);
+    if (fileBlockData.size() != kFileBlockToHashSizeBytes) {
+        if (dbFile.size() >= kFileBlockToHashSizeBytes) {
+            if (error) {
+                *error = tr("Database file read error.");
+            }
+            return false;
+        }
+    } else {
+        m_fileBlockHash = QCryptographicHash::hash(fileBlockData, QCryptographicHash::Md5);
+    }
+
     KeePass2Reader reader;
     if (!reader.readDatabase(&dbFile, std::move(key), this)) {
         if (error) {
@@ -260,14 +275,33 @@ bool Database::saveAs(const QString& filePath, SaveAction action, const QString&
         return false;
     }
 
-    if (filePath == m_data.filePath) {
-        // Fail-safe check to make sure we don't overwrite underlying file changes
-        // that have not yet triggered a file reload/merge operation.
-        if (!m_fileWatcher->hasSameFileChecksum()) {
-            if (error) {
-                *error = tr("Database file has unmerged changes.");
+    // Make sure we don't overwrite external modifications unless explicitly allowed
+    if (!m_ignoreFileChangesUntilSaved && !m_fileBlockHash.isEmpty() && filePath == m_data.filePath) {
+        QFile dbFile(filePath);
+        if (dbFile.exists()) {
+            if (!dbFile.open(QIODevice::ReadOnly)) {
+                if (error) {
+                    *error = tr("Unable to open file %1.").arg(filePath);
+                }
+                return false;
+            }
+            auto fileBlockData = dbFile.read(kFileBlockToHashSizeBytes);
+            if (fileBlockData.size() == kFileBlockToHashSizeBytes) {
+                auto hash = QCryptographicHash::hash(fileBlockData, QCryptographicHash::Md5);
+                if (m_fileBlockHash != hash) {
+                    if (error) {
+                        *error = tr("Database file has unmerged changes.");
+                    }
+                    // emit the databaseFileChanged(true) signal async
+                    QMetaObject::invokeMethod(this, "databaseFileChanged", Qt::QueuedConnection, Q_ARG(bool, true));
+                    return false;
+                }
+            } else if(dbFile.size() >= kFileBlockToHashSizeBytes) {
+                if (error) {
+                    *error = tr("Database file read error.");
+                }
+                return false;
             }
-            return false;
         }
     }
 
@@ -302,7 +336,7 @@ bool Database::saveAs(const QString& filePath, SaveAction action, const QString&
             SetFileAttributes(realFilePath.toStdString().c_str(), FILE_ATTRIBUTE_HIDDEN);
         }
 #endif
-
+        m_ignoreFileChangesUntilSaved = false;
         m_fileWatcher->start(realFilePath, 30, 1);
     } else {
         // Saving failed, don't rewatch file since it does not represent our database
@@ -327,8 +361,12 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt
     case Atomic: {
         QSaveFile saveFile(filePath);
         if (saveFile.open(QIODevice::WriteOnly)) {
+            HashingStream hashingStream(&saveFile, QCryptographicHash::Md5, kFileBlockToHashSizeBytes);
+            if (!hashingStream.open(QIODevice::WriteOnly)) {
+                return false;
+            }
             // write the database to the file
-            if (!writeDatabase(&saveFile, error)) {
+            if (!writeDatabase(&hashingStream, error)) {
                 return false;
             }
 
@@ -338,6 +376,9 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt
 #endif
 
             if (saveFile.commit()) {
+                // store the new hash
+                m_fileBlockHash = hashingStream.hashingResult();
+
                 // successfully saved database file
                 return true;
             }
@@ -351,8 +392,12 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt
     case TempFile: {
         QTemporaryFile tempFile;
         if (tempFile.open()) {
+            HashingStream hashingStream(&tempFile, QCryptographicHash::Md5, kFileBlockToHashSizeBytes);
+            if (!hashingStream.open(QIODevice::WriteOnly)) {
+                return false;
+            }
             // write the database to the file
-            if (!writeDatabase(&tempFile, error)) {
+            if (!writeDatabase(&hashingStream, error)) {
                 return false;
             }
             tempFile.close(); // flush to disk
@@ -372,6 +417,8 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt
                 // Retain original creation time
                 tempFile.setFileTime(createTime, QFile::FileBirthTime);
 #endif
+                // store the new hash
+                m_fileBlockHash = hashingStream.hashingResult();
                 return true;
             } else if (backupFilePath.isEmpty() || !restoreDatabase(filePath, backupFilePath)) {
                 // Failed to copy new database in place, and
@@ -393,10 +440,16 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt
         // Open the original database file for direct-write
         QFile dbFile(filePath);
         if (dbFile.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
-            if (!writeDatabase(&dbFile, error)) {
+            HashingStream hashingStream(&dbFile, QCryptographicHash::Md5, kFileBlockToHashSizeBytes);
+            if (!hashingStream.open(QIODevice::WriteOnly)) {
+                return false;
+            }
+            if (!writeDatabase(&hashingStream, error)) {
                 return false;
             }
             dbFile.close();
+            // store the new hash
+            m_fileBlockHash = hashingStream.hashingResult();
             return true;
         }
         if (error) {
@@ -514,6 +567,9 @@ void Database::releaseData()
     m_deletedObjects.clear();
     m_commonUsernames.clear();
     m_tagList.clear();
+
+    m_fileBlockHash.clear();
+    m_ignoreFileChangesUntilSaved = false;
 }
 
 /**
@@ -650,10 +706,33 @@ void Database::setFilePath(const QString& filePath)
         m_data.filePath = filePath;
         // Don't watch for changes until the next open or save operation
         m_fileWatcher->stop();
+        m_ignoreFileChangesUntilSaved = false;
         emit filePathChanged(oldPath, filePath);
     }
 }
 
+const QByteArray& Database::fileBlockHash() const
+{
+    return m_fileBlockHash;
+}
+
+void Database::setIgnoreFileChangesUntilSaved(bool ignore)
+{
+    if (m_ignoreFileChangesUntilSaved != ignore) {
+        m_ignoreFileChangesUntilSaved = ignore;
+        if (ignore) {
+            m_fileWatcher->pause();
+        } else {
+            m_fileWatcher->resume();
+        }
+    }
+}
+
+bool Database::ignoreFileChangesUntilSaved() const
+{
+    return m_ignoreFileChangesUntilSaved;
+}
+
 QList<DeletedObject> Database::deletedObjects()
 {
     return m_deletedObjects;

src/core/Database.h

@@ -75,6 +75,9 @@ class Database : public ModifiableObject
     ~Database() override;
 
 private:
+    // size of the block of file data to hash for detecting external changes
+    static const quint32 kFileBlockToHashSizeBytes = 1024; // 1 KiB
+
     bool writeDatabase(QIODevice* device, QString* error = nullptr);
     bool backupDatabase(const QString& filePath, const QString& destinationFilePath);
     bool restoreDatabase(const QString& filePath, const QString& fromBackupFilePath);
@@ -108,6 +111,10 @@ class Database : public ModifiableObject
     QString canonicalFilePath() const;
     void setFilePath(const QString& filePath);
 
+    const QByteArray& fileBlockHash() const;
+    void setIgnoreFileChangesUntilSaved(bool ignore);
+    bool ignoreFileChangesUntilSaved() const;
+
     Metadata* metadata();
     const Metadata* metadata() const;
     Group* rootGroup();
@@ -171,7 +178,7 @@ public slots:
     void databaseOpened();
     void databaseSaved();
     void databaseDiscarded();
-    void databaseFileChanged();
+    void databaseFileChanged(bool triggeredBySave);
     void databaseNonDataChanged();
     void tagListUpdated();
 
@@ -223,6 +230,8 @@ public slots:
     void startModifiedTimer();
     void stopModifiedTimer();
 
+    QByteArray m_fileBlockHash;
+    bool m_ignoreFileChangesUntilSaved;
     QPointer<Metadata> const m_metadata;
     DatabaseData m_data;
     QPointer<Group> m_rootGroup;

src/core/FileWatcher.cpp

@@ -79,17 +79,18 @@ void FileWatcher::stop()
     m_fileChecksum.clear();
     m_fileChecksumTimer.stop();
     m_fileChangeDelayTimer.stop();
+    m_paused = false;
 }
 
 void FileWatcher::pause()
 {
-    m_ignoreFileChange = true;
+    m_paused = true;
     m_fileChangeDelayTimer.stop();
 }
 
 void FileWatcher::resume()
 {
-    m_ignoreFileChange = false;
+    m_paused = false;
     // Add a short delay to start in the next event loop
     if (!m_fileIgnoreDelayTimer.isActive()) {
         m_fileIgnoreDelayTimer.start(0);
@@ -98,7 +99,7 @@ void FileWatcher::resume()
 
 bool FileWatcher::shouldIgnoreChanges()
 {
-    return m_filePath.isEmpty() || m_ignoreFileChange || m_fileIgnoreDelayTimer.isActive()
+    return m_filePath.isEmpty() || m_ignoreFileChange || m_paused || m_fileIgnoreDelayTimer.isActive()
            || m_fileChangeDelayTimer.isActive();
 }
 
@@ -116,16 +117,18 @@ void FileWatcher::checkFileChanged()
     // Prevent reentrance
     m_ignoreFileChange = true;
 
-    AsyncTask::runThenCallback([=] { return calculateChecksum(); },
-                               this,
-                               [=](QByteArray checksum) {
-                                   if (checksum != m_fileChecksum) {
-                                       m_fileChecksum = checksum;
-                                       m_fileChangeDelayTimer.start(0);
-                                   }
+    AsyncTask::runThenCallback(
+        [this] { return calculateChecksum(); },
+        this,
+        [this](QByteArray checksum) {
+            if (checksum != m_fileChecksum) {
+                m_fileChecksum = checksum;
+                m_fileChangeDelayTimer.start(0);
+            }
 
-                                   m_ignoreFileChange = false;
-                               });
+            m_ignoreFileChange = false;
+        }
+    );
 }
 
 QByteArray FileWatcher::calculateChecksum()

src/core/FileWatcher.h

@@ -56,6 +56,7 @@ private slots:
     QTimer m_fileChecksumTimer;
     int m_fileChecksumSizeBytes = -1;
     bool m_ignoreFileChange = false;
+    bool m_paused = false;
 };
 
 #endif // KEEPASSXC_FILEWATCHER_H

src/gui/DatabaseOpenDialog.cpp

@@ -192,11 +192,34 @@ void DatabaseOpenDialog::clearForms()
     m_tabBar->blockSignals(false);
 }
 
+void DatabaseOpenDialog::showMessage(const QString& text, MessageWidget::MessageType type, int autoHideTimeout)
+{
+    m_view->showMessage(text, type, autoHideTimeout);
+}
+
 QSharedPointer<Database> DatabaseOpenDialog::database() const
 {
     return m_db;
 }
 
+void DatabaseOpenDialog::done(int result)
+{
+    hide();
+
+    emit dialogFinished(result == QDialog::Accepted, m_currentDbWidget);
+    clearForms();
+
+    QDialog::done(result);
+
+#if QT_VERSION < QT_VERSION_CHECK(6, 3, 0)
+    // CDialogs are not really closed, just hidden, pre Qt 6.3?
+    if (testAttribute(Qt::WA_DeleteOnClose)) {
+        setAttribute(Qt::WA_DeleteOnClose, false);
+        deleteLater();
+    }
+#endif
+}
+
 void DatabaseOpenDialog::complete(bool accepted)
 {
     // save DB, since DatabaseOpenWidget will reset its data after accept() is called
@@ -207,7 +230,4 @@ void DatabaseOpenDialog::complete(bool accepted)
     } else {
         reject();
     }
-
-    emit dialogFinished(accepted, m_currentDbWidget);
-    clearForms();
 }

src/gui/DatabaseOpenDialog.h

@@ -19,6 +19,7 @@
 #define KEEPASSX_UNLOCKDATABASEDIALOG_H
 
 #include "core/Global.h"
+#include "gui/MessageWidget.h"
 
 #include <QDialog>
 #include <QList>
@@ -50,11 +51,13 @@ class DatabaseOpenDialog : public QDialog
     Intent intent() const;
     QSharedPointer<Database> database() const;
     void clearForms();
+    void showMessage(const QString& text, MessageWidget::MessageType type, int autoHideTimeout);
 
 signals:
     void dialogFinished(bool accepted, DatabaseWidget* dbWidget);
 
 public slots:
+    void done(int result) override;
     void complete(bool accepted);
     void tabChanged(int index);
 

src/gui/DatabaseOpenWidget.cpp

@@ -224,6 +224,11 @@ bool DatabaseOpenWidget::unlockingDatabase()
     return m_unlockingDatabase;
 }
 
+void DatabaseOpenWidget::showMessage(const QString& text, MessageWidget::MessageType type, int autoHideTimeout)
+{
+    m_ui->messageWidget->showMessage(text, type, autoHideTimeout);
+}
+
 void DatabaseOpenWidget::load(const QString& filename)
 {
     clearForms();