fix(security): remediate high/medium findings from branch audit

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 <noreply@anthropic.com>

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;

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);
     }

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();

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

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<FILE*>(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%

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<std::string*>(userp)->append(static_cast<char*>(contents), size * nmemb);
-    return size * nmemb;
+    auto* s = static_cast<std::string*>(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<char*>(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);

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<std::string*>(userp)->append(static_cast<char*>(contents), size * nmemb);
-    return size * nmemb;
+    auto* s = static_cast<std::string*>(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<char*>(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;
         }
     }
 

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)

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;
 }