From 4286fddc22fa1a579bb8073d8aaa9a378c1756db Mon Sep 17 00:00:00 2001 From: Nick Bolton Date: Mon, 27 Apr 2026 13:05:53 +0100 Subject: [PATCH 1/3] fix(cve): remove ipc command that creates privilege escalation vulnerability CVE-2026-41477 --- .gitignore | 3 + CLAUDE.md | 19 +++++ src/lib/deskflow/win32/DaemonApp.cpp | 84 ++++++++++++++++------ src/lib/deskflow/win32/DaemonApp.h | 6 +- src/lib/deskflow/win32/DaemonIpcServer.cpp | 50 ++++++++----- src/lib/deskflow/win32/DaemonIpcServer.h | 6 +- src/lib/gui/core/CoreProcess.cpp | 24 +++++-- src/lib/gui/ipc/DaemonIpcClient.cpp | 26 +++++-- src/lib/gui/ipc/DaemonIpcClient.h | 5 +- 9 files changed, 170 insertions(+), 53 deletions(-) create mode 100644 CLAUDE.md diff --git a/.gitignore b/.gitignore index 1723cb2a9..d403c8f8b 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,6 @@ deskflow-config.toml /scripts/*.egg-info /*.user *.ui.autosave + +# AI +.claude/settings.local.json diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..96fe80d1e --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,19 @@ +# Synergy — collaboration notes + +## Code style + +- Default to **no comments**. Only add one when the WHY is non-obvious — hidden constraints, subtle invariants, workarounds for specific bugs, behavior that would surprise a reader. Never add comments that explain WHAT or HOW (well-named identifiers do that). +- Don't reference the current task, fix, ticket, or CVE in comments. That belongs in the commit message. + +## Working with commits + +- **Never commit, amend, or rewrite history.** The user reviews staged changes and commits themselves. No `git commit`, no `git commit --amend`, no `git rebase`, no force-pushes. +- When cherry-picking, use `git cherry-pick --no-commit` so changes land in the index/working tree without producing a commit. Halt after conflict resolution and let the user commit. + +## Backporting from upstream Deskflow + +- Synergy is a downstream fork of [deskflow/deskflow](https://github.com/deskflow/deskflow). The `deskflow` git remote should already be configured. +- Branding is driven by `DESKFLOW_APP_ID = "synergy"` in `ext/synergy-extra/cmake/Extra.cmake` (the `synergy-extra` overlay). Most of `src/` is shared verbatim with deskflow upstream. +- Synergy doesn't track deskflow master continuously. Synergy is the more mature, stabilized branch: it cherry-picks critical updates (security fixes, important bugfixes) from Deskflow and occasionally re-forks from upstream as a beta. Deskflow master is the rolling dev branch with in-progress refactors; Synergy is the curated subset. +- Practical consequence: when porting from a deskflow PR, expect direct `git cherry-pick` to fail. Files may live at different paths than upstream (e.g. upstream may have moved `DaemonIpcServer.cpp` `win32/` → `ipc/` or `DaemonApp.cpp` `lib/` → `apps/` while Synergy still has the original location), and surrounding refactors may not be present. Plan to port hunks manually into the Synergy file locations. +- **Backport multi-commit PRs one commit at a time.** Apply, surface conflicts, resolve together, wait for the user to review/commit, then move to the next. diff --git a/src/lib/deskflow/win32/DaemonApp.cpp b/src/lib/deskflow/win32/DaemonApp.cpp index c21f6a3be..16df6ff17 100644 --- a/src/lib/deskflow/win32/DaemonApp.cpp +++ b/src/lib/deskflow/win32/DaemonApp.cpp @@ -27,6 +27,8 @@ #include #include +#include + using namespace std; using namespace deskflow::core; @@ -55,38 +57,63 @@ void DaemonApp::saveLogLevel(const QString &logLevel) const } } -void DaemonApp::setElevate(bool elevate) +void DaemonApp::setMode(const QString &mode) { - LOG_DEBUG("elevate value changed: %s", elevate ? "yes" : "no"); - m_elevate = elevate; + LOG_DEBUG("mode changed: %s", mode.toUtf8().constData()); + m_mode = mode; try { // saves setting for next time the daemon starts. - ARCH->setting("Elevate", std::string(elevate ? "1" : "0")); + ARCH->setting("Mode", mode.toStdString()); } catch (XArch &e) { - LOG_ERR("failed to save elevate setting: %s", e.what()); + LOG_ERR("failed to save mode setting: %s", e.what()); } } -void DaemonApp::setCommand(const QString &command) +void DaemonApp::setArgs(const QString &args) { - LOG_DEBUG("service command updated"); - m_command = command.toStdString(); + LOG_DEBUG("args updated"); + m_args = args; try { // saves setting for next time the daemon starts. - ARCH->setting("Command", command.toStdString()); + ARCH->setting("Args", args.toStdString()); } catch (XArch &e) { - LOG_ERR("failed to save command setting: %s", e.what()); + LOG_ERR("failed to save args setting: %s", e.what()); } } -void DaemonApp::applyWatchdogCommand() const +void DaemonApp::setElevate(bool elevate) { - LOG_DEBUG("applying watchdog command"); + LOG_DEBUG("elevate value changed: %s", elevate ? "yes" : "no"); + m_elevate = elevate; + try { + // saves setting for next time the daemon starts. + ARCH->setting("Elevate", std::string(elevate ? "1" : "0")); + } catch (XArch &e) { + LOG_ERR("failed to save elevate setting: %s", e.what()); + } +} + +void DaemonApp::applyWatchdogCommand() const +{ #if SYSAPI_WIN32 - m_pWatchdog->setProcessConfig(m_command, m_elevate); + QString binName; + if (m_mode == "server") { + binName = QStringLiteral(SERVER_BINARY_NAME ".exe"); + } else if (m_mode == "client") { + binName = QStringLiteral(CLIENT_BINARY_NAME ".exe"); + } else { + LOG_ERR("cannot apply watchdog command: invalid or unset mode: %s", m_mode.toUtf8().constData()); + return; + } + + const auto binPath = QStringLiteral("%1/%2").arg(QCoreApplication::applicationDirPath(), binName); + const auto command = QStringLiteral("\"%1\" %2").arg(binPath, m_args).toStdString(); + + LOG_DEBUG("applying watchdog command (elevate: %s)", m_elevate ? "yes" : "no"); + m_pWatchdog->setProcessConfig(command, m_elevate); #else LOG_ERR("applying watchdog command not implemented on this platform"); #endif @@ -96,8 +123,16 @@ void DaemonApp::clearWatchdogCommand() { LOG_DEBUG("clearing watchdog command"); - // Clear the setting to prevent it from being next time the daemon starts. - setCommand(""); + m_mode.clear(); + m_args.clear(); + m_elevate = false; + try { + ARCH->setting("Mode", std::string()); + ARCH->setting("Args", std::string()); + ARCH->setting("Elevate", std::string("0")); + } catch (XArch &e) { + LOG_ERR("failed to clear watchdog settings: %s", e.what()); + } #if SYSAPI_WIN32 m_pWatchdog->setProcessConfig("", false); @@ -121,11 +156,15 @@ void DaemonApp::connectIpcServer(const ipc::DaemonIpcServer *ipcServer) const Qt::DirectConnection ); QObject::connect( - ipcServer, &ipc::DaemonIpcServer::elevateModeChanged, this, &DaemonApp::setElevate, // + ipcServer, &ipc::DaemonIpcServer::modeChanged, this, &DaemonApp::setMode, // Qt::DirectConnection ); QObject::connect( - ipcServer, &ipc::DaemonIpcServer::commandChanged, this, &DaemonApp::setCommand, // + ipcServer, &ipc::DaemonIpcServer::argsChanged, this, &DaemonApp::setArgs, // + Qt::DirectConnection + ); + QObject::connect( + ipcServer, &ipc::DaemonIpcServer::elevateModeChanged, this, &DaemonApp::setElevate, // Qt::DirectConnection ); QObject::connect( @@ -180,11 +219,12 @@ void DaemonApp::run(QThread &daemonThread) #if SYSAPI_WIN32 m_pWatchdog = std::make_unique(m_foreground, *m_pFileLogOutputter); - std::string command = ARCH->setting("Command"); - bool elevate = ARCH->setting("Elevate") == "1"; - if (!command.empty()) { - LOG_DEBUG("using last known command: %s", command.c_str()); - m_pWatchdog->setProcessConfig(command, elevate); + m_mode = QString::fromStdString(ARCH->setting("Mode")); + m_args = QString::fromStdString(ARCH->setting("Args")); + m_elevate = ARCH->setting("Elevate") == "1"; + if (!m_mode.isEmpty()) { + LOG_DEBUG("using last known mode: %s", m_mode.toUtf8().constData()); + applyWatchdogCommand(); } #endif diff --git a/src/lib/deskflow/win32/DaemonApp.h b/src/lib/deskflow/win32/DaemonApp.h index 3490d7be9..c06d8c606 100644 --- a/src/lib/deskflow/win32/DaemonApp.h +++ b/src/lib/deskflow/win32/DaemonApp.h @@ -64,8 +64,9 @@ class DaemonApp : public QObject int mainLoop(); int daemonLoop(); void saveLogLevel(const QString &logLevel) const; + void setMode(const QString &mode); + void setArgs(const QString &args); void setElevate(bool elevate); - void setCommand(const QString &command); void applyWatchdogCommand() const; void clearWatchdogCommand(); void clearSettings() const; @@ -79,7 +80,8 @@ class DaemonApp : public QObject IEventQueue &m_events; FileLogOutputter *m_pFileLogOutputter = nullptr; deskflow::core::ipc::DaemonIpcServer *m_ipcServer = nullptr; - std::string m_command = ""; + QString m_mode; + QString m_args; bool m_elevate = false; bool m_foreground = false; }; diff --git a/src/lib/deskflow/win32/DaemonIpcServer.cpp b/src/lib/deskflow/win32/DaemonIpcServer.cpp index 94254823f..65234ee19 100644 --- a/src/lib/deskflow/win32/DaemonIpcServer.cpp +++ b/src/lib/deskflow/win32/DaemonIpcServer.cpp @@ -121,10 +121,12 @@ void DaemonIpcServer::processMessage(QLocalSocket *clientSocket, const QString & clientSocket->write(kAckMessage); } else if (command == "logLevel") { processLogLevel(clientSocket, parts); + } else if (command == "mode") { + processMode(clientSocket, parts); + } else if (command == "args") { + processArgs(clientSocket, message); } else if (command == "elevate") { processElevate(clientSocket, parts); - } else if (command == "command") { - processCommand(clientSocket, parts); } else if (command == "start") { LOG_DEBUG("ipc server got start message"); Q_EMIT startProcessRequested(); @@ -167,43 +169,59 @@ void DaemonIpcServer::processLogLevel(QLocalSocket *&clientSocket, const QString clientSocket->write(kAckMessage); } -void DaemonIpcServer::processElevate(QLocalSocket *&clientSocket, const QStringList &messageParts) +void DaemonIpcServer::processMode(QLocalSocket *&clientSocket, const QStringList &messageParts) { if (messageParts.size() < 2) { - LOG_ERR("ipc server got invalid elevate message"); + LOG_ERR("ipc server got invalid mode message"); clientSocket->write(kErrorMessage); return; } - const auto &elevate = messageParts[1]; - if (elevate != "yes" && elevate != "no") { - LOG_ERR("ipc server got invalid elevate value: %s", elevate.toUtf8().constData()); + const auto &mode = messageParts[1]; + if (mode != "server" && mode != "client") { + LOG_ERR("ipc server got invalid mode value: %s", mode.toUtf8().constData()); clientSocket->write(kErrorMessage); return; } - LOG_DEBUG("ipc server got new elevate value: %s", elevate.toUtf8().constData()); - Q_EMIT elevateModeChanged(elevate == "yes"); + LOG_DEBUG("ipc server got new mode value: %s", mode.toUtf8().constData()); + Q_EMIT modeChanged(mode); + clientSocket->write(kAckMessage); +} + +void DaemonIpcServer::processArgs(QLocalSocket *&clientSocket, const QString &message) +{ + // Slice on first '=' (not split) so arg values like `--key=value` survive. + const auto eqIdx = message.indexOf('='); + if (eqIdx < 0) { + LOG_ERR("ipc server got invalid args message"); + clientSocket->write(kErrorMessage); + return; + } + + const auto args = message.mid(eqIdx + 1); + LOG_DEBUG("ipc server got new args"); + Q_EMIT argsChanged(args); clientSocket->write(kAckMessage); } -void DaemonIpcServer::processCommand(QLocalSocket *&clientSocket, const QStringList &messageParts) +void DaemonIpcServer::processElevate(QLocalSocket *&clientSocket, const QStringList &messageParts) { if (messageParts.size() < 2) { - LOG_ERR("ipc server got invalid command message"); + LOG_ERR("ipc server got invalid elevate message"); clientSocket->write(kErrorMessage); return; } - const auto &command = messageParts[1]; - if (command.isEmpty()) { - LOG_ERR("ipc server got empty command"); + const auto &elevate = messageParts[1]; + if (elevate != "yes" && elevate != "no") { + LOG_ERR("ipc server got invalid elevate value: %s", elevate.toUtf8().constData()); clientSocket->write(kErrorMessage); return; } - LOG_DEBUG("ipc server got new command: %s", command.toUtf8().constData()); - Q_EMIT commandChanged(command); + LOG_DEBUG("ipc server got new elevate value: %s", elevate.toUtf8().constData()); + Q_EMIT elevateModeChanged(elevate == "yes"); clientSocket->write(kAckMessage); } diff --git a/src/lib/deskflow/win32/DaemonIpcServer.h b/src/lib/deskflow/win32/DaemonIpcServer.h index 1bc95716c..9e35cc1de 100644 --- a/src/lib/deskflow/win32/DaemonIpcServer.h +++ b/src/lib/deskflow/win32/DaemonIpcServer.h @@ -26,8 +26,9 @@ class DaemonIpcServer : public QObject signals: void logLevelChanged(const QString &logLevel); + void modeChanged(const QString &mode); + void argsChanged(const QString &args); void elevateModeChanged(bool elevate); - void commandChanged(const QString &command); void startProcessRequested(); void stopProcessRequested(); void clearSettingsRequested(); @@ -35,8 +36,9 @@ class DaemonIpcServer : public QObject private: void processMessage(QLocalSocket *clientSocket, const QString &message); void processLogLevel(QLocalSocket *&clientSocket, const QStringList &messageParts); + void processMode(QLocalSocket *&clientSocket, const QStringList &messageParts); + void processArgs(QLocalSocket *&clientSocket, const QString &message); void processElevate(QLocalSocket *&clientSocket, const QStringList &messageParts); - void processCommand(QLocalSocket *&clientSocket, const QStringList &messageParts); private slots: void handleNewConnection(); diff --git a/src/lib/gui/core/CoreProcess.cpp b/src/lib/gui/core/CoreProcess.cpp index 02060b741..ddcaed534 100644 --- a/src/lib/gui/core/CoreProcess.cpp +++ b/src/lib/gui/core/CoreProcess.cpp @@ -278,7 +278,7 @@ void CoreProcess::startForegroundProcess(const QString &app, const QStringList & } } -void CoreProcess::startProcessFromDaemon(const QString &app, const QStringList &args) +void CoreProcess::startProcessFromDaemon(const QString & /*app*/, const QStringList &args) { using enum ProcessState; @@ -286,12 +286,26 @@ void CoreProcess::startProcessFromDaemon(const QString &app, const QStringList & qFatal("core process must be in starting state"); } - QString commandQuoted = makeQuotedArgs(app, args); + const QString modeStr = (mode() == Mode::Server) ? QStringLiteral("server") : QStringLiteral("client"); + const QString argsQuoted = makeQuotedArgs(QString(), args).trimmed(); - qInfo("running command: %s", qPrintable(commandQuoted)); + qInfo("sending start to daemon (mode: %s)", qPrintable(modeStr)); + qDebug("daemon args: %s", qPrintable(argsQuoted)); - if (!m_daemonIpcClient->sendStartProcess(commandQuoted, m_appConfig.elevateMode())) { - qWarning("cannot start process, ipc command failed"); + if (!m_daemonIpcClient->sendMode(modeStr)) { + qWarning("cannot start process, ipc sendMode failed"); + return; + } + if (!m_daemonIpcClient->sendArgs(argsQuoted)) { + qWarning("cannot start process, ipc sendArgs failed"); + return; + } + if (!m_daemonIpcClient->sendElevate(m_appConfig.elevateMode())) { + qWarning("cannot start process, ipc sendElevate failed"); + return; + } + if (!m_daemonIpcClient->sendStartProcess()) { + qWarning("cannot start process, ipc start failed"); return; } diff --git a/src/lib/gui/ipc/DaemonIpcClient.cpp b/src/lib/gui/ipc/DaemonIpcClient.cpp index a2fb602a2..9272cd1de 100644 --- a/src/lib/gui/ipc/DaemonIpcClient.cpp +++ b/src/lib/gui/ipc/DaemonIpcClient.cpp @@ -183,18 +183,34 @@ bool DaemonIpcClient::sendLogLevel(const QString &logLevel) return true; } -bool DaemonIpcClient::sendStartProcess(const QString &command, ElevateMode elevateMode) +bool DaemonIpcClient::sendMode(const QString &mode) { if (!keepAlive()) return false; - if (!sendMessage("elevate=" + (elevateMode == ElevateMode::kAlways ? QStringLiteral("yes") : QStringLiteral("no")))) { + return sendMessage("mode=" + mode); +} + +bool DaemonIpcClient::sendArgs(const QString &args) +{ + if (!keepAlive()) return false; - } - if (!sendMessage("command=" + command)) { + return sendMessage("args=" + args); +} + +bool DaemonIpcClient::sendElevate(ElevateMode elevateMode) +{ + if (!keepAlive()) + return false; + + return sendMessage("elevate=" + (elevateMode == ElevateMode::kAlways ? QStringLiteral("yes") : QStringLiteral("no"))); +} + +bool DaemonIpcClient::sendStartProcess() +{ + if (!keepAlive()) return false; - } return sendMessage("start"); } diff --git a/src/lib/gui/ipc/DaemonIpcClient.h b/src/lib/gui/ipc/DaemonIpcClient.h index 198b677d1..417bb4242 100644 --- a/src/lib/gui/ipc/DaemonIpcClient.h +++ b/src/lib/gui/ipc/DaemonIpcClient.h @@ -32,7 +32,10 @@ class DaemonIpcClient : public QObject bool connectToServer(); void disconnectFromServer(); bool sendLogLevel(const QString &logLevel); - bool sendStartProcess(const QString &command, ElevateMode elevateMode); + bool sendMode(const QString &mode); + bool sendArgs(const QString &args); + bool sendElevate(ElevateMode elevateMode); + bool sendStartProcess(); bool sendStopProcess(); bool sendClearSettings(); QString requestLogPath(); From 4c6efa4d4a1aa6f629b49dbdf59c2453fcb24ad4 Mon Sep 17 00:00:00 2001 From: Nick Bolton Date: Mon, 27 Apr 2026 13:08:23 +0100 Subject: [PATCH 2/3] refactor: simplify startProcessFromDaemon signature by removing unused parameter --- src/lib/gui/core/CoreProcess.cpp | 4 ++-- src/lib/gui/core/CoreProcess.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/gui/core/CoreProcess.cpp b/src/lib/gui/core/CoreProcess.cpp index ddcaed534..73456be2f 100644 --- a/src/lib/gui/core/CoreProcess.cpp +++ b/src/lib/gui/core/CoreProcess.cpp @@ -278,7 +278,7 @@ void CoreProcess::startForegroundProcess(const QString &app, const QStringList & } } -void CoreProcess::startProcessFromDaemon(const QString & /*app*/, const QStringList &args) +void CoreProcess::startProcessFromDaemon(const QStringList &args) { using enum ProcessState; @@ -422,7 +422,7 @@ void CoreProcess::start(std::optional processModeOption) if (processMode == ProcessMode::kDesktop) { startForegroundProcess(app, args); } else if (processMode == ProcessMode::kService) { - startProcessFromDaemon(app, args); + startProcessFromDaemon(args); } m_lastProcessMode = processMode; diff --git a/src/lib/gui/core/CoreProcess.h b/src/lib/gui/core/CoreProcess.h index f1a27bc2e..ac48bc6b3 100644 --- a/src/lib/gui/core/CoreProcess.h +++ b/src/lib/gui/core/CoreProcess.h @@ -141,7 +141,7 @@ private slots: private: void startForegroundProcess(const QString &app, const QStringList &args); - void startProcessFromDaemon(const QString &app, const QStringList &args); + void startProcessFromDaemon(const QStringList &args); void stopForegroundProcess() const; void stopProcessFromDaemon(); bool addGenericArgs(QStringList &args, const ProcessMode processMode) const; From bba954a65fd1060973b2a0dc6774bad76d5e6c5d Mon Sep 17 00:00:00 2001 From: Nick Bolton Date: Mon, 27 Apr 2026 13:51:29 +0100 Subject: [PATCH 3/3] fix: clear legacy Command setting to prevent auto-running of stashed payloads --- src/lib/deskflow/win32/DaemonApp.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/lib/deskflow/win32/DaemonApp.cpp b/src/lib/deskflow/win32/DaemonApp.cpp index 16df6ff17..32afb1f8a 100644 --- a/src/lib/deskflow/win32/DaemonApp.cpp +++ b/src/lib/deskflow/win32/DaemonApp.cpp @@ -226,6 +226,17 @@ void DaemonApp::run(QThread &daemonThread) LOG_DEBUG("using last known mode: %s", m_mode.toUtf8().constData()); applyWatchdogCommand(); } + + // Older daemons accepted `command=` IPC and persisted it here. Clearing + // stops a stashed payload from auto-running if the user reverts to one. + try { + if (!ARCH->setting("Command").empty()) { + LOG_DEBUG("clearing legacy Command setting"); + ARCH->setting("Command", std::string()); + } + } catch (XArch &e) { + LOG_ERR("failed to clear legacy Command setting: %s", e.what()); + } #endif LOG_DEBUG("starting daemon thread");