fix(security): synchronize SecureVault::scope_ between the main thread and worker security ops
Concurrency audit: SecureVault::scope_ (std::string) is written by setWalletScope() on the main thread (wallet switch) while getVaultPath() — reached from retrieve()/hasVault()/changePin()/store() — runs on RPC worker threads during a send-time or startup unlock. A concurrent wallet switch and a worker vault op is a data race on the string (and could resolve the wrong vault file). Guard scope_ with a small mutex: setWalletScope() assigns under it; getVaultPath() copies it out under the lock and uses the copy. The lock is held only for the string copy — never across Argon2id or file I/O — and getVaultPath() never calls setWalletScope(), so no deadlock. Argon2id params, key derivation, AEAD, the vault file format and retrieve/store/changePin are byte-identical. SecureVault is held via unique_ptr and used by pointer, so the added (non-movable) mutex is fine. Synchronization only. Threading fix — clean build + ctest; the race is runtime-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -48,7 +48,11 @@ void SecureVault::setWalletScope(const std::string& walletFile) {
|
||||
// any other wallet gets a per-wallet tag. A readable sanitized prefix PLUS an 8-hex FNV-1a of the
|
||||
// RAW filename — the hash disambiguates so two distinct files can never share a vault even if their
|
||||
// sanitized forms collide (e.g. "wallet foo.dat" vs "wallet_foo.dat").
|
||||
if (walletFile.empty() || walletFile == "wallet.dat") { scope_.clear(); return; }
|
||||
if (walletFile.empty() || walletFile == "wallet.dat") {
|
||||
std::lock_guard<std::mutex> lk(scopeMutex_);
|
||||
scope_.clear();
|
||||
return;
|
||||
}
|
||||
std::string s;
|
||||
s.reserve(walletFile.size());
|
||||
for (unsigned char c : walletFile)
|
||||
@@ -58,11 +62,18 @@ void SecureVault::setWalletScope(const std::string& walletFile) {
|
||||
char suffix[9];
|
||||
std::snprintf(suffix, sizeof(suffix), "%08x", static_cast<unsigned>(h & 0xffffffffu));
|
||||
if (s.size() > 48) s.resize(48);
|
||||
scope_ = s + "-" + suffix;
|
||||
std::string newScope = s + "-" + suffix;
|
||||
std::lock_guard<std::mutex> lk(scopeMutex_);
|
||||
scope_ = std::move(newScope);
|
||||
}
|
||||
|
||||
std::string SecureVault::getVaultPath() const {
|
||||
const std::string name = scope_.empty() ? std::string("vault.dat") : ("vault-" + scope_ + ".dat");
|
||||
std::string scope;
|
||||
{
|
||||
std::lock_guard<std::mutex> lk(scopeMutex_);
|
||||
scope = scope_;
|
||||
}
|
||||
const std::string name = scope.empty() ? std::string("vault.dat") : ("vault-" + scope + ".dat");
|
||||
return (fs::path(Platform::getConfigDir()) / name).string();
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
#include <string>
|
||||
#include <vector>
|
||||
#include <cstdint>
|
||||
#include <mutex>
|
||||
|
||||
namespace dragonx {
|
||||
namespace util {
|
||||
@@ -101,6 +102,9 @@ private:
|
||||
const uint8_t* salt, size_t saltLen,
|
||||
uint8_t* key, size_t keyLen) const;
|
||||
|
||||
// Guards scope_: setWalletScope() runs on the main thread (wallet switch) while retrieve()/hasVault()/
|
||||
// changePin() run on RPC worker threads and read it via getVaultPath().
|
||||
mutable std::mutex scopeMutex_;
|
||||
std::string scope_; // "" = default/legacy wallet (vault.dat); else a sanitized wallet-file tag
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user