fix(security): apply audit remediations to dev (ported + dev-only)

Dev-branch audit (docs/_archive/security-audit-dev-2026-07-22.md) re-found the
master issues (absent on dev) plus new ones in dev-only code. Applied here;
full-node build + test suite green; the subtle fixes were adversarially re-verified.

Ported from the master remediation (adapted to dev's code):
- bootstrap: reject zip-slip / path-traversal archive members (isSafeArchivePath)
  before writing (S2-1). (dev already fail-closes on a missing checksum.)
- xmrig updater: fail closed when a signature is required but no key is pinned (F1-1).
- http_download.httpGetString: 16 MiB hard cap + MAXFILESIZE on the shared
  metadata/price fetch (F1-2 / caps the updater + exchange paths at one site).
- rpc_client: explicit SSL_VERIFYPEER/VERIFYHOST (F4-1) and a 256 MiB response
  cap in WriteCallback (F4-3).
- lite_connection_service: reject remote http:// lite servers, loopback only (L1-1).
  Loopback is matched by a strict dotted-decimal 127.0.0.0/8 check (not a
  startsWith("127.") prefix, which would wrongly accept 127.0.0.1.evil.com), with
  userinfo/fragment stripping.
- lite controller: propagate encrypt/decrypt save() failure instead of reporting
  success (F7-1).
- xmrig_manager: chmod(0600) the pool config before writing secrets (F5-2).
- app: clear the copied secret from the OS clipboard on shutdown (F3b-1).
- export_transactions: neutralize CSV/spreadsheet formula injection (F13-1).
- build pipeline: build-from-source lite backend + remove the self-attested
  CMake signature gate (F15-1); pinned+verified appimagetool (F15-3/4);
  verified Sapling params in setup.sh (F15-6); build.sh exits 0 on success.
  (F14-1 empty-quoted-arg and F8-2 NUL-termination were already fixed on dev.)

Dev-only findings:
- rpc_client.callRaw: scrub the raw buffer + parsed tree (templated scrubJsonSecrets
  for ordered_json) so console dumpprivkey/z_exportkey keys don't linger in freed
  heap (N1-1).
- seed_wallet_creator: wipe the exported mnemonic on the failure path so a discarded
  failed result never carries a live seed (W1-2).
- export_all_keys: write the plaintext key dump 0600 + atomically via
  writeFileAtomically (U1-2).
- chat_database: restrict chat_messages.sqlite and its WAL/SHM sidecars to owner-only (C3-1).

Not done (need a decision, documented in the report):
- Chat header metadata (cid/z/p) rides outside the AEAD (C1/C2) — binding it is a
  wire-protocol change requiring SilentDragonXLite interop review.
- Bootstrap lacks an offline-rooted signature (S2-2 residual) — needs signing infra.
- Console scrollback retains console-typed key-export output in plaintext (N1-1
  residual) — inherent to an echoing console; would need output redaction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/app.cpp

@@ -5320,6 +5320,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();
@@ -5522,10 +5527,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;
@@ -5536,6 +5540,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

@@ -573,6 +573,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/chat/chat_database.cpp

@@ -228,6 +228,17 @@ bool ChatDatabase::ensureOpen()
     exec("PRAGMA journal_mode=WAL");
     exec("PRAGMA synchronous=NORMAL");
 
+    // C3-1: restrict the chat DB and its WAL/SHM sidecars to owner-only. sqlite creates them with
+    // umask-derived permissions (often world/group-readable); they hold per-row nonces + AEAD
+    // ciphertext of the user's messages. Best-effort (errors swallowed; a no-op-ish on Windows).
+    {
+        std::error_code perr;
+        const auto ownerOnly = std::filesystem::perms::owner_read | std::filesystem::perms::owner_write;
+        std::filesystem::permissions(database_path_, ownerOnly, std::filesystem::perm_options::replace, perr);
+        std::filesystem::permissions(database_path_ + "-wal", ownerOnly, std::filesystem::perm_options::replace, perr);
+        std::filesystem::permissions(database_path_ + "-shm", ownerOnly, std::filesystem::perm_options::replace, perr);
+    }
+
     if (!createSchema()) {
         close();
         return false;

src/daemon/seed_wallet_creator.cpp

@@ -136,6 +136,14 @@ SeedWalletResult SeedWalletCreator::create(bool keepDatadir,
         }
     }
 
+    // W1-2: never hand back a live seed on a failure path. If the mnemonic was exported but a
+    // later step failed (empty address, or z_getnewaddress threw), the caller discards this
+    // result without wiping it, which would leave the seed resident. Success keeps it deliberately.
+    if (!r.ok && !r.seedPhrase.empty()) {
+        sodium_memzero(&r.seedPhrase[0], r.seedPhrase.size());
+        r.seedPhrase.clear();
+    }
+
     // 7. Stop the isolated node (graceful; it flushes its tiny empty chain quickly).
     cli.disconnect();
     temp.stop(20000);

src/daemon/xmrig_manager.cpp

@@ -208,19 +208,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

@@ -25,9 +25,11 @@ namespace {
 
 // Recursively zero every string value in a JSON tree in place — used to wipe a discarded parse tree
 // that held a secret (B7). Operates on the underlying std::string buffers via get_ref.
-void scrubJsonSecrets(nlohmann::json& j) {
+// Templated so it works on both nlohmann::json and nlohmann::ordered_json (callRaw uses the latter).
+template <typename J>
+void scrubJsonSecrets(J& j) {
     if (j.is_string()) {
-        auto& s = j.get_ref<std::string&>();
+        auto& s = j.template get_ref<std::string&>();
         if (!s.empty()) sodium_memzero(&s[0], s.size());
     } else if (j.is_object() || j.is_array()) {
         for (auto& el : j) scrubJsonSecrets(el);
@@ -96,6 +98,10 @@ 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 yet prevents 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;
 }
@@ -202,6 +208,10 @@ 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 on the localhost http:// case; essential for a remote https daemon.
+    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
@@ -508,14 +518,22 @@ std::string RPCClient::callRaw(const std::string& method, const json& params)
     }
 
     auto& result = oj["result"];
+    std::string out;
     if (result.is_null()) {
-        return "null";
+        out = "null";
     } else if (result.is_string()) {
         // Return the raw string (not JSON-encoded) — caller wraps as needed
-        return result.get<std::string>();
+        out = result.get<std::string>();
     } else {
-        return result.dump(4);
+        out = result.dump(4);
     }
+    // B7: this raw path serves arbitrary console commands including dumpprivkey / z_exportkey,
+    // whose response carries plaintext key material. Zero the raw buffer and the parsed tree so
+    // the secret does not linger in freed heap (matching callSecret). The single returned copy is
+    // the caller's to manage.
+    scrubJsonSecrets(oj);
+    if (!response_data.empty()) sodium_memzero(&response_data[0], response_data.size());
+    return out;
 }
 
 void RPCClient::doRPC(const std::string& method, const json& params, Callback cb, ErrorCallback err)

src/ui/windows/export_all_keys_dialog.cpp

@@ -211,12 +211,11 @@ void ExportAllKeysDialog::render(App* app)
                         std::string filepath = configDir + "/" + filename;
                         bool writeOk = false;
                         if (exported > 0) {
-                            std::ofstream file(filepath);
-                            if (file.is_open()) {
-                                file << keys;
-                                file.close();
-                                writeOk = true;
-                            }
+                            // Write the plaintext private keys 0600 + atomically (never
+                            // world/group-readable, never a half-written file) — the same restricted
+                            // atomic-write idiom the PIN vault uses. A default ofstream would create
+                            // the key dump with umask-derived (often 0644) permissions.
+                            writeOk = util::Platform::writeFileAtomically(filepath, keys, /*restrictPermissions=*/true);
                         }
                         if (!keys.empty()) sodium_memzero(&keys[0], keys.size());  // don't leave every key in freed heap
 

src/ui/windows/export_transactions_dialog.cpp

@@ -36,21 +36,30 @@ static bool s_exporting = false;
 // 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()

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

src/util/http_download.cpp

@@ -19,10 +19,19 @@ namespace util {
 
 namespace {
 
+// Cap for in-memory text/JSON responses (release metadata, price/candle data). Far above any
+// legitimate body, but bounds memory if a hostile/MITM'd server streams an unbounded response.
+constexpr std::size_t kMaxMetadataBytes = 16u * 1024 * 1024;
+
 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 (short count) makes curl fail the transfer instead of exhausting memory.
+    if (s->size() + n > kMaxMetadataBytes) return 0;
+    s->append(static_cast<char*>(contents), n);
+    return n;
 }
 
 size_t writeFileCb(void* contents, size_t size, size_t nmemb, void* userp)
@@ -52,6 +61,7 @@ std::string httpGetString(const std::string& url, const char* logTag)
     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, static_cast<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

@@ -314,6 +314,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,56 @@ LiteConnectionSettings defaultLiteConnectionSettings()
     return settings;
 }
 
+// Strict dotted-decimal check for the 127.0.0.0/8 loopback block: exactly four numeric octets
+// (each 0-255) with the first equal to 127. This must NOT be a prefix match — "127.0.0.1.evil.com"
+// and "127.evil.com" are attacker-controlled DNS names that a startsWith("127.") test would wrongly
+// treat as loopback, reopening the plaintext-downgrade hole.
+static bool isNumericIpv4Loopback(const std::string& h)
+{
+    int parts = 0;
+    size_t start = 0;
+    while (true) {
+        const size_t dot = h.find('.', start);
+        const std::string seg = h.substr(start, dot == std::string::npos ? std::string::npos : dot - start);
+        if (seg.empty() || seg.size() > 3) return false;
+        int v = 0;
+        for (char c : seg) { if (c < '0' || c > '9') return false; v = v * 10 + (c - '0'); }
+        if (v > 255) return false;
+        if (parts == 0 && v != 127) return false;   // 127.0.0.0/8 only
+        ++parts;
+        if (dot == std::string::npos) break;
+        start = dot + 1;
+    }
+    return parts == 4;
+}
+
+static bool isLoopbackLiteHostSpec(const std::string& hostPort)
+{
+    std::string h = hostPort;
+    const size_t term = h.find_first_of("/?#");   // strip path/query/fragment
+    if (term != std::string::npos) h = h.substr(0, term);
+    const size_t at = h.rfind('@');               // strip userinfo (user:pass@host)
+    if (at != std::string::npos) h = h.substr(at + 1);
+    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 == "::1" || isNumericIpv4Loopback(h);
+}
+
 bool isLiteServerUrlUsable(const std::string& serverUrl)
 {
     const std::string normalized = liteTrimCopy(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 isLoopbackLiteHostSpec(normalized.substr(sizeof("http://") - 1));
+    return false;
 }
 
 bool isOfficialLiteServer(const std::string& serverUrl)

src/wallet/lite_wallet_controller.cpp

@@ -1066,7 +1066,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 auto 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;
 }
 
@@ -1084,7 +1093,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 auto 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;
 }