fix(startup): move the single-instance lock to a private per-user file
The single-instance lock lived in /tmp, which is world-writable. Creating it with mode 0666 there gets masked to 0644, so a second local user cannot open it O_RDWR and is wrongly blocked from running the wallet at all. A world-writable directory also lets another user pre-create or symlink the lock path. - The lock now lives in a per-user directory: $XDG_RUNTIME_DIR when it is an absolute directory the user owns, otherwise the per-user, per-variant config directory that startup has already created. Never /tmp. - It is created mode 0600 with O_NOFOLLOW, and after opening we fstat it and refuse anything that is not a regular file we own, rejecting a planted symlink or a foreign-owned file. - The lock file is no longer unlinked on release; flock drops on close, and leaving the file in place avoids an unlink/relock race (a stale file is harmless). Tests cover acquiring and conflicting on the lock, the 0600 mode, that release leaves the file in place, and that a symlinked lock path is refused. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1286,6 +1286,7 @@ if(BUILD_TESTING)
|
||||
src/util/daemon_updater.cpp
|
||||
src/util/daemon_updater_core.cpp
|
||||
src/util/bootstrap.cpp
|
||||
src/util/single_instance.cpp
|
||||
${MINIZ_SOURCES}
|
||||
)
|
||||
|
||||
|
||||
@@ -7,12 +7,14 @@
|
||||
#include <cstdio>
|
||||
|
||||
#include "../util/logger.h"
|
||||
#include "platform.h" // dragonx::util::Platform::getObsidianDragonDir()
|
||||
|
||||
#ifdef _WIN32
|
||||
#include <windows.h>
|
||||
#else
|
||||
#include <unistd.h>
|
||||
#include <sys/file.h>
|
||||
#include <sys/stat.h>
|
||||
#include <fcntl.h>
|
||||
#include <errno.h>
|
||||
#include <cstring>
|
||||
@@ -24,12 +26,7 @@ namespace util {
|
||||
SingleInstance::SingleInstance(const std::string& app_name)
|
||||
: app_name_(app_name)
|
||||
{
|
||||
#ifndef _WIN32
|
||||
// Construct lock file path
|
||||
const char* tmpdir = getenv("TMPDIR");
|
||||
if (!tmpdir) tmpdir = "/tmp";
|
||||
lock_path_ = std::string(tmpdir) + "/" + app_name_ + ".lock";
|
||||
#endif
|
||||
// POSIX lock path is computed in tryLock(), once the per-user dir exists.
|
||||
}
|
||||
|
||||
SingleInstance::~SingleInstance()
|
||||
@@ -68,14 +65,35 @@ bool SingleInstance::tryLock()
|
||||
|
||||
#else
|
||||
// Linux/macOS: Use file lock (flock)
|
||||
// O_CLOEXEC: children (xdg-open + the browser it starts, dragonxd, xmrig) must not inherit the lock;
|
||||
// after a crash they'd keep the flock held and block relaunching the wallet.
|
||||
lock_fd_ = open(lock_path_.c_str(), O_CREAT | O_RDWR | O_CLOEXEC, 0666);
|
||||
|
||||
// Per-user lock dir: $XDG_RUNTIME_DIR (if it is an absolute dir we own) else the per-user, per-variant
|
||||
// config dir, which main() has already created before tryLock(). Never /tmp (world-writable: a 0666
|
||||
// lock there is masked to 0644, so a second local user can't open it O_RDWR and is wrongly blocked).
|
||||
std::string dir;
|
||||
const char* xrd = getenv("XDG_RUNTIME_DIR");
|
||||
if (xrd && xrd[0] == '/') {
|
||||
struct stat ds;
|
||||
if (stat(xrd, &ds) == 0 && S_ISDIR(ds.st_mode) && ds.st_uid == geteuid())
|
||||
dir = xrd;
|
||||
}
|
||||
if (dir.empty()) dir = Platform::getObsidianDragonDir();
|
||||
lock_path_ = dir + "/" + app_name_ + ".lock";
|
||||
|
||||
// O_CLOEXEC: children (xdg-open + the browser it starts, dragonxd, xmrig) must not inherit the lock, or
|
||||
// after a crash they'd hold the flock and block relaunch. O_NOFOLLOW + the fstat below reject a planted
|
||||
// symlink or a foreign-owned file. 0600: per-user.
|
||||
lock_fd_ = open(lock_path_.c_str(), O_CREAT | O_RDWR | O_CLOEXEC | O_NOFOLLOW, 0600);
|
||||
|
||||
if (lock_fd_ < 0) {
|
||||
DEBUG_LOGF("Failed to open lock file %s: %s\n", lock_path_.c_str(), strerror(errno));
|
||||
return false;
|
||||
}
|
||||
struct stat ls;
|
||||
if (fstat(lock_fd_, &ls) != 0 || !S_ISREG(ls.st_mode) || ls.st_uid != geteuid()) {
|
||||
DEBUG_LOGF("Lock file %s is not a regular file we own - refusing\n", lock_path_.c_str());
|
||||
close(lock_fd_);
|
||||
lock_fd_ = -1;
|
||||
return false;
|
||||
}
|
||||
|
||||
// Try non-blocking exclusive lock
|
||||
if (flock(lock_fd_, LOCK_EX | LOCK_NB) != 0) {
|
||||
@@ -124,10 +142,9 @@ void SingleInstance::unlock()
|
||||
flock(lock_fd_, LOCK_UN);
|
||||
close(lock_fd_);
|
||||
lock_fd_ = -1;
|
||||
|
||||
// Remove the lock file
|
||||
unlink(lock_path_.c_str());
|
||||
DEBUG_LOGF("Single instance lock released (lock file removed)\n");
|
||||
// The lock file is intentionally kept: flock releases on close, and not unlinking
|
||||
// avoids an unlink/relock race.
|
||||
DEBUG_LOGF("Single instance lock released\n");
|
||||
}
|
||||
#endif
|
||||
|
||||
|
||||
@@ -127,6 +127,8 @@
|
||||
#include <cstdlib>
|
||||
#include <deque>
|
||||
#include <filesystem>
|
||||
#include <sys/stat.h>
|
||||
#include "util/single_instance.h"
|
||||
#include <fstream>
|
||||
#include <iostream>
|
||||
#include <algorithm>
|
||||
@@ -3974,6 +3976,44 @@ void testBootstrapDecideResume()
|
||||
EXPECT_TRUE(Bootstrap::decideResume(3, false, 999) == R::RangedGet); // bogus error-page length ignored
|
||||
}
|
||||
|
||||
// C219/C220: the single-instance lock lives in a per-user dir, is 0600, O_NOFOLLOW (POSIX only).
|
||||
// Mutates XDG_RUNTIME_DIR, so it saves/restores it and runs before anything that reads that env.
|
||||
void testSingleInstance()
|
||||
{
|
||||
#ifndef _WIN32
|
||||
namespace fs = std::filesystem;
|
||||
std::error_code ec;
|
||||
const fs::path tmp = fs::temp_directory_path(ec) / ("od_si_test_" + std::to_string(::getpid()));
|
||||
fs::create_directories(tmp, ec);
|
||||
::chmod(tmp.c_str(), 0700);
|
||||
const char* prev = ::getenv("XDG_RUNTIME_DIR");
|
||||
const std::string saved = prev ? prev : "";
|
||||
const bool had = prev != nullptr;
|
||||
::setenv("XDG_RUNTIME_DIR", tmp.c_str(), 1);
|
||||
{
|
||||
dragonx::util::SingleInstance a("od-test"), b("od-test");
|
||||
EXPECT_TRUE(a.tryLock()); // first wins
|
||||
EXPECT_TRUE(!b.tryLock()); // flock conflicts across OFDs, even in one process
|
||||
const fs::path lockf = tmp / "od-test.lock";
|
||||
struct stat st{};
|
||||
EXPECT_TRUE(::stat(lockf.c_str(), &st) == 0);
|
||||
EXPECT_EQ((int)(st.st_mode & 0777), 0600);
|
||||
a.unlock();
|
||||
EXPECT_TRUE(fs::exists(lockf, ec)); // lock file is NOT removed on unlock (stale is harmless)
|
||||
// A planted symlink at the lock path is refused by O_NOFOLLOW (open fails -> tryLock false).
|
||||
const fs::path sym = tmp / "od-sym.lock";
|
||||
fs::create_symlink("/nonexistent/target", sym, ec);
|
||||
if (!ec) {
|
||||
dragonx::util::SingleInstance c("od-sym");
|
||||
EXPECT_TRUE(!c.tryLock());
|
||||
}
|
||||
}
|
||||
if (had) ::setenv("XDG_RUNTIME_DIR", saved.c_str(), 1);
|
||||
else ::unsetenv("XDG_RUNTIME_DIR");
|
||||
fs::remove_all(tmp, ec);
|
||||
#endif
|
||||
}
|
||||
|
||||
void testBootstrapSpaceSufficient()
|
||||
{
|
||||
using dragonx::util::Bootstrap;
|
||||
@@ -18462,6 +18502,7 @@ int main()
|
||||
testBootstrapSpaceSufficient();
|
||||
testBootstrapParseChecksum();
|
||||
testBootstrapDecideResume();
|
||||
testSingleInstance();
|
||||
testBootstrapPlanArchive();
|
||||
#ifndef _WIN32
|
||||
testBootstrapReclaimable();
|
||||
|
||||
Reference in New Issue
Block a user