Use secure temp directory for remote sync (#10911)

share/translations/keepassxc_en.ts

@@ -2674,6 +2674,10 @@ Disable safe saves and try again?</source>
         <source>Do you want to remove the passkey from this entry?</source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <source>Could not upload the database. Remote handler was not initialized.</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>EditEntryWidget</name>
@@ -8960,6 +8964,14 @@ This option is deprecated, use --set-key-file instead.</source>
         <source>Failed to upload merged database. Command `%1` exited with status code: %2</source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <source>Could not create temporary directory &apos;%1&apos;</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Could not change permissions of temporary directory &apos;%1&apos; to owner</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>ReportsWidgetBrowserStatistics</name>

src/gui/DatabaseWidget.cpp

@@ -92,6 +92,7 @@ DatabaseWidget::DatabaseWidget(QSharedPointer<Database> db, QWidget* parent)
     , m_tagView(new TagView(this))
     , m_saveAttempts(0)
     , m_remoteSettings(new RemoteSettings(m_db, this))
+    , m_remoteHandler(nullptr)
     , m_entrySearcher(new EntrySearcher(false))
 {
     Q_ASSERT(m_db);
@@ -1085,7 +1086,7 @@ void DatabaseWidget::syncWithRemote(const RemoteParams* params)
     setDisabled(true);
     emit databaseSyncInProgress();
 
-    QScopedPointer<RemoteHandler> remoteHandler(new RemoteHandler(this));
+    m_remoteHandler.reset(new RemoteHandler(this));
     RemoteHandler::RemoteResult result;
     result.success = false;
     result.errorMessage = tr("Remote Sync did not contain any download or upload commands.");
@@ -1094,7 +1095,7 @@ void DatabaseWidget::syncWithRemote(const RemoteParams* params)
     if (!params->downloadCommand.isEmpty()) {
         emit updateSyncProgress(25, tr("Downloading..."));
         // Start a download first then merge and upload in the callback
-        result = remoteHandler->download(params);
+        result = m_remoteHandler->download(params);
         if (result.success) {
             QString error;
             QSharedPointer<Database> remoteDb = QSharedPointer<Database>::create();
@@ -1134,17 +1135,20 @@ void DatabaseWidget::syncDatabaseWithLockedDatabase(const QString& filePath, con
 
 void DatabaseWidget::uploadAndFinishSync(const RemoteParams* params, RemoteHandler::RemoteResult result)
 {
-    QScopedPointer<RemoteHandler> remoteHandler(new RemoteHandler(this));
-    if (result.success && !params->uploadCommand.isEmpty()) {
+    if (!m_remoteHandler) {
+        result.success = false;
+        result.errorMessage = tr("Could not upload the database. Remote handler was not initialized.");
+    } else if (result.success && !params->uploadCommand.isEmpty()) {
         emit updateSyncProgress(75, tr("Uploading..."));
-        result = remoteHandler->upload(result.filePath, params);
+        result = m_remoteHandler->upload(params);
     }
 
     finishSync(params, result);
 }
 
 void DatabaseWidget::finishSync(const RemoteParams* params, RemoteHandler::RemoteResult result)
 {
+    m_remoteHandler.reset();
     setDisabled(false);
     emit updateSyncProgress(-1, "");
     if (result.success) {

src/gui/DatabaseWidget.h

@@ -321,6 +321,7 @@ private slots:
     int m_saveAttempts;
 
     QScopedPointer<RemoteSettings> m_remoteSettings;
+    QScopedPointer<RemoteHandler> m_remoteHandler;
 
     // Search state
     QScopedPointer<EntrySearcher> m_entrySearcher;

src/gui/remote/DatabaseSettingsWidgetRemote.cpp

@@ -177,12 +177,12 @@ void DatabaseSettingsWidgetRemote::testDownload()
     params->downloadCommand = m_ui->downloadCommand->text();
     params->downloadInput = m_ui->inputForDownload->toPlainText();
 
-    QScopedPointer<RemoteHandler> remoteHandler(new RemoteHandler(this));
     if (params->downloadCommand.isEmpty()) {
         m_ui->messageWidget->showMessage(tr("Download command cannot be empty."), MessageWidget::Warning);
         return;
     }
 
+    QScopedPointer<RemoteHandler> remoteHandler(new RemoteHandler(this));
     RemoteHandler::RemoteResult result = remoteHandler->download(params);
     if (!result.success) {
         m_ui->messageWidget->showMessage(tr("Download failed with error: %1").arg(result.errorMessage),
@@ -197,4 +197,4 @@ void DatabaseSettingsWidgetRemote::testDownload()
     }
 
     m_ui->messageWidget->showMessage(tr("Download successful."), MessageWidget::Positive);
-}
+}

src/gui/remote/RemoteHandler.cpp

@@ -23,16 +23,6 @@
 #include "core/AsyncTask.h"
 #include "core/Database.h"
 
-namespace
-{
-    QString getTempFileLocation()
-    {
-        QString uuid = QUuid::createUuid().toString().remove(0, 1);
-        uuid.chop(1);
-        return QDir::toNativeSeparators(QDir::temp().absoluteFilePath("RemoteDatabase-" + uuid + ".kdbx"));
-    }
-} // namespace
-
 std::function<QScopedPointer<RemoteProcess>(QObject*)> RemoteHandler::m_createRemoteProcess([](QObject* parent) {
     return QScopedPointer<RemoteProcess>(new RemoteProcess(parent));
 });
@@ -42,24 +32,40 @@ RemoteHandler::RemoteHandler(QObject* parent)
 {
 }
 
+RemoteHandler::~RemoteHandler()
+{
+    QFileInfo file(m_tempFileLocation);
+    if (file.absoluteDir().exists() && file.absoluteDir().dirName().startsWith(PREFIX)) {
+        file.absoluteDir().removeRecursively();
+    }
+}
+
 void RemoteHandler::setRemoteProcessFunc(std::function<QScopedPointer<RemoteProcess>(QObject*)> func)
 {
     m_createRemoteProcess = std::move(func);
 }
 
 RemoteHandler::RemoteResult RemoteHandler::download(const RemoteParams* params)
 {
-    return AsyncTask::runAndWaitForFuture([params] {
+    return AsyncTask::runAndWaitForFuture([this, params] {
         RemoteResult result;
         if (!params) {
             result.success = false;
             result.errorMessage = tr("Invalid download parameters provided.");
             return result;
         }
 
-        auto filePath = getTempFileLocation();
+        QString error;
+        m_tempFileLocation = getTempFileLocation(&error);
+        result.filePath = m_tempFileLocation;
+        if (!error.isEmpty()) {
+            result.success = false;
+            result.errorMessage = error;
+            return result;
+        }
+
         auto remoteProcess = m_createRemoteProcess(nullptr); // use nullptr parent, otherwise there is a warning
-        remoteProcess->setTempFileLocation(filePath);
+        remoteProcess->setTempFileLocation(m_tempFileLocation);
         remoteProcess->start(params->downloadCommand);
         if (!params->downloadInput.isEmpty()) {
             remoteProcess->write(params->downloadInput + "\n");
@@ -76,13 +82,12 @@ RemoteHandler::RemoteResult RemoteHandler::download(const RemoteParams* params)
 
         if (finished && statusCode == 0) {
             // Check if the file actually downloaded
-            QFileInfo fileInfo(filePath);
+            QFileInfo fileInfo(m_tempFileLocation);
             if (!fileInfo.exists() || fileInfo.size() == 0) {
                 result.success = false;
                 result.errorMessage = tr("Command `%1` failed to download database.").arg(params->downloadCommand);
             } else {
                 result.success = true;
-                result.filePath = filePath;
             }
         } else if (finished) {
             result.success = false;
@@ -99,18 +104,19 @@ RemoteHandler::RemoteResult RemoteHandler::download(const RemoteParams* params)
     });
 }
 
-RemoteHandler::RemoteResult RemoteHandler::upload(const QString& filePath, const RemoteParams* params)
+RemoteHandler::RemoteResult RemoteHandler::upload(const RemoteParams* params)
 {
-    return AsyncTask::runAndWaitForFuture([filePath, params] {
+    return AsyncTask::runAndWaitForFuture([this, params] {
         RemoteResult result;
+        result.filePath = m_tempFileLocation;
         if (!params) {
             result.success = false;
             result.errorMessage = tr("Invalid database pointer or upload parameters provided.");
             return result;
         }
 
         auto remoteProcess = m_createRemoteProcess(nullptr); // use nullptr parent, otherwise there is a warning
-        remoteProcess->setTempFileLocation(filePath);
+        remoteProcess->setTempFileLocation(m_tempFileLocation);
         remoteProcess->start(params->uploadCommand);
         if (!params->uploadInput.isEmpty()) {
             remoteProcess->write(params->uploadInput + "\n");
@@ -143,3 +149,31 @@ RemoteHandler::RemoteResult RemoteHandler::upload(const QString& filePath, const
         return result;
     });
 }
+
+QString RemoteHandler::getTempFileLocation(QString* error)
+{
+    QString uuid = QUuid::createUuid().toString().remove(0, 1);
+    uuid.chop(1);
+    QString writableLocation = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation);
+    if (writableLocation.isEmpty()) {
+        writableLocation = QStandardPaths::writableLocation(QStandardPaths::TempLocation);
+    }
+
+    QString tempDirLocation = QDir(writableLocation).absoluteFilePath(PREFIX + uuid);
+    QString tempFileLocation =
+        QDir::toNativeSeparators(QDir(tempDirLocation).absoluteFilePath("RemoteDatabase-" + uuid + ".kdbx"));
+
+    if (!QDir().mkdir(tempDirLocation)) {
+        *error = tr("Could not create temporary directory '%1'").arg(tempDirLocation);
+        return "";
+    }
+
+    if (!QFile::setPermissions(tempDirLocation,
+                               QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner)) {
+        QDir(tempDirLocation).removeRecursively();
+        *error = tr("Could not change permissions of temporary directory '%1' to owner").arg(tempDirLocation);
+        return "";
+    }
+
+    return tempFileLocation;
+}

src/gui/remote/RemoteHandler.h

@@ -30,7 +30,7 @@ class RemoteHandler : public QObject
 
 public:
     explicit RemoteHandler(QObject* parent = nullptr);
-    ~RemoteHandler() override = default;
+    ~RemoteHandler() override;
 
     struct RemoteResult
     {
@@ -42,14 +42,18 @@ class RemoteHandler : public QObject
     };
 
     RemoteResult download(const RemoteParams* params);
-    RemoteResult upload(const QString& filePath, const RemoteParams* params);
+    RemoteResult upload(const RemoteParams* params);
 
     // Used for testing only
     static void setRemoteProcessFunc(std::function<QScopedPointer<RemoteProcess>(QObject*)> func);
 
 private:
+    QString m_tempFileLocation;
+
+    static QString getTempFileLocation(QString* error);
+
     static std::function<QScopedPointer<RemoteProcess>(QObject*)> m_createRemoteProcess;
-    static QString m_tempFileLocation;
+    inline static const QString PREFIX = "KPXC-Sync-";
 
     Q_DISABLE_COPY(RemoteHandler)
 };

src/gui/remote/RemoteProcess.cpp

@@ -17,7 +17,6 @@
 
 #include "RemoteProcess.h"
 
-#include <QTemporaryDir>
 #include <QUuid>
 
 RemoteProcess::RemoteProcess(QObject* parent)