security: payment-URI amount validation, export filename sanitize, chat crash-guard, HTTPS-only, conf 0600, pool auto-lock
Six bounded defensive fixes: - Payment-URI amount is now strictly validated (reject NaN/inf/sign/garbage, <=8 decimals, 0..21,000,000) at parse time, so a crafted QR/URI can't inject a bogus amount into the Send form. The z_sendmany call the user confirms is unchanged; this is input validation only. - export-all-keys filename is sanitized (basename-only, no traversal) via a shared util::sanitizeExportFilename (factored out of the CSV export dialog). - Chat append()/upsert() wrap the strict serialize in try/catch so a malformed (invalid-UTF-8, attacker-influenceable) body is skipped, not crashed on — matching appendBatch's existing drop behavior. - http_download (used by the updaters + pool stats) restricts transfers AND redirects to HTTPS and bounds the download path's redirects, so a redirect can't downgrade to http:// or another scheme. - The default DRGX.conf (rpcuser/rpcpassword) is written 0600 from the start via writeFileAtomically, closing the brief world-readable ofstream+chmod window. - Auto-lock is suppressed only for SOLO mining (needs the wallet), not pool mining (pays an external address), which was holding the wallet open forever. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -791,12 +791,11 @@ void App::checkAutoLock() {
|
||||
if (capture_mode_) return; // don't auto-lock while a UI sweep forces the encrypted/locked state
|
||||
if (!state_.isEncrypted() || state_.isLocked()) return;
|
||||
|
||||
// Don't auto-lock while mining — mining is a long-running intentional
|
||||
// operation and locking the wallet on the daemon side stops solo mining.
|
||||
bool miningActive = state_.mining.generate
|
||||
|| (xmrig_manager_ && xmrig_manager_->isRunning());
|
||||
if (miningActive) {
|
||||
// Keep resetting the idle timer so we don't lock the instant mining stops
|
||||
// Don't auto-lock while SOLO mining — it runs in the daemon and locking the wallet there would stop
|
||||
// it (and it shields matured rewards, which needs the wallet). Pool mining pays an external address
|
||||
// and needs no unlocked wallet, so it must NOT hold the wallet open indefinitely.
|
||||
if (state_.mining.generate) {
|
||||
// Keep resetting the idle timer so we don't lock the instant solo mining stops.
|
||||
idle_.noteActivity();
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -108,8 +108,18 @@ bool ChatDatabase::append(const ChatMessage& message)
|
||||
|
||||
std::vector<unsigned char> nonce;
|
||||
std::vector<unsigned char> cipher;
|
||||
std::string plain = serialize(message); // full plaintext (decrypted body + metadata)
|
||||
const bool encrypted = encrypt(plain, nonce, cipher);
|
||||
std::string plain;
|
||||
bool encrypted = false;
|
||||
try {
|
||||
// serialize() dumps strictly, so an invalid-UTF-8 (attacker-influenceable) body THROWS here;
|
||||
// skip the row instead of letting it escape the write path and crash (appendBatch already does).
|
||||
plain = serialize(message);
|
||||
encrypted = encrypt(plain, nonce, cipher);
|
||||
} catch (const std::exception& e) {
|
||||
if (!plain.empty()) sodium_memzero(&plain[0], plain.size());
|
||||
DEBUG_LOGF("[chat] append skipped a malformed message: %s\n", e.what());
|
||||
return false;
|
||||
}
|
||||
if (!plain.empty()) sodium_memzero(&plain[0], plain.size()); // don't leave it on the heap
|
||||
if (!encrypted) return false;
|
||||
|
||||
@@ -187,8 +197,16 @@ bool ChatDatabase::upsert(const ChatMessage& message)
|
||||
|
||||
std::vector<unsigned char> nonce;
|
||||
std::vector<unsigned char> cipher;
|
||||
std::string plain = serialize(message);
|
||||
const bool encrypted = encrypt(plain, nonce, cipher);
|
||||
std::string plain;
|
||||
bool encrypted = false;
|
||||
try {
|
||||
plain = serialize(message); // strict dump → a malformed body throws; skip rather than crash
|
||||
encrypted = encrypt(plain, nonce, cipher);
|
||||
} catch (const std::exception& e) {
|
||||
if (!plain.empty()) sodium_memzero(&plain[0], plain.size());
|
||||
DEBUG_LOGF("[chat] upsert skipped a malformed message: %s\n", e.what());
|
||||
return false;
|
||||
}
|
||||
if (!plain.empty()) sodium_memzero(&plain[0], plain.size());
|
||||
if (!encrypted) return false;
|
||||
|
||||
@@ -416,7 +434,7 @@ std::string ChatDatabase::serialize(const ChatMessage& message) const
|
||||
json["ts"] = message.timestamp;
|
||||
json["pos"] = static_cast<std::uint64_t>(message.payload_position);
|
||||
json["dl"] = static_cast<int>(message.delivery);
|
||||
return json.dump();
|
||||
return json.dump(); // strict: an invalid-UTF-8 body throws and is DROPPED by the callers (contract)
|
||||
}
|
||||
|
||||
bool ChatDatabase::deserialize(const std::string& json, ChatMessage& out) const
|
||||
|
||||
+9
-18
@@ -460,15 +460,14 @@ bool Connection::createDefaultConfig(const std::string& path)
|
||||
return false;
|
||||
}
|
||||
|
||||
std::ofstream file(path);
|
||||
if (!file.is_open()) {
|
||||
DEBUG_LOGF("Failed to create config file: %s\n", path.c_str());
|
||||
return false;
|
||||
}
|
||||
|
||||
// Build the content in memory, then write it 0600 from the start (writeFileAtomically with
|
||||
// restrictPermissions) — the old ofstream+chmod left a brief world-readable window during which
|
||||
// another local user could read the freshly-generated rpcuser/rpcpassword.
|
||||
std::ostringstream file;
|
||||
|
||||
// Get the data directory for exportdir
|
||||
std::string dataDir = getDefaultDataDir();
|
||||
|
||||
|
||||
file << "# DragonX configuration file\n";
|
||||
file << "# Auto-generated by DragonX Wallet\n";
|
||||
file << "\n";
|
||||
@@ -485,18 +484,10 @@ bool Connection::createDefaultConfig(const std::string& path)
|
||||
file << "addnode=seed.dragonx.is\n";
|
||||
file << "addnode=node1.dragonx.is\n";
|
||||
file << "addnode=node5.dragonx.is\n";
|
||||
|
||||
file.close();
|
||||
|
||||
// The file holds the freshly-generated rpcuser/rpcpassword in plaintext. ofstream creates it
|
||||
// with the process umask (typically world-readable 0644), so restrict it to owner read/write
|
||||
// before another local user can read the credentials.
|
||||
{
|
||||
namespace fs = std::filesystem;
|
||||
std::error_code ec;
|
||||
fs::permissions(path, fs::perms::owner_read | fs::perms::owner_write,
|
||||
fs::perm_options::replace, ec);
|
||||
if (ec) DEBUG_LOGF("Could not restrict config permissions on %s: %s\n", path.c_str(), ec.message().c_str());
|
||||
if (!util::Platform::writeFileAtomically(path, file.str(), /*restrictPermissions=*/true)) {
|
||||
DEBUG_LOGF("Failed to create config file: %s\n", path.c_str());
|
||||
return false;
|
||||
}
|
||||
|
||||
DEBUG_LOGF("Created default config file: %s\n", path.c_str());
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
#include "../../rpc/rpc_worker.h"
|
||||
#include "../../util/i18n.h"
|
||||
#include "../../util/platform.h"
|
||||
#include "../../util/export_filename.h"
|
||||
#include "../notifications.h"
|
||||
#include "../schema/ui_schema.h"
|
||||
#include "../material/draw_helpers.h"
|
||||
@@ -160,8 +161,11 @@ void ExportAllKeysDialog::render(App* app)
|
||||
if (s_include_t) {
|
||||
for (const auto& a : state.t_addresses) t_addrs.push_back(a.address);
|
||||
}
|
||||
std::string filename(s_filename);
|
||||
|
||||
// Sanitize the typed filename (basename only, no traversal) and reflect it back in the
|
||||
// buffer so the user sees exactly what gets written. Mirrors the CSV export dialog.
|
||||
std::string filename = util::sanitizeExportFilename(s_filename, ".txt", "dragonx_keys.txt");
|
||||
snprintf(s_filename, sizeof(s_filename), "%s", filename.c_str());
|
||||
|
||||
// Run all key exports on worker thread
|
||||
if (app->worker()) {
|
||||
app->worker()->post([rpc = app->rpc(), z_addrs, t_addrs, filename]() -> rpc::RPCWorker::MainCb {
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
#include "../../app.h"
|
||||
#include "../../util/i18n.h"
|
||||
#include "../../util/platform.h"
|
||||
#include "../../util/export_filename.h"
|
||||
#include "../../data/transaction_csv.h"
|
||||
#include "transactions_tab.h"
|
||||
#include "../notifications.h"
|
||||
@@ -49,31 +50,6 @@ static bool parseYmd(const char* s, std::int64_t& outStartOfDay) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Make a user-typed filename safe to write into the config dir: basename only, illegal chars → '_',
|
||||
// no leading dots / trailing dots-or-spaces, length-capped, always ending in .csv.
|
||||
static std::string sanitizeExportFilename(std::string name) {
|
||||
const size_t slash = name.find_last_of("/\\");
|
||||
if (slash != std::string::npos) name = name.substr(slash + 1);
|
||||
auto notspace = [](unsigned char c) { return !std::isspace(c); };
|
||||
name.erase(name.begin(), std::find_if(name.begin(), name.end(), notspace));
|
||||
name.erase(std::find_if(name.rbegin(), name.rend(), notspace).base(), name.end());
|
||||
for (char& c : name) {
|
||||
const unsigned char u = static_cast<unsigned char>(c);
|
||||
if (u < 0x20 || c == '<' || c == '>' || c == ':' || c == '"' || c == '|' || c == '?' || c == '*')
|
||||
c = '_';
|
||||
}
|
||||
const size_t firstGood = name.find_first_not_of('.');
|
||||
if (firstGood == std::string::npos) name.clear(); else name.erase(0, firstGood);
|
||||
while (!name.empty() && (name.back() == '.' || name.back() == ' ')) name.pop_back();
|
||||
if (name.size() > 120) name.resize(120);
|
||||
std::string lower = name;
|
||||
std::transform(lower.begin(), lower.end(), lower.begin(),
|
||||
[](unsigned char c) { return static_cast<char>(std::tolower(c)); });
|
||||
if (lower.size() < 4 || lower.compare(lower.size() - 4, 4, ".csv") != 0) name += ".csv";
|
||||
if (name.empty() || name == ".csv") name = "dragonx_transactions.csv";
|
||||
return name;
|
||||
}
|
||||
|
||||
void ExportTransactionsDialog::show()
|
||||
{
|
||||
s_open = true;
|
||||
@@ -226,7 +202,7 @@ void ExportTransactionsDialog::render(App* app)
|
||||
std::string configDir = util::Platform::getConfigDir();
|
||||
// Sanitize in-buffer so the user sees exactly what was written (and we can't be pointed outside
|
||||
// the config dir by a pasted path).
|
||||
std::string safeName = sanitizeExportFilename(s_filename);
|
||||
std::string safeName = util::sanitizeExportFilename(s_filename, ".csv", "dragonx_transactions.csv");
|
||||
snprintf(s_filename, sizeof(s_filename), "%s", safeName.c_str());
|
||||
std::string filepath = configDir + "/" + safeName;
|
||||
// Build the chosen subset (scope 0 exports the whole list with no copy), then write atomically
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
// DragonX Wallet - ImGui Edition
|
||||
// Copyright 2024-2026 The Hush Developers
|
||||
// Released under the GPLv3
|
||||
#pragma once
|
||||
|
||||
#include <string>
|
||||
#include <algorithm>
|
||||
#include <cctype>
|
||||
|
||||
namespace dragonx {
|
||||
namespace util {
|
||||
|
||||
// Make a user-typed filename safe to write into a trusted config/export directory: basename only
|
||||
// (no path traversal), illegal / control chars -> '_', no leading dots or trailing dots-or-spaces,
|
||||
// Windows reserved device stems prefixed with '_', length-capped, and always ending in requiredExt
|
||||
// (e.g. ".csv"). Returns `fallback` if nothing usable remains. Header-only so every export dialog
|
||||
// shares one implementation.
|
||||
inline std::string sanitizeExportFilename(std::string name, const char* requiredExt, const char* fallback) {
|
||||
const std::size_t slash = name.find_last_of("/\\");
|
||||
if (slash != std::string::npos) name = name.substr(slash + 1);
|
||||
auto notSpace = [](unsigned char c) { return !std::isspace(c); };
|
||||
name.erase(name.begin(), std::find_if(name.begin(), name.end(), notSpace));
|
||||
name.erase(std::find_if(name.rbegin(), name.rend(), notSpace).base(), name.end());
|
||||
for (char& c : name) {
|
||||
const unsigned char u = static_cast<unsigned char>(c);
|
||||
if (u < 0x20 || c == '<' || c == '>' || c == ':' || c == '"' || c == '|' ||
|
||||
c == '?' || c == '*' || c == '/' || c == '\\')
|
||||
c = '_';
|
||||
}
|
||||
const std::size_t firstGood = name.find_first_not_of('.');
|
||||
if (firstGood == std::string::npos) name.clear(); else name.erase(0, firstGood);
|
||||
while (!name.empty() && (name.back() == '.' || name.back() == ' ')) name.pop_back();
|
||||
if (name.size() > 120) name.resize(120);
|
||||
// Windows reserved device names (case-insensitive, checked on the stem before the first dot).
|
||||
{
|
||||
std::string stem = name;
|
||||
const std::size_t dot = stem.find('.');
|
||||
if (dot != std::string::npos) stem = stem.substr(0, dot);
|
||||
std::transform(stem.begin(), stem.end(), stem.begin(),
|
||||
[](unsigned char c) { return static_cast<char>(std::toupper(c)); });
|
||||
static const char* const kReserved[] = {
|
||||
"CON", "PRN", "AUX", "NUL",
|
||||
"COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9",
|
||||
"LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9",
|
||||
};
|
||||
for (const char* r : kReserved) if (stem == r) { name = "_" + name; break; }
|
||||
}
|
||||
std::string lower = name;
|
||||
std::transform(lower.begin(), lower.end(), lower.begin(),
|
||||
[](unsigned char c) { return static_cast<char>(std::tolower(c)); });
|
||||
std::string lext = requiredExt ? requiredExt : "";
|
||||
std::transform(lext.begin(), lext.end(), lext.begin(),
|
||||
[](unsigned char c) { return static_cast<char>(std::tolower(c)); });
|
||||
if (!lext.empty() &&
|
||||
(lower.size() < lext.size() || lower.compare(lower.size() - lext.size(), lext.size(), lext) != 0))
|
||||
name += requiredExt;
|
||||
if (name.empty() || (requiredExt && name == requiredExt)) name = fallback ? fallback : "export";
|
||||
return name;
|
||||
}
|
||||
|
||||
} // namespace util
|
||||
} // namespace dragonx
|
||||
@@ -60,6 +60,20 @@ int cancelXferCb(void* clientp, curl_off_t, curl_off_t, curl_off_t, curl_off_t)
|
||||
return static_cast<const std::atomic<bool>*>(clientp)->load(std::memory_order_relaxed) ? 1 : 0;
|
||||
}
|
||||
|
||||
// Restrict the transfer AND any redirect to HTTPS so a redirect can't silently downgrade to http://,
|
||||
// file://, ftp:// or an unexpected scheme. Every caller of these shared helpers (daemon/xmrig updaters,
|
||||
// pool stats) uses https URLs. (Verify-peer/host stay on at the call sites.)
|
||||
void restrictToHttps(CURL* curl)
|
||||
{
|
||||
#if LIBCURL_VERSION_NUM >= 0x075500 // 7.85.0: the string-form protocol setopts
|
||||
curl_easy_setopt(curl, CURLOPT_PROTOCOLS_STR, "https");
|
||||
curl_easy_setopt(curl, CURLOPT_REDIR_PROTOCOLS_STR, "https");
|
||||
#else
|
||||
curl_easy_setopt(curl, CURLOPT_PROTOCOLS, CURLPROTO_HTTPS);
|
||||
curl_easy_setopt(curl, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTPS);
|
||||
#endif
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
HttpGetResult httpGet(const std::string& url, const char* logTag, const HttpGetOptions& opt)
|
||||
@@ -78,6 +92,7 @@ HttpGetResult httpGet(const std::string& url, const char* logTag, const HttpGetO
|
||||
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &sink);
|
||||
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
|
||||
curl_easy_setopt(curl, CURLOPT_MAXREDIRS, opt.maxRedirects);
|
||||
restrictToHttps(curl);
|
||||
curl_easy_setopt(curl, CURLOPT_USERAGENT, "ObsidianDragon/1.0");
|
||||
curl_easy_setopt(curl, CURLOPT_TIMEOUT, opt.timeoutSec);
|
||||
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, opt.connectTimeoutSec);
|
||||
@@ -131,6 +146,8 @@ bool httpDownloadToFile(const std::string& url, const std::string& destPath,
|
||||
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writeFileCb);
|
||||
curl_easy_setopt(curl, CURLOPT_WRITEDATA, fp);
|
||||
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
|
||||
curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 5L); // bound redirects (this path set none before)
|
||||
restrictToHttps(curl);
|
||||
curl_easy_setopt(curl, CURLOPT_USERAGENT, "ObsidianDragon/1.0");
|
||||
curl_easy_setopt(curl, CURLOPT_TIMEOUT, 0L);
|
||||
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 30L);
|
||||
|
||||
@@ -10,6 +10,29 @@
|
||||
#include <algorithm>
|
||||
#include <cctype>
|
||||
#include <cstdio>
|
||||
#include <cmath>
|
||||
|
||||
namespace {
|
||||
// Strictly parse a decimal DRGX amount from an (attacker-influenceable) payment URI / QR: digits with at
|
||||
// most one '.', at most 8 fractional digits, in [0, 21000000]. Rejects empty, NaN/inf, signs, whitespace
|
||||
// and trailing garbage that std::stod would otherwise accept — so a crafted URI can't inject a bogus
|
||||
// amount into the Send form.
|
||||
bool parseStrictAmount(const std::string& s, double& out) {
|
||||
if (s.empty() || s.size() > 32) return false;
|
||||
int dots = 0, digits = 0, frac = 0; bool afterDot = false;
|
||||
for (char c : s) {
|
||||
if (c == '.') { if (++dots > 1) return false; afterDot = true; }
|
||||
else if (c >= '0' && c <= '9') { ++digits; if (afterDot && ++frac > 8) return false; }
|
||||
else return false;
|
||||
}
|
||||
if (digits == 0) return false;
|
||||
double v = 0.0;
|
||||
try { v = std::stod(s); } catch (...) { return false; }
|
||||
if (!std::isfinite(v) || v < 0.0 || v > 21000000.0) return false;
|
||||
out = v;
|
||||
return true;
|
||||
}
|
||||
} // namespace
|
||||
|
||||
namespace dragonx {
|
||||
namespace util {
|
||||
@@ -141,11 +164,13 @@ PaymentURI parsePaymentURI(const std::string& uri)
|
||||
std::transform(key.begin(), key.end(), key.begin(), ::tolower);
|
||||
|
||||
if (key == "amount" || key == "amt") {
|
||||
try {
|
||||
result.amount = std::stod(value);
|
||||
} catch (...) {
|
||||
// Invalid amount, keep as 0
|
||||
double a = 0.0;
|
||||
if (!parseStrictAmount(value, a)) {
|
||||
result.error = "Invalid amount";
|
||||
result.valid = false;
|
||||
return result; // reject the whole URI rather than silently treating it as 0
|
||||
}
|
||||
result.amount = a;
|
||||
} else if (key == "label") {
|
||||
result.label = value;
|
||||
} else if (key == "memo") {
|
||||
@@ -174,7 +199,7 @@ PaymentURI parsePaymentURI(const std::string& uri)
|
||||
}
|
||||
|
||||
// Validate amount if present
|
||||
if (result.amount < 0) {
|
||||
if (!std::isfinite(result.amount) || result.amount < 0 || result.amount > 21000000.0) {
|
||||
result.error = "Invalid negative amount";
|
||||
return result;
|
||||
}
|
||||
|
||||
+11
-1
@@ -1033,7 +1033,17 @@ void testPaymentUri()
|
||||
|
||||
auto invalid = dragonx::util::parsePaymentURI("drgx:" + taddr + "?amount=-1");
|
||||
EXPECT_FALSE(invalid.valid);
|
||||
EXPECT_EQ(invalid.error, std::string("Invalid negative amount"));
|
||||
EXPECT_EQ(invalid.error, std::string("Invalid amount")); // strict parse rejects '-' up front now
|
||||
|
||||
// Crafted amounts that std::stod would have accepted are now rejected before the Send form.
|
||||
for (const char* bad : {"nan", "inf", "1e3", "0x10", "1.5abc", "0.000000001", "21000001", ""}) {
|
||||
auto r = dragonx::util::parsePaymentURI("drgx:" + taddr + "?amount=" + bad);
|
||||
EXPECT_FALSE(r.valid);
|
||||
}
|
||||
for (const char* ok : {"1", "0.00000001", "21000000", "1.5"}) {
|
||||
auto r = dragonx::util::parsePaymentURI("drgx:" + taddr + "?amount=" + ok);
|
||||
EXPECT_TRUE(r.valid);
|
||||
}
|
||||
}
|
||||
|
||||
void testWalletFileProbe()
|
||||
|
||||
Reference in New Issue
Block a user