Backport Bug Fixes and Crash Fixes to 2.7.7

docs/topics/UserInterface.adoc

@@ -86,7 +86,7 @@ Additionally, the following environment variables may be useful when running the
 |KPXC_CONFIG                    | Override default path to roaming configuration file
 |KPXC_CONFIG_LOCAL              | Override default path to local configuration file
 |KPXC_INITIAL_DIR               | Override initial location picking for databases
-|SSH_AUTH_SOCKET                | Path of the unix file socket that the agent uses for communication with other processes (SSH Agent)
+|SSH_AUTH_SOCK                  | Path of the unix file socket that the agent uses for communication with other processes (SSH Agent)
 |QT_SCALE_FACTOR [numeric]      | Defines a global scale factor for the whole application, including point-sized fonts.
 |QT_SCREEN_SCALE_FACTORS [list] | Specifies scale factors for each screen. See https://doc.qt.io/qt-5/highdpi.html#high-dpi-support-in-qt
 |QT_SCALE_FACTOR_ROUNDING_POLICY | Control device pixel ratio rounding to the nearest integer. See https://doc.qt.io/qt-5/highdpi.html#high-dpi-support-in-qt

share/translations/keepassxc_en.ts

@@ -3468,10 +3468,6 @@ Supported extensions are: %1.</source>
         <source>Unable to fetch favicon.</source>
         <translation type="unfinished"></translation>
     </message>
-    <message>
-        <source>You can enable the DuckDuckGo website icon service under Tools -&gt; Settings -&gt; Security</source>
-        <translation type="unfinished"></translation>
-    </message>
     <message>
         <source>Existing icon selected.</source>
         <translation type="unfinished"></translation>
@@ -3513,6 +3509,10 @@ Supported extensions are: %1.</source>
             <numerusform></numerusform>
         </translation>
     </message>
+    <message>
+        <source>You can enable the DuckDuckGo website icon service under Application Settings -&gt; Security</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>EditWidgetProperties</name>

share/windows/wix-template.xml

@@ -92,6 +92,9 @@
                     <RegistryValue Root="HKCU" Key="Software\KeePassXC" Name="DesktopShortcut" Type="integer" Value="1" KeyPath="yes"/>
                 </Component>
             </Directory>
+
+            <!-- Windows system folder -->
+            <Directory Id="SystemFolder" Name="SystemFolder" />
         </DirectoryRef>
 
         <!-- Custom properties to control installation options -->
@@ -116,12 +119,17 @@
         <Property Id="WixShellExecTarget" Value="[#CM_FP_KeePassXC.exe]" />
         <CustomAction Id="LaunchApplication" BinaryKey="WixCA" DllEntry="WixShellExec" Impersonate="yes" />
 
-        <!-- Action to kill running KeePassXC processes -->
-        <Property Id="WixSilentExecCmdLine" Value='taskkill /IM KeePassXC.exe; taskkill /IM keepassxc-proxy.exe /F' />
-        <CustomAction Id="KillKeePassXC" BinaryKey="WixCA" DllEntry="WixSilentExec" Execute="immediate" Return="ignore" />
+        <!-- Action to kill running KeePassXC process -->
+        <Property Id="WixQuietExecCmdLine" Value='"[SystemFolder]taskkill.exe" /IM KeePassXC.exe' />
+        <CustomAction Id="KillKeePassXC" BinaryKey="WixCA" DllEntry="WixQuietExec" Execute="immediate" Return="ignore" />
+
+        <!-- Action to kill running keepassxc-proxy processes -->
+        <Property Id="WixSilentExecCmdLine" Value='"[SystemFolder]taskkill.exe" /IM keepassxc-proxy.exe /F' />
+        <CustomAction Id="KillKeePassXCProxy" BinaryKey="WixCA" DllEntry="WixSilentExec" Execute="immediate" Return="ignore" />
 
         <InstallExecuteSequence>
             <Custom Action="KillKeePassXC" Before="InstallValidate" />
+            <Custom Action="KillKeePassXCProxy" Before="InstallValidate" />
             <!-- Prevent pinned taskbar shortcut from being removed -->
             <RemoveShortcuts Suppress="yes" />
         </InstallExecuteSequence>

src/CMakeLists.txt

@@ -59,6 +59,7 @@ set(keepassx_SOURCES
         core/TimeDelta.cpp
         core/TimeInfo.cpp
         core/Tools.cpp
+        core/Totp.cpp
         core/Translator.cpp
         core/UrlTools.cpp
         cli/Utils.cpp
@@ -193,8 +194,7 @@ set(keepassx_SOURCES
         streams/LayeredStream.cpp
         streams/qtiocompressor.cpp
         streams/StoreDataStream.cpp
-        streams/SymmetricCipherStream.cpp
-        totp/totp.cpp)
+        streams/SymmetricCipherStream.cpp)
 if(APPLE)
     set(keepassx_SOURCES
             ${keepassx_SOURCES}

src/core/Entry.cpp

@@ -24,7 +24,7 @@
 #include "core/Metadata.h"
 #include "core/PasswordHealth.h"
 #include "core/Tools.h"
-#include "totp/totp.h"
+#include "core/Totp.h"
 
 #include <QDir>
 #include <QRegularExpression>
@@ -566,7 +566,7 @@ void Entry::setTotp(QSharedPointer<Totp::Settings> settings)
     m_attributes->remove(Totp::ATTRIBUTE_SEED);
     m_attributes->remove(Totp::ATTRIBUTE_SETTINGS);
 
-    if (settings->key.isEmpty()) {
+    if (!settings || settings->key.isEmpty()) {
         m_data.totpSettings.reset();
     } else {
         m_data.totpSettings = std::move(settings);
@@ -1279,11 +1279,11 @@ void Entry::setGroup(Group* group, bool trackPrevious)
         }
     }
 
+    QObject::setParent(group);
+
     m_group = group;
     group->addEntry(this);
 
-    QObject::setParent(group);
-
     if (m_updateTimeinfo) {
         m_data.timeInfo.setLocationChanged(Clock::currentDateTimeUtc());
     }

src/core/Merger.cpp

@@ -338,6 +338,7 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex
     const int comparison = compare(targetEntry->timeInfo().lastModificationTime(),
                                    sourceEntry->timeInfo().lastModificationTime(),
                                    CompareItemIgnoreMilliseconds);
+    const int maxItems = targetEntry->database()->metadata()->historyMaxItems();
     if (comparison < 0) {
         Group* currentGroup = targetEntry->group();
         Entry* clonedEntry = sourceEntry->clone(Entry::CloneIncludeHistory);
@@ -346,15 +347,15 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex
                qPrintable(sourceEntry->title()),
                qPrintable(currentGroup->name()));
         changes << tr("Synchronizing from newer source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex());
-        moveEntry(clonedEntry, currentGroup);
-        mergeHistory(targetEntry, clonedEntry, mergeMethod);
+        mergeHistory(targetEntry, clonedEntry, mergeMethod, maxItems);
         eraseEntry(targetEntry);
+        moveEntry(clonedEntry, currentGroup);
     } else {
         qDebug("Merge %s/%s with local on top/under %s",
                qPrintable(targetEntry->title()),
                qPrintable(sourceEntry->title()),
                qPrintable(targetEntry->group()->name()));
-        const bool changed = mergeHistory(sourceEntry, targetEntry, mergeMethod);
+        const bool changed = mergeHistory(sourceEntry, targetEntry, mergeMethod, maxItems);
         if (changed) {
             changes
                 << tr("Synchronizing from older source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex());
@@ -400,7 +401,10 @@ Merger::resolveEntryConflict(const MergeContext& context, const Entry* sourceEnt
     return changes;
 }
 
-bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod)
+bool Merger::mergeHistory(const Entry* sourceEntry,
+                          Entry* targetEntry,
+                          Group::MergeMode mergeMethod,
+                          const int maxItems)
 {
     Q_UNUSED(mergeMethod);
     const auto targetHistoryItems = targetEntry->historyItems();
@@ -473,7 +477,6 @@ bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::M
     }
 
     bool changed = false;
-    const int maxItems = targetEntry->database()->metadata()->historyMaxItems();
     const auto updatedHistoryItems = merged.values();
     for (int i = 0; i < maxItems; ++i) {
         const Entry* oldEntry = targetHistoryItems.value(targetHistoryItems.count() - i);

src/core/Merger.h

@@ -50,7 +50,7 @@ class Merger : public QObject
     ChangeList mergeDeletions(const MergeContext& context);
     ChangeList mergeMetadata(const MergeContext& context);
     bool markOlderEntry(Entry* entry);
-    bool mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod);
+    bool mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod, const int maxItems);
     void moveEntry(Entry* entry, Group* targetGroup);
     void moveGroup(Group* group, Group* targetGroup);
     // remove an entry without a trace in the deletedObjects - needed for elemination cloned entries

src/core/PasswordGenerator.cpp

@@ -261,8 +261,10 @@ QVector<PasswordGroup> PasswordGenerator::passwordGroups() const
     if (!m_custom.isEmpty()) {
         PasswordGroup group;
 
-        for (auto ch : m_custom) {
-            group.append(ch);
+        for (const auto& ch : m_custom) {
+            if (!group.contains(ch)) {
+                group.append(ch);
+            }
         }
 
         passwordGroups.append(group);

src/totp/totp.cpp → src/core/Totp.cpp

@@ -16,7 +16,7 @@
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "totp.h"
+#include "Totp.h"
 
 #include "core/Base32.h"
 #include "core/Clock.h"
@@ -59,6 +59,11 @@ static QString getNameForHashType(const Totp::Algorithm hashType)
 
 QSharedPointer<Totp::Settings> Totp::parseSettings(const QString& rawSettings, const QString& key)
 {
+    // Early out if both strings are empty
+    if (rawSettings.isEmpty() && key.isEmpty()) {
+        return {};
+    }
+
     // Create default settings
     auto settings = createSettings(key, DEFAULT_DIGITS, DEFAULT_STEP);
 
@@ -96,6 +101,11 @@ QSharedPointer<Totp::Settings> Totp::parseSettings(const QString& rawSettings, c
                 settings->algorithm = getHashTypeByName(query.queryItemValue("otpHashMode"));
             }
         } else {
+            if (settings->key.isEmpty()) {
+                // Legacy format cannot work with an empty key
+                return {};
+            }
+
             // Parse semi-colon separated values ([step];[digits|S])
             settings->format = StorageFormat::LEGACY;
             auto vars = rawSettings.split(";");

src/fdosecrets/objects/Collection.cpp

@@ -495,6 +495,10 @@ namespace FdoSecrets
         }
 
         auto item = Item::Create(this, entry);
+        if (!item) {
+            return;
+        }
+
         m_items << item;
         m_entryToItem[entry] = item;
 

src/format/OpVaultReaderSections.cpp

@@ -18,7 +18,7 @@
 #include "OpVaultReader.h"
 
 #include "core/Entry.h"
-#include "totp/totp.h"
+#include "core/Totp.h"
 
 #include <QDebug>
 #include <QJsonArray>

src/gui/DatabaseOpenWidget.cpp

@@ -513,12 +513,12 @@ void DatabaseOpenWidget::hardwareKeyResponse(bool found)
 
 void DatabaseOpenWidget::openHardwareKeyHelp()
 {
-    QDesktopServices::openUrl(QUrl("https://keepassxc.org/docs#faq-cat-yubikey"));
+    QDesktopServices::openUrl(QUrl("https://keepassxc.org/docs/#faq-yubikey-2fa"));
 }
 
 void DatabaseOpenWidget::openKeyFileHelp()
 {
-    QDesktopServices::openUrl(QUrl("https://keepassxc.org/docs#faq-cat-keyfile"));
+    QDesktopServices::openUrl(QUrl("https://keepassxc.org/docs/#faq-keyfile-howto"));
 }
 
 void DatabaseOpenWidget::setUserInteractionLock(bool state)

src/gui/DatabaseWidget.cpp

@@ -493,6 +493,10 @@ void DatabaseWidget::setupTotp()
 
     auto setupTotpDialog = new TotpSetupDialog(this, currentEntry);
     connect(setupTotpDialog, SIGNAL(totpUpdated()), SIGNAL(entrySelectionChanged()));
+    if (currentWidget() == m_editEntryWidget) {
+        // Entry is being edited, tell it when we are finished updating TOTP
+        connect(setupTotpDialog, SIGNAL(totpUpdated()), m_editEntryWidget, SLOT(updateTotp()));
+    }
     connect(this, &DatabaseWidget::databaseLockRequested, setupTotpDialog, &TotpSetupDialog::close);
     setupTotpDialog->open();
 }
@@ -2151,7 +2155,7 @@ bool DatabaseWidget::performSave(QString& errorMessage, const QString& fileName)
     m_groupView->setDisabled(false);
     m_tagView->setDisabled(false);
 
-    if (focusWidget) {
+    if (focusWidget && focusWidget->isVisible()) {
         focusWidget->setFocus();
     }
 

src/gui/EditWidgetIcons.cpp

@@ -222,7 +222,7 @@ void EditWidgetIcons::iconReceived(const QString& url, const QImage& icon)
         QString message(tr("Unable to fetch favicon."));
         if (!config()->get(Config::Security_IconDownloadFallback).toBool()) {
             message.append("\n").append(
-                tr("You can enable the DuckDuckGo website icon service under Tools -> Settings -> Security"));
+                tr("You can enable the DuckDuckGo website icon service under Application Settings -> Security"));
         }
         emit messageEditEntry(message, MessageWidget::Error);
         return;

src/gui/EntryPreviewWidget.cpp

@@ -21,10 +21,10 @@
 
 #include "Application.h"
 #include "core/Config.h"
+#include "core/Totp.h"
 #include "gui/Clipboard.h"
 #include "gui/Font.h"
 #include "gui/Icons.h"
-#include "totp/totp.h"
 #if defined(WITH_XC_KEESHARE)
 #include "keeshare/KeeShare.h"
 #include "keeshare/KeeShareSettings.h"

src/gui/TotpDialog.cpp

@@ -20,9 +20,9 @@
 #include "ui_TotpDialog.h"
 
 #include "core/Clock.h"
+#include "core/Totp.h"
 #include "gui/Clipboard.h"
 #include "gui/MainWindow.h"
-#include "totp/totp.h"
 
 #include <QPushButton>
 #include <QShortcut>

src/gui/TotpExportSettingsDialog.cpp

@@ -17,11 +17,11 @@
 
 #include "TotpExportSettingsDialog.h"
 
+#include "core/Totp.h"
 #include "gui/Clipboard.h"
 #include "gui/MainWindow.h"
 #include "gui/SquareSvgWidget.h"
 #include "qrcode/QrCode.h"
-#include "totp/totp.h"
 
 #include <QBoxLayout>
 #include <QBuffer>

src/gui/TotpSetupDialog.cpp

@@ -19,8 +19,8 @@
 #include "ui_TotpSetupDialog.h"
 
 #include "core/Base32.h"
+#include "core/Totp.h"
 #include "gui/MessageBox.h"
-#include "totp/totp.h"
 
 TotpSetupDialog::TotpSetupDialog(QWidget* parent, Entry* entry)
     : QDialog(parent)

src/gui/csvImport/CsvImportWidget.cpp

@@ -22,9 +22,9 @@
 #include <QStringListModel>
 
 #include "core/Clock.h"
+#include "core/Totp.h"
 #include "format/KeePass2Writer.h"
 #include "gui/MessageBox.h"
-#include "totp/totp.h"
 
 // I wanted to make the CSV import GUI future-proof, so if one day you need a new field,
 // all you have to do is add a field to m_columnHeader, and the GUI will follow:
@@ -208,7 +208,7 @@ void CsvImportWidget::writeDatabase()
         auto otpString = m_parserModel->data(m_parserModel->index(r, 6));
         if (otpString.isValid() && !otpString.toString().isEmpty()) {
             auto totp = Totp::parseSettings(otpString.toString());
-            if (totp->key.isEmpty()) {
+            if (!totp || totp->key.isEmpty()) {
                 // Bare secret, use default TOTP settings
                 totp = Totp::parseSettings({}, otpString.toString());
             }

src/gui/entry/EditEntryWidget.cpp

@@ -114,6 +114,7 @@ EditEntryWidget::EditEntryWidget(QWidget* parent)
     m_entryModifiedTimer.setSingleShot(true);
     m_entryModifiedTimer.setInterval(0);
     connect(&m_entryModifiedTimer, &QTimer::timeout, this, [this] {
+        // TODO: Upon refactor of this widget, this needs to merge unsaved changes in the UI
         if (isVisible() && m_entry) {
             setForms(m_entry);
         }
@@ -704,6 +705,13 @@ void EditEntryWidget::toKeeAgentSettings(KeeAgentSettings& settings) const
     settings.setSaveAttachmentToTempFile(m_sshAgentSettings.saveAttachmentToTempFile());
 }
 
+void EditEntryWidget::updateTotp()
+{
+    if (m_entry) {
+        m_attributesModel->setEntryAttributes(m_entry->attributes());
+    }
+}
+
 void EditEntryWidget::browsePrivateKey()
 {
     auto fileName = fileDialog()->getOpenFileName(this, tr("Select private key"), FileDialog::getLastDir("sshagent"));
@@ -828,15 +836,15 @@ void EditEntryWidget::loadEntry(Entry* entry,
     m_create = create;
     m_history = history;
 
-    connect(m_entry, &Entry::modified, this, [this] { m_entryModifiedTimer.start(); });
-
     if (history) {
         setHeadline(QString("%1 \u2022 %2").arg(parentName, tr("Entry history")));
     } else {
         if (create) {
             setHeadline(QString("%1 \u2022 %2").arg(parentName, tr("Add entry")));
         } else {
             setHeadline(QString("%1 \u2022 %2 \u2022 %3").arg(parentName, entry->title(), tr("Edit entry")));
+            // Reload entry details if changed outside of the edit dialog
+            connect(m_entry, &Entry::modified, this, [this] { m_entryModifiedTimer.start(); });
         }
     }
 
@@ -1143,6 +1151,7 @@ bool EditEntryWidget::commitEntry()
     }
 
     m_historyModel->setEntries(m_entry->historyItems(), m_entry);
+    setPageHidden(m_historyWidget, m_history || m_entry->historyItems().count() < 1);
     m_advancedUi->attachmentsWidget->linkAttachments(m_entry->attachments());
 
     showMessage(tr("Entry updated successfully."), MessageWidget::Positive);

src/gui/entry/EditEntryWidget.h

@@ -118,6 +118,7 @@ private slots:
     void updateSSHAgentAttachment();
     void updateSSHAgentAttachments();
     void updateSSHAgentKeyInfo();
+    void updateTotp();
     void browsePrivateKey();
     void addKeyToAgent();
     void removeKeyFromAgent();