From 55540160293d94b0f0e651b02d615ce1e7343885 Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 22 Jul 2026 01:16:24 -0500 Subject: [PATCH 1/5] fix(security): remediate high/medium findings from branch audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Static audit of the wallet surfaced supply-chain, TLS, and secret-lifecycle issues. This fixes the six High-priority items plus four high-value Medium items; all build clean (full-node + tests) and the suite passes. Supply chain / integrity: - bootstrap: reject zip-slip / path-traversal archive members before writing (isSafeArchivePath); the sibling updaters flatten via baseName(), bootstrap keeps sub-paths so it validates instead (F2-1). - bootstrap: require a SHA-256 checksum — fail closed instead of extracting unverified chain data when none is published (F2-2). - xmrig updater: fail closed when a signature is required but no key is pinned, matching the daemon updater (was: silent downgrade to checksum-only) (F1-1). - xmrig/daemon updaters: cap metadata responses (MAXFILESIZE + 16 MiB hard cap in the write callback, which also catches chunked/no-Content-Length) (F1-2). Network / TLS: - rpc_client: set SSL_VERIFYPEER/VERIFYHOST explicitly instead of relying on libcurl defaults (F4-1). - rpc_client: bound response accumulation at 256 MiB to prevent OOM from a compromised daemon (F4-3). - lite_connection_service: reject remote http:// lite servers (loopback still allowed) to prevent a silent TLS downgrade of lightwalletd traffic (F8-1). Secret lifecycle / fund safety: - lite controller: propagate the save() result from encrypt/decryptWallet so a failed persist is not reported as success (F7-1). - xmrig_manager: chmod(0600) the pool config before writing the API token / wallet address, closing the world-readable window (F5-2). - app: clear a copied secret from the OS clipboard on shutdown so it does not outlive the process when the 45s auto-clear never fires (F3b-1). Co-Authored-By: Claude Fable 5 --- src/app.cpp | 17 +++++++-- src/app.h | 3 ++ src/daemon/xmrig_manager.cpp | 11 +++--- src/rpc/rpc_client.cpp | 10 ++++++ src/util/bootstrap.cpp | 50 +++++++++++++++++++++++--- src/util/daemon_updater.cpp | 11 ++++-- src/util/xmrig_updater.cpp | 17 +++++++-- src/wallet/lite_connection_service.cpp | 23 +++++++++++- src/wallet/lite_wallet_controller.cpp | 22 ++++++++++-- 9 files changed, 145 insertions(+), 19 deletions(-) diff --git a/src/app.cpp b/src/app.cpp index cd3f63d..a2c4ae8 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -4042,6 +4042,11 @@ void App::renderLoadingOverlay(float contentH) void App::shutdown() { + // Wipe any copied secret from the OS clipboard before we exit — the 45s auto-clear timer + // never fires if the user quits sooner, which would otherwise leave a key/seed resident. + // (ImGui context is still alive here; App::shutdown() runs before ImGui::DestroyContext().) + clearSecretClipboardIfArmed(); + // Clean up bootstrap if running if (bootstrap_) { bootstrap_->cancel(); @@ -4200,10 +4205,9 @@ void App::copySecretToClipboard(const std::string& secret) ui::Notifications::instance().info("Copied — clipboard auto-clears in 45s", 4.0f); } -void App::pumpSecretClipboardClear() +void App::clearSecretClipboardIfArmed() { - if (clipboard_clear_deadline_ <= 0.0) return; - if (ImGui::GetTime() < clipboard_clear_deadline_) return; + if (clipboard_secret_hash_ == 0) return; // Only clear if the clipboard STILL holds our secret (the user may have copied something else). if (const char* cb = ImGui::GetClipboardText()) { std::uint64_t h = 1469598103934665603ULL; @@ -4214,6 +4218,13 @@ void App::pumpSecretClipboardClear() clipboard_secret_hash_ = 0; } +void App::pumpSecretClipboardClear() +{ + if (clipboard_clear_deadline_ <= 0.0) return; + if (ImGui::GetTime() < clipboard_clear_deadline_) return; + clearSecretClipboardIfArmed(); +} + void App::maybeFinishTransactionSendProgress() { using Job = services::NetworkRefreshService::Job; diff --git a/src/app.h b/src/app.h index 3f6c7bc..8b83199 100644 --- a/src/app.h +++ b/src/app.h @@ -410,6 +410,9 @@ public: // plaintext. Call pumpSecretClipboardClear() each frame to action the clear. void copySecretToClipboard(const std::string& secret); void pumpSecretClipboardClear(); + // Immediately clear the clipboard if it still holds the armed secret (ignores the 45s timer). + // Called on app shutdown so a copied key/seed does not outlive the process in the OS clipboard. + void clearSecretClipboardIfArmed(); bool isTransactionRefreshInProgress() const { return network_refresh_.jobInProgress(services::NetworkRefreshService::Job::Transactions); } diff --git a/src/daemon/xmrig_manager.cpp b/src/daemon/xmrig_manager.cpp index 9daa411..6bd56a9 100644 --- a/src/daemon/xmrig_manager.cpp +++ b/src/daemon/xmrig_manager.cpp @@ -206,19 +206,20 @@ bool XmrigManager::generateConfig(const Config& cfg, const std::string& outPath) try { fs::create_directories(fs::path(outPath).parent_path()); - std::ofstream ofs(outPath); + std::ofstream ofs(outPath, std::ios::trunc); if (!ofs.is_open()) { last_error_ = "Cannot write xmrig config: " + outPath; DEBUG_LOGF("[ERROR] XmrigManager: %s\n", last_error_.c_str()); return false; } - ofs << j.dump(4); - ofs.close(); - #ifndef _WIN32 - // 0600 permissions — only owner can read/write + // Restrict to owner (0600) BEFORE writing any secret material (API token, wallet + // address, worker name). The file is still empty here, so the config is never + // world-readable — closing the window between creation and the previous post-write chmod. chmod(outPath.c_str(), 0600); #endif + ofs << j.dump(4); + ofs.close(); return true; } catch (const std::exception& e) { last_error_ = std::string("Config write error: ") + e.what(); diff --git a/src/rpc/rpc_client.cpp b/src/rpc/rpc_client.cpp index c2fc683..e7dd26d 100644 --- a/src/rpc/rpc_client.cpp +++ b/src/rpc/rpc_client.cpp @@ -85,6 +85,11 @@ void RPCClient::setTraceSource(std::string source) // Callback for libcurl to write response data static size_t WriteCallback(void* contents, size_t size, size_t nmemb, std::string* userp) { size_t totalSize = size * nmemb; + // Bound accumulation so a hostile/compromised daemon cannot OOM the client with an unbounded + // response body. 256 MiB is far above any legitimate JSON-RPC response (large getblock / + // listunspent / listtransactions) yet prevents true memory exhaustion. + static constexpr size_t kMaxRpcResponseBytes = 256u * 1024 * 1024; + if (userp->size() + totalSize > kMaxRpcResponseBytes) return 0; // short count aborts the transfer userp->append((char*)contents, totalSize); return totalSize; } @@ -191,6 +196,11 @@ bool RPCClient::connect(const std::string& host, const std::string& port, // budget for the TCP + TLS handshake over real network latency (1s would spuriously fail). const long connectTimeout = Connection::isLocalHost(host) ? 2L : 10L; curl_easy_setopt(impl_->curl, CURLOPT_CONNECTTIMEOUT, connectTimeout); + // Enforce TLS certificate + hostname verification explicitly rather than relying on libcurl's + // build defaults. Harmless for the localhost http:// case (ignored on non-TLS transports); + // essential when the wallet talks to a remote daemon over https://. + curl_easy_setopt(impl_->curl, CURLOPT_SSL_VERIFYPEER, 1L); + curl_easy_setopt(impl_->curl, CURLOPT_SSL_VERIFYHOST, 2L); // Test connection with getinfo. Use a SHORT timeout for the probe on localhost: a healthy // local daemon answers in milliseconds and a warming one returns -28 just as fast, so a long diff --git a/src/util/bootstrap.cpp b/src/util/bootstrap.cpp index 2deb3fa..b216d7e 100644 --- a/src/util/bootstrap.cpp +++ b/src/util/bootstrap.cpp @@ -31,6 +31,31 @@ static bool endsWith(const std::string& s, const std::string& suffix) { return s.compare(s.size() - suffix.size(), suffix.size(), suffix) == 0; } +// Reject archive member names that would escape the extraction root (zip-slip). +// The snapshot legitimately carries sub-paths (blocks/, chainstate/), so — unlike the +// updaters, which flatten to baseName() — we keep the relative path but refuse any entry +// that is absolute, drive/UNC-rooted, or contains a ".." component. +static bool isSafeArchivePath(const std::string& name) { + if (name.empty()) return false; + std::string n = name; + std::replace(n.begin(), n.end(), '\\', '/'); // normalize Windows separators + if (n.front() == '/') return false; // absolute POSIX path + const char c0 = n[0]; + if (n.size() >= 2 && n[1] == ':' && + ((c0 >= 'A' && c0 <= 'Z') || (c0 >= 'a' && c0 <= 'z'))) + return false; // Windows drive letter (C:...) + size_t start = 0; + while (start <= n.size()) { + const size_t slash = n.find('/', start); + const std::string comp = + n.substr(start, slash == std::string::npos ? std::string::npos : slash - start); + if (comp == "..") return false; // path traversal component + if (slash == std::string::npos) break; + start = slash + 1; + } + return true; +} + static size_t writeFileCallback(void* contents, size_t size, size_t nmemb, void* userp) { size_t total = size * nmemb; FILE* fp = static_cast(userp); @@ -383,6 +408,16 @@ bool Bootstrap::extract(const std::string& zipPath, const std::string& dataDir) std::string filename = stat.m_filename; + // *** SECURITY: reject zip-slip / path-traversal entries before building any path *** + // A legitimate snapshot from the project host never contains these; an entry that does + // indicates a malicious/corrupt archive, so abort rather than silently skip. + if (!isSafeArchivePath(filename)) { + DEBUG_LOGF("[Bootstrap] Unsafe archive path rejected: %s\n", filename.c_str()); + setProgress(State::Failed, "Refusing to extract unsafe archive entry: " + filename); + mz_zip_reader_end(&zip); + return false; + } + // *** CRITICAL: Skip wallet.dat *** if (filename == "wallet.dat" || endsWith(filename, "/wallet.dat")) { DEBUG_LOGF("[Bootstrap] Skipping wallet.dat (protected)\n"); @@ -723,10 +758,17 @@ bool Bootstrap::verifyChecksums(const std::string& zipPath, const std::string& b bool haveSHA256 = !sha256Content.empty(); bool haveMD5 = !md5Content.empty(); - if (!haveSHA256 && !haveMD5) { - DEBUG_LOGF("[Bootstrap] Warning: no checksum files available — skipping verification\n"); - // Allow the process to continue (server may not have checksum files yet) - return true; + // SECURITY: fail closed. A SHA-256 checksum is mandatory — refuse to extract an + // unverified multi-GB snapshot (MD5 alone is not acceptable). The project host controls + // the bootstrap origin and publishes DRAGONX.zip.sha256; its absence means either a + // stale server or a downgrade attempt, both of which must block extraction. + // (Residual: the checksum is same-origin as the archive; a pinned signature over the + // snapshot is the deeper fix — see the remediation checklist.) + if (!haveSHA256) { + DEBUG_LOGF("[Bootstrap] No SHA-256 checksum available — refusing to extract unverified data\n"); + setProgress(State::Failed, + "No SHA-256 checksum published for the bootstrap — refusing to extract unverified data."); + return false; } // Determine progress ranges: if both checksums exist, split 0-50% / 50-100% diff --git a/src/util/daemon_updater.cpp b/src/util/daemon_updater.cpp index a869e85..058aa31 100644 --- a/src/util/daemon_updater.cpp +++ b/src/util/daemon_updater.cpp @@ -34,11 +34,17 @@ namespace { // the daemon binaries plus Sapling params, so the caps are well above the miner updater's. constexpr curl_off_t kMaxArchiveBytes = 256LL * 1024 * 1024; // 256 MiB constexpr std::size_t kMaxMemberBytes = 128u * 1024 * 1024; // 128 MiB per extracted file +constexpr std::size_t kMaxMetadataBytes = 16u * 1024 * 1024; // 16 MiB cap for JSON/sig metadata size_t writeStringCb(void* contents, size_t size, size_t nmemb, void* userp) { - static_cast(userp)->append(static_cast(contents), size * nmemb); - return size * nmemb; + auto* s = static_cast(userp); + const size_t n = size * nmemb; + // Hard cap: a chunked response omits Content-Length, so CURLOPT_MAXFILESIZE cannot catch it — + // aborting here bounds memory when a hostile/MITM'd server streams an unbounded body. + if (s->size() + n > kMaxMetadataBytes) return 0; // short count -> curl aborts the transfer + s->append(static_cast(contents), n); + return n; } size_t writeFileCb(void* contents, size_t size, size_t nmemb, void* userp) @@ -130,6 +136,7 @@ std::string DaemonUpdater::httpGet(const std::string& url) curl_easy_setopt(curl, CURLOPT_USERAGENT, "ObsidianDragon/1.0"); curl_easy_setopt(curl, CURLOPT_TIMEOUT, 30L); curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 15L); + curl_easy_setopt(curl, CURLOPT_MAXFILESIZE_LARGE, (curl_off_t)kMaxMetadataBytes); // reject oversized metadata curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1L); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L); const CURLcode res = curl_easy_perform(curl); diff --git a/src/util/xmrig_updater.cpp b/src/util/xmrig_updater.cpp index 047d120..2a55945 100644 --- a/src/util/xmrig_updater.cpp +++ b/src/util/xmrig_updater.cpp @@ -30,11 +30,17 @@ namespace { // a defense even ahead of the checksum check (which only runs once the file is fully downloaded). constexpr curl_off_t kMaxArchiveBytes = 64LL * 1024 * 1024; // 64 MiB constexpr std::size_t kMaxMemberBytes = 64u * 1024 * 1024; // 64 MiB per extracted file +constexpr std::size_t kMaxMetadataBytes = 16u * 1024 * 1024; // 16 MiB cap for JSON/sig metadata size_t writeStringCb(void* contents, size_t size, size_t nmemb, void* userp) { - static_cast(userp)->append(static_cast(contents), size * nmemb); - return size * nmemb; + auto* s = static_cast(userp); + const size_t n = size * nmemb; + // Hard cap: a chunked response omits Content-Length, so CURLOPT_MAXFILESIZE cannot catch it — + // aborting here bounds memory when a hostile/MITM'd server streams an unbounded body. + if (s->size() + n > kMaxMetadataBytes) return 0; // short count -> curl aborts the transfer + s->append(static_cast(contents), n); + return n; } size_t writeFileCb(void* contents, size_t size, size_t nmemb, void* userp) @@ -119,6 +125,7 @@ std::string XmrigUpdater::httpGet(const std::string& url) curl_easy_setopt(curl, CURLOPT_USERAGENT, "ObsidianDragon/1.0"); curl_easy_setopt(curl, CURLOPT_TIMEOUT, 30L); curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 15L); + curl_easy_setopt(curl, CURLOPT_MAXFILESIZE_LARGE, (curl_off_t)kMaxMetadataBytes); // reject oversized metadata curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1L); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L); const CURLcode res = curl_easy_perform(curl); @@ -382,6 +389,12 @@ void XmrigUpdater::installResolved(const std::string& targetDir, const XmrigRele return; } } + } else if (kXmrigRequireSignature) { + // Signatures are required but no key is pinned in this build: fail closed rather than + // silently downgrading to checksum-only (the checksum is same-origin as the archive). + fs::remove(zipPath, ec); + setProgress(State::Failed, "No signing key is pinned in this build — refusing to install."); + return; } } diff --git a/src/wallet/lite_connection_service.cpp b/src/wallet/lite_connection_service.cpp index 67b3ae4..705b19a 100644 --- a/src/wallet/lite_connection_service.cpp +++ b/src/wallet/lite_connection_service.cpp @@ -81,10 +81,31 @@ LiteConnectionSettings defaultLiteConnectionSettings() return settings; } +static bool isLoopbackHostSpec(const std::string& hostPort) +{ + std::string h = hostPort; + const size_t slash = h.find('/'); // strip any path + if (slash != std::string::npos) h = h.substr(0, slash); + if (!h.empty() && h.front() == '[') { // [::1]:port (bracketed IPv6) + const size_t close = h.find(']'); + h = (close == std::string::npos) ? h : h.substr(1, close - 1); + } else { // host or host:port + const size_t colon = h.find(':'); + if (colon != std::string::npos) h = h.substr(0, colon); + } + return h == "localhost" || h == "127.0.0.1" || h == "::1" || startsWith(h, "127."); +} + bool isLiteServerUrlUsable(const std::string& serverUrl) { const std::string normalized = trimCopy(serverUrl); - return startsWith(normalized, "https://") || startsWith(normalized, "http://"); + if (startsWith(normalized, "https://")) return true; + // SECURITY: plaintext http:// is a TLS downgrade for lightwalletd traffic (view keys, + // transactions, addresses). Permit it only for loopback (a local dev lightwalletd); + // reject remote plaintext servers instead of silently accepting the downgrade. + if (startsWith(normalized, "http://")) + return isLoopbackHostSpec(normalized.substr(sizeof("http://") - 1)); + return false; } bool isOfficialLiteServer(const std::string& serverUrl) diff --git a/src/wallet/lite_wallet_controller.cpp b/src/wallet/lite_wallet_controller.cpp index 23cf564..1992ace 100644 --- a/src/wallet/lite_wallet_controller.cpp +++ b/src/wallet/lite_wallet_controller.cpp @@ -1070,7 +1070,16 @@ LiteEncryptionResult LiteWalletController::encryptWallet(std::string passphrase) } out = parseEncryptionOpResponse(bridge_->execute("encrypt", passphrase)); secureWipeLiteSecret(passphrase); - if (out.ok) bridge_->execute("save", ""); // persist the now-encrypted wallet + if (out.ok) { + // Persist the now-encrypted wallet. If the save fails, do NOT report success — the + // on-disk wallet would still be unencrypted, contradicting what the user was told. + const LiteBridgeStringResult saved = bridge_->execute("save", ""); + if (!saved.ok) { + out.ok = false; + out.error = "wallet encrypted in memory but saving to disk failed" + + (saved.error.empty() ? std::string() : (": " + saved.error)); + } + } return out; } @@ -1088,7 +1097,16 @@ LiteEncryptionResult LiteWalletController::decryptWallet(std::string passphrase) } out = parseEncryptionOpResponse(bridge_->execute("decrypt", passphrase)); secureWipeLiteSecret(passphrase); - if (out.ok) bridge_->execute("save", ""); // persist the now-unencrypted wallet + if (out.ok) { + // Persist the now-unencrypted wallet. If the save fails, do NOT report success — the + // on-disk wallet would still be encrypted, contradicting what the user was told. + const LiteBridgeStringResult saved = bridge_->execute("save", ""); + if (!saved.ok) { + out.ok = false; + out.error = "wallet decrypted in memory but saving to disk failed" + + (saved.error.empty() ? std::string() : (": " + saved.error)); + } + } return out; } From 13788b5dd05a5d31f14126f3a926c854cc9946d1 Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 22 Jul 2026 01:56:13 -0500 Subject: [PATCH 2/5] fix(security): harden untrusted-input parsing (Tier 4 audit items) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - export: neutralize CSV/spreadsheet formula injection in transaction export — escapeCSV() now prefixes fields beginning with = + - @ / tab / CR with a single quote so a hostile memo/address cannot execute on open (F13-1). - console: preserve explicitly-quoted empty arguments in ParseConsoleCommandArgs so a "" no longer collapses and shifts later positional RPC params; adds a regression assertion to testConsoleInputModel (F14-1). - send: copy pending payment-URI address/memo with snprintf("%s") instead of strncpy(…, sizeof-1), which does not NUL-terminate on truncation (F8-2). All build clean (full-node + tests) and the suite passes. Co-Authored-By: Claude Fable 5 --- src/ui/windows/console_input_model.cpp | 4 +++- src/ui/windows/export_transactions_dialog.cpp | 21 +++++++++++++------ src/ui/windows/send_tab.cpp | 5 +++-- tests/test_phase4.cpp | 6 ++++++ 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/ui/windows/console_input_model.cpp b/src/ui/windows/console_input_model.cpp index ee9fc91..646395d 100644 --- a/src/ui/windows/console_input_model.cpp +++ b/src/ui/windows/console_input_model.cpp @@ -127,7 +127,9 @@ std::vector ParseConsoleCommandArgs(const std::string& command) if (index >= command.size()) break; std::string token; + bool quoted = false; if (command[index] == '"' || command[index] == '\'') { + quoted = true; // an explicit "" is a real empty argument, not a gap to skip char quote = command[index++]; while (index < command.size() && command[index] != quote) { token += command[index++]; @@ -148,7 +150,7 @@ std::vector ParseConsoleCommandArgs(const std::string& command) token += command[index++]; } } - if (!token.empty()) args.push_back(token); + if (!token.empty() || quoted) args.push_back(token); } return args; } diff --git a/src/ui/windows/export_transactions_dialog.cpp b/src/ui/windows/export_transactions_dialog.cpp index e1b8b1e..084e413 100644 --- a/src/ui/windows/export_transactions_dialog.cpp +++ b/src/ui/windows/export_transactions_dialog.cpp @@ -29,21 +29,30 @@ static std::string s_status; // Helper to escape CSV field static std::string escapeCSV(const std::string& field) { - if (field.find(',') != std::string::npos || - field.find('"') != std::string::npos || - field.find('\n') != std::string::npos) { + // Neutralize spreadsheet formula injection: a field beginning with '=', '+', '-', '@', + // tab, or CR is interpreted as a formula by Excel/LibreOffice, letting an attacker-supplied + // memo/address execute on open. Prefix such fields with a single quote so they render as text. + std::string safe = field; + if (!safe.empty()) { + const char c0 = safe.front(); + if (c0 == '=' || c0 == '+' || c0 == '-' || c0 == '@' || c0 == '\t' || c0 == '\r') + safe.insert(safe.begin(), '\''); + } + if (safe.find(',') != std::string::npos || + safe.find('"') != std::string::npos || + safe.find('\n') != std::string::npos) { // Escape quotes and wrap in quotes std::string escaped; - escaped.reserve(field.size() + 4); + escaped.reserve(safe.size() + 4); escaped += '"'; - for (char c : field) { + for (char c : safe) { if (c == '"') escaped += "\"\""; else escaped += c; } escaped += '"'; return escaped; } - return field; + return safe; } void ExportTransactionsDialog::show() diff --git a/src/ui/windows/send_tab.cpp b/src/ui/windows/send_tab.cpp index 34bedb0..90cade6 100644 --- a/src/ui/windows/send_tab.cpp +++ b/src/ui/windows/send_tab.cpp @@ -1110,9 +1110,10 @@ void RenderSendTab(App* app) // Handle pending payment from URI if (app->hasPendingPayment()) { - strncpy(s_to_address, app->getPendingToAddress().c_str(), sizeof(s_to_address) - 1); + // snprintf always NUL-terminates; strncpy(…, sizeof-1) does not on truncation. + snprintf(s_to_address, sizeof(s_to_address), "%s", app->getPendingToAddress().c_str()); s_amount = app->getPendingAmount(); - strncpy(s_memo, app->getPendingMemo().c_str(), sizeof(s_memo) - 1); + snprintf(s_memo, sizeof(s_memo), "%s", app->getPendingMemo().c_str()); app->clearPendingPayment(); } diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index 2bb4afd..eb6e319 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -2378,6 +2378,12 @@ void testConsoleInputModel() auto args = dragonx::ui::ParseConsoleCommandArgs("z_sendmany \"from addr\" [{\"address\":\"zs\",\"amount\":1.25}] true"); EXPECT_EQ(args.size(), static_cast(4)); EXPECT_EQ(args[1], std::string("from addr")); + // An explicitly-quoted empty argument must be preserved (not dropped), otherwise + // later positional RPC params silently shift by one (F14-1). + auto emptyArgs = dragonx::ui::ParseConsoleCommandArgs("walletpassphrase \"\" 60"); + EXPECT_EQ(emptyArgs.size(), static_cast(3)); + EXPECT_EQ(emptyArgs[1], std::string("")); + EXPECT_EQ(emptyArgs[2], std::string("60")); auto call = dragonx::ui::BuildConsoleRpcCall("setgenerate true 4"); EXPECT_TRUE(call.valid); EXPECT_EQ(call.method, std::string("setgenerate")); From c6bd4eb733aa655fb146c00faf6c68bbd8814faa Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 22 Jul 2026 02:02:17 -0500 Subject: [PATCH 3/5] fix(build): verify pinned build inputs (appimagetool, Sapling params) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Release-pipeline supply-chain hardening from the audit (F15-3/4, F15-6): - appimagetool: stop fetching the moving "continuous" AppImageKit tag and running it unverified on the release builder. Pin appimagetool 1.9.0 and verify its SHA-256 (46fdd785…83d1) before exec, refusing to package on mismatch; a system-installed appimagetool (maintainer's own) is still used as-is. Applied in both build.sh and scripts/create-appimage.sh. - setup.sh --sapling: the z.cash param download is plain HTTPS with no signature; verify both params against their fixed canonical SHA-256 (the same values already pinned in build-lite-backend-artifact.sh) and delete the file on mismatch. All three scripts pass bash -n. Co-Authored-By: Claude Fable 5 --- build.sh | 28 +++++++++++++++++++--------- scripts/create-appimage.sh | 27 ++++++++++++++++++--------- setup.sh | 23 +++++++++++++++++++---- 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/build.sh b/build.sh index c3ea180..b9d49f7 100755 --- a/build.sh +++ b/build.sh @@ -478,18 +478,28 @@ APPRUN done [[ -f "$bd/_deps/sdl3-build/libSDL3.so" ]] && cp "$bd/_deps/sdl3-build/libSDL3.so"* "$APPDIR/usr/lib/" 2>/dev/null || true - # appimagetool + # appimagetool — pinned to a tagged release and SHA-256 verified before we exec it. + # The old "continuous" tag is a MOVING build fetched over the network and run on the release + # builder; a compromised/MITM'd artifact would execute here. Verify, or refuse to package. + local APPIMAGETOOL_URL="https://github.com/AppImage/appimagetool/releases/download/1.9.0/appimagetool-x86_64.AppImage" + local APPIMAGETOOL_SHA256="46fdd785094c7f6e545b61afcfb0f3d98d8eab243f644b4b17698c01d06083d1" local APPIMAGETOOL="" if command -v appimagetool &>/dev/null; then - APPIMAGETOOL="appimagetool" - elif [[ -f "$bd/appimagetool-x86_64.AppImage" ]]; then - APPIMAGETOOL="$bd/appimagetool-x86_64.AppImage" + APPIMAGETOOL="appimagetool" # maintainer's own trusted system install else - info "Downloading appimagetool ..." - wget -q -O "$bd/appimagetool-x86_64.AppImage" \ - "https://github.com/AppImage/AppImageKit/releases/download/continuous/appimagetool-x86_64.AppImage" - chmod +x "$bd/appimagetool-x86_64.AppImage" - APPIMAGETOOL="$bd/appimagetool-x86_64.AppImage" + local at="$bd/appimagetool-x86_64.AppImage" + # Re-verify any cached copy too; a stale unverified download must not be trusted. + if [[ ! -f "$at" ]] || ! echo "${APPIMAGETOOL_SHA256} ${at}" | sha256sum -c --status; then + info "Downloading appimagetool 1.9.0 (pinned) ..." + wget -q -O "$at" "$APPIMAGETOOL_URL" + if ! echo "${APPIMAGETOOL_SHA256} ${at}" | sha256sum -c --status; then + err "appimagetool SHA-256 verification failed — refusing to use it" + rm -f "$at" + return 1 + fi + chmod +x "$at" + fi + APPIMAGETOOL="$at" fi local ARCH diff --git a/scripts/create-appimage.sh b/scripts/create-appimage.sh index 794a482..5e28faf 100755 --- a/scripts/create-appimage.sh +++ b/scripts/create-appimage.sh @@ -24,18 +24,27 @@ if [ ! -f "${BUILD_DIR}/bin/ObsidianDragon" ]; then exit 1 fi -# Check for appimagetool +# Check for appimagetool — pinned to a tagged release and SHA-256 verified before we exec it. +# The old "continuous" tag is a moving, unverified network download that runs on the release +# builder; verify it or refuse to package. +APPIMAGETOOL_URL="https://github.com/AppImage/appimagetool/releases/download/1.9.0/appimagetool-x86_64.AppImage" +APPIMAGETOOL_SHA256="46fdd785094c7f6e545b61afcfb0f3d98d8eab243f644b4b17698c01d06083d1" APPIMAGETOOL="" if command -v appimagetool &> /dev/null; then - APPIMAGETOOL="appimagetool" -elif [ -f "${BUILD_DIR}/appimagetool-x86_64.AppImage" ]; then - APPIMAGETOOL="${BUILD_DIR}/appimagetool-x86_64.AppImage" + APPIMAGETOOL="appimagetool" # maintainer's own trusted system install else - print_status "Downloading appimagetool..." - wget -q -O "${BUILD_DIR}/appimagetool-x86_64.AppImage" \ - "https://github.com/AppImage/AppImageKit/releases/download/continuous/appimagetool-x86_64.AppImage" - chmod +x "${BUILD_DIR}/appimagetool-x86_64.AppImage" - APPIMAGETOOL="${BUILD_DIR}/appimagetool-x86_64.AppImage" + AT="${BUILD_DIR}/appimagetool-x86_64.AppImage" + if [ ! -f "$AT" ] || ! echo "${APPIMAGETOOL_SHA256} ${AT}" | sha256sum -c --status; then + print_status "Downloading appimagetool 1.9.0 (pinned)..." + wget -q -O "$AT" "$APPIMAGETOOL_URL" + if ! echo "${APPIMAGETOOL_SHA256} ${AT}" | sha256sum -c --status; then + print_error "appimagetool SHA-256 verification failed — refusing to use it" + rm -f "$AT" + exit 1 + fi + chmod +x "$AT" + fi + APPIMAGETOOL="$AT" fi print_status "Creating AppDir structure..." diff --git a/setup.sh b/setup.sh index 2340bee..dd20983 100755 --- a/setup.sh +++ b/setup.sh @@ -391,11 +391,26 @@ elif $SETUP_SAPLING; then SPEND_URL="https://z.cash/downloads/sapling-spend.params" OUTPUT_URL="https://z.cash/downloads/sapling-output.params" + # Consensus-critical MPC parameters with fixed, well-known SHA-256 (identical across every + # Zcash-family node; also pinned in scripts/build-lite-backend-artifact.sh). z.cash is + # plain HTTPS with no signature, so verify the digest and refuse a tampered/corrupt file. + SPEND_SHA256="8e48ffd23abb3a5fd9c5589204f32d9c31285a04b78096ba40a79b75677efc13" + OUTPUT_SHA256="2f0ebbcbb9bb0bcffe95a397e7eba89c29eb4dde6191c339db88570e3f3fb0e4" - curl -fSL -o "$PARAMS_DIR/sapling-spend.params" "$SPEND_URL" && \ - ok "Downloaded sapling-spend.params" - curl -fSL -o "$PARAMS_DIR/sapling-output.params" "$OUTPUT_URL" && \ - ok "Downloaded sapling-output.params" + if curl -fSL -o "$PARAMS_DIR/sapling-spend.params" "$SPEND_URL" \ + && echo "${SPEND_SHA256} $PARAMS_DIR/sapling-spend.params" | sha256sum -c --status; then + ok "Downloaded + verified sapling-spend.params" + else + rm -f "$PARAMS_DIR/sapling-spend.params" + err "sapling-spend.params download or SHA-256 verification failed — not installed" + fi + if curl -fSL -o "$PARAMS_DIR/sapling-output.params" "$OUTPUT_URL" \ + && echo "${OUTPUT_SHA256} $PARAMS_DIR/sapling-output.params" | sha256sum -c --status; then + ok "Downloaded + verified sapling-output.params" + else + rm -f "$PARAMS_DIR/sapling-output.params" + err "sapling-output.params download or SHA-256 verification failed — not installed" + fi fi else skip "Sapling params not found (use --sapling to download, or they'll be extracted at runtime from embedded builds)" From 080f3d966817506079538715e973bae6efe97b69 Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 22 Jul 2026 02:12:45 -0500 Subject: [PATCH 4/5] fix(build): enforce build-from-source for the lite backend (F15-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lite-backend "signature verification" was attestation theater: build-lite-backend-artifact.sh recorded a self-attested signature_verification.verification_status="verified" with no cryptographic check (the "verified" SHA was simply the artifact's own SHA), and CMake's DRAGONX_LITE_BACKEND_REQUIRE_SIGNATURE gate trusted it — so a hand-edited manifest could link an arbitrary prebuilt backend into ObsidianDragonLite. Per the chosen remediation (build-from-source is the trust root): - build-lite-backend-artifact.sh: reject --artifact/--no-build (prebuilt) and all --signature-* flags; the backend is always built from the vendored in-tree source (third_party/silentdragonxlite). Record built_from_source:true in the manifest provenance. Usage/header updated. - CMakeLists.txt: remove the DRAGONX_LITE_BACKEND_REQUIRE_SIGNATURE option and its manifest signature-verification gate (kept the real required-symbol inventory check); update the config summary line. Full-node CMake reconfigure + build pass; script bash -n clean and the removed flags now hard-error. (The lite-backend link path needs cargo + backend and was not runtime-tested in this environment.) Co-Authored-By: Claude Fable 5 --- CMakeLists.txt | 34 ++-------- scripts/build-lite-backend-artifact.sh | 91 +++++--------------------- 2 files changed, 22 insertions(+), 103 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7ef092b..6832f1c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -53,7 +53,6 @@ set_property(CACHE DRAGONX_LITE_BACKEND_LINK_MODE PROPERTY STRINGS imported) set(DRAGONX_LITE_BACKEND_ABI "sdxl-c-v1" CACHE STRING "Expected lite backend C ABI version") set(DRAGONX_LITE_BACKEND_SYMBOLS_FILE "" CACHE FILEPATH "Path to generated lite backend exported-symbol inventory") set(DRAGONX_LITE_BACKEND_MANIFEST "" CACHE FILEPATH "Optional path to generated lite backend artifact manifest") -option(DRAGONX_LITE_BACKEND_REQUIRE_SIGNATURE "Require verified signature metadata in the lite backend artifact manifest" OFF) set(DRAGONX_LITE_BACKEND_REQUIRED_SYMBOLS litelib_wallet_exists litelib_initialize_new @@ -126,31 +125,12 @@ if(DRAGONX_ENABLE_LITE_BACKEND) if(DRAGONX_LITE_BACKEND_MANIFEST AND NOT EXISTS "${DRAGONX_LITE_BACKEND_MANIFEST}") message(FATAL_ERROR "DRAGONX_LITE_BACKEND_MANIFEST does not exist: ${DRAGONX_LITE_BACKEND_MANIFEST}") endif() - if(DRAGONX_LITE_BACKEND_REQUIRE_SIGNATURE) - if(NOT DRAGONX_LITE_BACKEND_MANIFEST) - message(FATAL_ERROR "DRAGONX_LITE_BACKEND_REQUIRE_SIGNATURE requires DRAGONX_LITE_BACKEND_MANIFEST") - endif() - file(READ "${DRAGONX_LITE_BACKEND_MANIFEST}" DRAGONX_LITE_BACKEND_MANIFEST_JSON) - string(JSON DRAGONX_LITE_SIGNATURE_STATUS ERROR_VARIABLE DRAGONX_LITE_SIGNATURE_STATUS_ERROR GET "${DRAGONX_LITE_BACKEND_MANIFEST_JSON}" signature_verification verification_status) - if(DRAGONX_LITE_SIGNATURE_STATUS_ERROR) - message(FATAL_ERROR "DRAGONX_LITE_BACKEND_MANIFEST is missing signature verification status") - endif() - if(NOT DRAGONX_LITE_SIGNATURE_STATUS STREQUAL "verified") - message(FATAL_ERROR "DRAGONX_LITE_BACKEND_REQUIRE_SIGNATURE requires verified signature metadata") - endif() - string(JSON DRAGONX_LITE_SIGNATURE_VERIFIED_SHA ERROR_VARIABLE DRAGONX_LITE_SIGNATURE_VERIFIED_SHA_ERROR GET "${DRAGONX_LITE_BACKEND_MANIFEST_JSON}" signature_verification verified_artifact_sha256) - string(JSON DRAGONX_LITE_ARTIFACT_SHA ERROR_VARIABLE DRAGONX_LITE_ARTIFACT_SHA_ERROR GET "${DRAGONX_LITE_BACKEND_MANIFEST_JSON}" artifact sha256) - if(DRAGONX_LITE_SIGNATURE_VERIFIED_SHA_ERROR OR DRAGONX_LITE_ARTIFACT_SHA_ERROR) - message(FATAL_ERROR "DRAGONX_LITE_BACKEND_MANIFEST is missing artifact/signature SHA-256 metadata") - endif() - if(NOT DRAGONX_LITE_SIGNATURE_VERIFIED_SHA STREQUAL DRAGONX_LITE_ARTIFACT_SHA) - message(FATAL_ERROR "DRAGONX_LITE_BACKEND_MANIFEST signature metadata does not verify the artifact SHA-256") - endif() - string(JSON DRAGONX_LITE_SIGNATURE_PERFORMED ERROR_VARIABLE DRAGONX_LITE_SIGNATURE_PERFORMED_ERROR GET "${DRAGONX_LITE_BACKEND_MANIFEST_JSON}" signature_verification verification_performed) - if(DRAGONX_LITE_SIGNATURE_PERFORMED_ERROR OR NOT DRAGONX_LITE_SIGNATURE_PERFORMED) - message(FATAL_ERROR "DRAGONX_LITE_BACKEND_REQUIRE_SIGNATURE requires verification_performed=true") - endif() - endif() + # Note (F15-1): the former signature-metadata gate was removed. It trusted a + # "verification_status: verified" field that scripts/build-lite-backend-artifact.sh + # self-attested with no cryptographic check (the "verified" SHA was just the artifact's + # own SHA). The trust root is now build-from-source: that script builds the backend from + # the vendored in-tree source and refuses prebuilt artifacts, so the library linked here + # is the one built from reviewed source. The required-symbol inventory check above stays. add_library(dragonx_lite_backend UNKNOWN IMPORTED) set_target_properties(dragonx_lite_backend PROPERTIES @@ -1097,5 +1077,5 @@ message(STATUS " Lite backend: ${DRAGONX_LITE_BACKEND_READY}") message(STATUS " Lite lib: ${DRAGONX_LITE_BACKEND_LIBRARY}") message(STATUS " Lite symbols: ${DRAGONX_LITE_BACKEND_SYMBOLS_FILE}") message(STATUS " Lite manifest: ${DRAGONX_LITE_BACKEND_MANIFEST}") -message(STATUS " Lite signature: ${DRAGONX_LITE_BACKEND_REQUIRE_SIGNATURE}") +message(STATUS " Lite trust: built-from-source (vendored third_party/silentdragonxlite)") message(STATUS "") diff --git a/scripts/build-lite-backend-artifact.sh b/scripts/build-lite-backend-artifact.sh index e83b021..66292c1 100755 --- a/scripts/build-lite-backend-artifact.sh +++ b/scripts/build-lite-backend-artifact.sh @@ -67,25 +67,9 @@ Options: --backend-dir PATH SilentDragonXLite/lib source directory. --silentdragonxlitelib-dir PATH Override the wrapper's silentdragonxlitelib dependency path. --out-dir PATH Output directory for copied artifact and metadata. - --artifact PATH Inventory an existing artifact instead of building. - --no-build Do not run cargo; requires --artifact. --reproducible Add deterministic Rust path remaps for clean builds. --remap-path-prefix FROM=TO Extra rustc path remap used with --reproducible. --builder NAME Redacted builder/provenance label. Default: local. - --signature-required Fail if verified signature metadata is not supplied. - --signature-file PATH Existing sidecar signature file to record. - --signature-format FORMAT Signature format: minisign, gpg, sigstore, external, or other. - --signature-verification-tool T Verification tool and version used by the release builder. - --signature-verification-command C - Verification command already run by the release builder. - --signature-key-fingerprint F Reviewed public-key fingerprint, when applicable. - --signature-certificate-identity ID - Reviewed certificate identity, when applicable. - --signature-certificate-issuer I - Reviewed certificate issuer, when applicable. - --signature-transparency-log-url URL - Transparency log entry, when applicable. - --signature-verified-sha256 SHA Artifact SHA-256 verified by the signature check. -j, --jobs N Cargo parallel jobs. --cargo-arg ARG Extra argument forwarded to cargo build. -h, --help Show this help. @@ -95,9 +79,13 @@ Outputs: //lite-backend-symbols.txt //lite-backend-artifact-manifest.json -The script captures symbols, checksums, and optional read-only signature -verification metadata only. It does not load the library, resolve function -pointers, call SDXL, sign, upload, or publish artifacts. +The lite backend is always built from the vendored in-tree source +(third_party/silentdragonxlite), which is the trust root. Prebuilt artifacts +and self-attested signature metadata are NOT accepted (F15-1) — the previous +scheme only recorded an unverified "verified" claim. The script captures the +freshly-built artifact's symbols and checksum, and records build provenance. +It does not load the library, resolve function pointers, call SDXL, sign, +upload, or publish artifacts. EOF } @@ -166,15 +154,8 @@ while [[ $# -gt 0 ]]; do OUT_DIR="$(absolute_path "$2")" shift 2 ;; - --artifact) - [[ $# -ge 2 ]] || die "--artifact requires a value" - ARTIFACT_PATH="$(absolute_path "$2")" - BUILD_ARTIFACT=false - shift 2 - ;; - --no-build) - BUILD_ARTIFACT=false - shift + --artifact|--no-build) + die "$1 was removed (F15-1): the lite backend must be built from the vendored in-tree source (third_party/silentdragonxlite); prebuilt artifacts are no longer accepted." ;; --reproducible) REPRODUCIBLE=true @@ -191,54 +172,11 @@ while [[ $# -gt 0 ]]; do BUILDER="$2" shift 2 ;; - --signature-required) - SIGNATURE_REQUIRED=true - shift - ;; - --signature-file|--signature-path) - [[ $# -ge 2 ]] || die "$1 requires a value" - SIGNATURE_FILE="$(absolute_path "$2")" - shift 2 - ;; - --signature-format) - [[ $# -ge 2 ]] || die "--signature-format requires a value" - SIGNATURE_FORMAT="$2" - shift 2 - ;; - --signature-verification-tool|--signature-tool) - [[ $# -ge 2 ]] || die "$1 requires a value" - SIGNATURE_VERIFICATION_TOOL="$2" - shift 2 - ;; - --signature-verification-command) - [[ $# -ge 2 ]] || die "--signature-verification-command requires a value" - SIGNATURE_VERIFICATION_COMMAND="$2" - shift 2 - ;; - --signature-key-fingerprint) - [[ $# -ge 2 ]] || die "--signature-key-fingerprint requires a value" - SIGNATURE_KEY_FINGERPRINT="$2" - shift 2 - ;; - --signature-certificate-identity) - [[ $# -ge 2 ]] || die "--signature-certificate-identity requires a value" - SIGNATURE_CERTIFICATE_IDENTITY="$2" - shift 2 - ;; - --signature-certificate-issuer) - [[ $# -ge 2 ]] || die "--signature-certificate-issuer requires a value" - SIGNATURE_CERTIFICATE_ISSUER="$2" - shift 2 - ;; - --signature-transparency-log-url) - [[ $# -ge 2 ]] || die "--signature-transparency-log-url requires a value" - SIGNATURE_TRANSPARENCY_LOG_URL="$2" - shift 2 - ;; - --signature-verified-sha256) - [[ $# -ge 2 ]] || die "--signature-verified-sha256 requires a value" - SIGNATURE_VERIFIED_SHA256="$2" - shift 2 + --signature-required|--signature-file|--signature-path|--signature-format|\ +--signature-verification-tool|--signature-tool|--signature-verification-command|\ +--signature-key-fingerprint|--signature-certificate-identity|--signature-certificate-issuer|\ +--signature-transparency-log-url|--signature-verified-sha256) + die "signature-attestation flags were removed (F15-1): they recorded a self-attested \"verified\" claim without running any cryptographic verifier. The lite backend is built from the vendored in-tree source, which is the trust root." ;; -j|--jobs) [[ $# -ge 2 ]] || die "--jobs requires a value" @@ -766,6 +704,7 @@ MANIFEST_FILE="$PLATFORM_OUT_DIR/lite-backend-artifact-manifest.json" printf ' },\n' printf ' "provenance": {\n' printf ' "owner_ready": true,\n' + printf ' "built_from_source": true,\n' printf ' "metadata_provided": true,\n' printf ' "source": '; json_escape "$BACKEND_SOURCE_DIR"; printf ',\n' printf ' "cargo_build_source": '; json_escape "$BUILD_BACKEND_DIR"; printf ',\n' From 0e5533f6b9d3fa3271d7d70ad942547ba9bd34e1 Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 22 Jul 2026 10:44:01 -0500 Subject: [PATCH 5/5] fix(build): exit 0 on a successful single-platform release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build.sh runs under `set -e`, and its final line was `[[ -d release/mac ]] && echo …`. On a build that did not produce a mac artifact (e.g. --win-release), the `[[ -d ]]` test is false; set -e exempts the left side of an && so the script does not abort, but that non-zero status becomes the script's exit code — making a fully successful build report failure (and breaking CI status). Add an explicit `exit 0` at the end; real failures already exit 1 at their point of failure. Co-Authored-By: Claude Fable 5 --- build.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build.sh b/build.sh index b9d49f7..f3629b8 100755 --- a/build.sh +++ b/build.sh @@ -1306,3 +1306,9 @@ if $DO_LINUX || $DO_WIN || $DO_MAC; then [[ -d "$SCRIPT_DIR/release/windows" ]] && echo -e " ${CYAN}windows/${NC} — .exe + .zip" [[ -d "$SCRIPT_DIR/release/mac" ]] && echo -e " ${CYAN}mac/${NC} — .app + .dmg" fi + +# Reaching here means the build completed (real failures exit 1 at their point of failure). +# Exit 0 explicitly: the final `[[ -d release/mac ]] && echo` above returns non-zero on a +# non-mac build — and since set -e exempts the left side of an &&, that status would otherwise +# become the script's exit code and make a successful build report failure (e.g. to CI). +exit 0