From df14533ad36c58d4c1a5bf500cbc083d801e8331 Mon Sep 17 00:00:00 2001 From: DanS Date: Sun, 5 Jul 2026 12:54:05 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20Tier-1=20UX=20robustness=20=E2=80=94=20b?= =?UTF-8?q?ackup/export=20integrity,=20verified=20installs,=20input=20vali?= =?UTF-8?q?dation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the app-wide robustness audit (calibrated to the import-key dialog). These are the safety-critical fixes; several could cost users funds: - Backup/export false success (FUND LOSS): exportAllKeys pre-seeded a header, so a keyless result (the usual case when the wallet is encrypted+locked) still looked non-empty and backupWallet wrote a private-key-less file and reported "Backup saved". Now exportAllKeys returns an exported count; backupWallet and the export-all dialog refuse to write / report success on 0 keys, disclose the count, flag partial results as INCOMPLETE, and the backup dialog confirms before overwriting an existing file (+ trims the path). - Bootstrap: fail CLOSED — refuse to install an unverified multi-GB archive when no checksum is published (was `return true`), mirroring the xmrig/daemon updaters. - Restore-from-seed: require a valid BIP39 word count (12/15/18/21/24) with a live "should be 24 words — you have N" hint instead of accepting any non-empty text. - Encryption passphrase: detect leading/trailing whitespace and BLOCK with a warning (not a silent trim, which would change the passphrase and lock the user out). - Receive payment QR: emit the canonical `drgx:` scheme with a URL-encoded memo via a new shared util::buildPaymentUri (the request-payment dialog now routes through it too, so they can't diverge); the old "dragonx:" + raw memo was unparseable by the wallet's own scanner. Also clamps the receive "Recent Received" list to the 4 most recent (companion to the recent-lists commit). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app.cpp | 48 ++++++++++++++++--- src/app.h | 3 +- src/app_network.cpp | 54 +++++++++++++--------- src/app_wizard.cpp | 16 ++++++- src/ui/windows/export_all_keys_dialog.cpp | 29 ++++++++---- src/ui/windows/receive_tab.cpp | 11 +++-- src/ui/windows/request_payment_dialog.cpp | 56 ++--------------------- src/util/bootstrap.cpp | 10 ++-- src/util/payment_uri.cpp | 46 +++++++++++++++++++ src/util/payment_uri.h | 15 ++++++ 10 files changed, 188 insertions(+), 100 deletions(-) diff --git a/src/app.cpp b/src/app.cpp index c809fb1..beb9697 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -2509,7 +2509,28 @@ void App::renderLiteFirstRunPrompt() while (!seedTrim.empty() && std::isspace((unsigned char)seedTrim.front())) seedTrim.erase(seedTrim.begin()); while (!seedTrim.empty() && std::isspace((unsigned char)seedTrim.back())) seedTrim.pop_back(); - ImGui::BeginDisabled(seedTrim.empty()); + // Require a valid BIP39 word count before enabling Restore — otherwise a truncated or + // garbage phrase (previously any non-empty text passed) is submitted and fails opaquely. + int seedWords = 0; + { bool inWord = false; + for (char c : seedTrim) { + bool sp = (c == ' ' || c == '\t' || c == '\n' || c == '\r'); + if (!sp && !inWord) { seedWords++; inWord = true; } + else if (sp) inWord = false; + } } + bool seedLenOk = (seedWords == 12 || seedWords == 15 || seedWords == 18 || + seedWords == 21 || seedWords == 24); + if (!seedTrim.empty() && !seedLenOk) { + ImGui::PushStyleColor(ImGuiCol_Text, ImGui::ColorConvertU32ToFloat4(ui::material::Warning())); + ImGui::PushTextWrapPos(ImGui::GetCursorPosX() + 380.0f); + ImGui::TextUnformatted(("Recovery phrase should be 24 words — you have " + + std::to_string(seedWords) + ".").c_str()); + ImGui::PopTextWrapPos(); + ImGui::PopStyleColor(); + ImGui::Spacing(); + } + + ImGui::BeginDisabled(!seedLenOk); if (ui::material::TactileButton(TR("lite_restore_btn"), ImVec2(btnW, 0))) { wallet::LiteWalletRestoreRequest req; req.seedPhrase = seedTrim; @@ -2782,8 +2803,10 @@ void App::renderBackupDialog() ImGui::Text("Backup File Path:"); static char backup_path[512] = "dragonx-backup.txt"; + static bool s_backup_confirm_overwrite = false; ImGui::SetNextItemWidth(-1); - ImGui::InputText("##backuppath", backup_path, sizeof(backup_path)); + if (ImGui::InputText("##backuppath", backup_path, sizeof(backup_path))) + s_backup_confirm_overwrite = false; // path edited — re-check overwrite on next Save ImGui::Spacing(); @@ -2800,17 +2823,30 @@ void App::renderBackupDialog() if (ui::material::StyledButton("Save Backup", ImVec2(btnW, 0), ui::material::resolveButtonFont(btnFont))) { std::string path(backup_path); + // Trim surrounding whitespace (a pasted path can carry a trailing newline). + while (!path.empty() && (path.front()==' '||path.front()=='\t'||path.front()=='\n'||path.front()=='\r')) path.erase(path.begin()); + while (!path.empty() && (path.back()==' '||path.back()=='\t'||path.back()=='\n'||path.back()=='\r')) path.pop_back(); if (!path.empty()) { - backupWallet(path, [this](bool success, const std::string& msg) { - backup_success_ = success; - backup_status_ = msg; - }); + std::error_code ec; + if (!s_backup_confirm_overwrite && std::filesystem::exists(path, ec)) { + // Don't clobber an existing file (possibly a good earlier backup) without confirmation. + s_backup_confirm_overwrite = true; + backup_success_ = false; + backup_status_ = "A file already exists there — click Save Backup again to overwrite it."; + } else { + s_backup_confirm_overwrite = false; + backupWallet(path, [this](bool success, const std::string& msg) { + backup_success_ = success; + backup_status_ = msg; + }); + } } } ImGui::SameLine(); if (ui::material::StyledButton("Close", ImVec2(btnW, 0), ui::material::resolveButtonFont(btnFont))) { show_backup_ = false; backup_status_.clear(); + s_backup_confirm_overwrite = false; } ui::material::EndOverlayDialog(); diff --git a/src/app.h b/src/app.h index c2a994f..b739239 100644 --- a/src/app.h +++ b/src/app.h @@ -269,7 +269,8 @@ public: // Key export/import void exportPrivateKey(const std::string& address, std::function callback); - void exportAllKeys(std::function callback); + // callback receives (keys, exportedCount, totalAddresses) so callers can detect a keyless/partial export. + void exportAllKeys(std::function callback); void importPrivateKey(const std::string& key, std::function callback); // Wallet backup diff --git a/src/app_network.cpp b/src/app_network.cpp index 95d1844..6e5e0b3 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -2323,21 +2323,22 @@ void App::exportPrivateKey(const std::string& address, std::function callback) +void App::exportAllKeys(std::function callback) { if (!state_.connected || !rpc_) { - if (callback) callback(""); + if (callback) callback("", 0, 0); return; } - + // Collect all keys into a string auto keys_result = std::make_shared(); auto pending = std::make_shared(0); auto total = std::make_shared(0); - + auto exported = std::make_shared(0); // keys actually retrieved (vs. failed/locked) + // First get all addresses auto all_addresses = std::make_shared>(); - + // Add t-addresses for (const auto& addr : state_.t_addresses) { all_addresses->push_back(addr.address); @@ -2346,27 +2347,28 @@ void App::exportAllKeys(std::function callback) for (const auto& addr : state_.z_addresses) { all_addresses->push_back(addr.address); } - + *total = all_addresses->size(); *pending = *total; - + if (*total == 0) { - if (callback) callback("# No addresses to export\n"); + if (callback) callback("# No addresses to export\n", 0, 0); return; } - + *keys_result = "# DragonX Wallet Private Keys Export\n"; *keys_result += "# WARNING: Keep this file secure! Anyone with these keys can spend your coins!\n\n"; - + for (const auto& addr : *all_addresses) { - exportPrivateKey(addr, [keys_result, pending, total, callback, addr](const std::string& key) { + exportPrivateKey(addr, [keys_result, pending, total, exported, callback, addr](const std::string& key) { if (!key.empty()) { *keys_result += "# " + addr + "\n"; *keys_result += key + "\n\n"; + (*exported)++; } (*pending)--; if (*pending == 0 && callback) { - callback(*keys_result); + callback(*keys_result, *exported, *total); } }); } @@ -2413,25 +2415,33 @@ void App::backupWallet(const std::string& destination, std::functionLegacySize + 6.0f * dp; } + // Warn + block if the passphrase has leading/trailing whitespace. Silently trimming it would + // change the passphrase the user believes they set and lock them out on the next unlock. + bool passEdgeSpace = false; + if (size_t pl = strlen(encrypt_pass_buf_)) { + char a = encrypt_pass_buf_[0], b = encrypt_pass_buf_[pl - 1]; + passEdgeSpace = (a == ' ' || a == '\t' || b == ' ' || b == '\t'); + } + if (passEdgeSpace) { + dl->AddText(captionFont, captionFont->LegacySize, ImVec2(cx, cy), + ui::material::Error(), "Passphrase has leading/trailing spaces — remove them"); + cy += captionFont->LegacySize + 6.0f * dp; + } + // Buttons { bool passValid = strlen(encrypt_pass_buf_) >= 8 && - strcmp(encrypt_pass_buf_, encrypt_confirm_buf_) == 0; + strcmp(encrypt_pass_buf_, encrypt_confirm_buf_) == 0 && + !passEdgeSpace; // PIN is optional: if entered, must be valid + confirmed std::string pinStr(wizard_pin_buf_); bool pinEntered = !pinStr.empty(); diff --git a/src/ui/windows/export_all_keys_dialog.cpp b/src/ui/windows/export_all_keys_dialog.cpp index fee1919..d33fd29 100644 --- a/src/ui/windows/export_all_keys_dialog.cpp +++ b/src/ui/windows/export_all_keys_dialog.cpp @@ -177,9 +177,9 @@ void ExportAllKeysDialog::render(App* app) if (result.is_string()) { keys += "# Address: " + addr + "\n"; keys += result.get() + "\n\n"; + exported++; // count only real successes (locked/failed keys don't count) } } catch (...) {} - exported++; } } @@ -193,17 +193,18 @@ void ExportAllKeysDialog::render(App* app) if (result.is_string()) { keys += "# Address: " + addr + "\n"; keys += result.get() + "\n\n"; + exported++; // count only real successes (locked/failed keys don't count) } } catch (...) {} - exported++; } } - // Save to file (still on worker thread) + // Only write a file if we actually exported keys — a keyless file would be a + // misleading "export" (the usual cause of 0 keys is an encrypted+locked wallet). std::string configDir = util::Platform::getConfigDir(); std::string filepath = configDir + "/" + filename; bool writeOk = false; - { + if (exported > 0) { std::ofstream file(filepath); if (file.is_open()) { file << keys; @@ -211,16 +212,26 @@ void ExportAllKeysDialog::render(App* app) writeOk = true; } } - + return [exported, total, filepath, writeOk]() { s_exported_count = exported; s_exporting = false; - if (writeOk) { - s_status = "Exported to: " + filepath; - Notifications::instance().success(TR("export_keys_success"), 5.0f); - } else { + if (exported == 0) { + s_status = "No keys exported (0 of " + std::to_string(total) + + ") — unlock the wallet (if encrypted) and try again."; + Notifications::instance().error("No keys could be exported — is the wallet unlocked?"); + } else if (!writeOk) { s_status = "Failed to write file"; Notifications::instance().error("Failed to save key file"); + } else if (exported < total) { + s_status = "Exported " + std::to_string(exported) + " of " + + std::to_string(total) + " keys to: " + filepath + + " (INCOMPLETE — some addresses had no spending key or the wallet is locked)"; + Notifications::instance().warning("Partial export: " + std::to_string(exported) + + " of " + std::to_string(total) + " keys"); + } else { + s_status = "Exported to: " + filepath; + Notifications::instance().success(TR("export_keys_success"), 5.0f); } }; }); diff --git a/src/ui/windows/receive_tab.cpp b/src/ui/windows/receive_tab.cpp index 1e01250..a1a224a 100644 --- a/src/ui/windows/receive_tab.cpp +++ b/src/ui/windows/receive_tab.cpp @@ -12,6 +12,7 @@ #include "../../app.h" #include "../../config/settings.h" #include "../../util/i18n.h" +#include "../../util/payment_uri.h" #include "../../util/platform.h" #include "../../util/text_format.h" #include "../../config/version.h" @@ -365,6 +366,7 @@ static void RenderRecentReceived(const AddressInfo& /* addr */, if (tx.type != "receive" && tx.type != "mined") continue; recvs.push_back(&tx); } + if (recvs.size() > 4) recvs.resize(4); // show only the 4 most recent (newest-first) float listH = std::max(rowH, ImGui::GetContentRegionAvail().y); @@ -539,11 +541,10 @@ void RenderReceiveTab(App* app) // Generate QR data std::string qr_data = selected.address; if (s_request_amount > 0) { - qr_data = std::string("dragonx:") + selected.address + - "?amount=" + std::to_string(s_request_amount); - if (s_request_memo[0] && isZ) { - qr_data += "&memo=" + std::string(s_request_memo); - } + // Canonical, URL-encoded "drgx:" URI via the shared builder. The old inline code emitted an + // unparseable "dragonx:" scheme and appended the memo raw (spaces/&/= corrupted the QR). + qr_data = util::buildPaymentUri(selected.address, s_request_amount, "", + (s_request_memo[0] && isZ) ? std::string(s_request_memo) : std::string()); } if (qr_data != s_cached_qr_data) { if (s_qr_texture) { diff --git a/src/ui/windows/request_payment_dialog.cpp b/src/ui/windows/request_payment_dialog.cpp index 53d1ee4..557a9fd 100644 --- a/src/ui/windows/request_payment_dialog.cpp +++ b/src/ui/windows/request_payment_dialog.cpp @@ -5,6 +5,7 @@ #include "request_payment_dialog.h" #include "../../app.h" #include "../../util/i18n.h" +#include "../../util/payment_uri.h" #include "../notifications.h" #include "../schema/ui_schema.h" #include "../widgets/qr_code.h" @@ -34,59 +35,8 @@ static bool s_uri_dirty = true; // Helper to build payment URI static std::string buildPaymentUri() { - if (s_address[0] == '\0') return ""; - - std::ostringstream uri; - uri << "drgx:" << s_address; - - bool hasParams = false; - auto addParam = [&](const char* key, const std::string& value) { - if (value.empty()) return; - uri << (hasParams ? "&" : "?") << key << "=" << value; - hasParams = true; - }; - - if (s_amount > 0) { - std::ostringstream amt; - amt << std::fixed << std::setprecision(8) << s_amount; - addParam("amount", amt.str()); - } - - if (s_label[0] != '\0') { - // URL encode label - std::string encoded; - for (char c : std::string(s_label)) { - if (isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') { - encoded += c; - } else if (c == ' ') { - encoded += "%20"; - } else { - char hex[4]; - snprintf(hex, sizeof(hex), "%%%02X", (unsigned char)c); - encoded += hex; - } - } - addParam("label", encoded); - } - - if (s_memo[0] != '\0') { - // URL encode memo - std::string encoded; - for (char c : std::string(s_memo)) { - if (isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') { - encoded += c; - } else if (c == ' ') { - encoded += "%20"; - } else { - char hex[4]; - snprintf(hex, sizeof(hex), "%%%02X", (unsigned char)c); - encoded += hex; - } - } - addParam("memo", encoded); - } - - return uri.str(); + // Route through the shared builder so this dialog and the receive tab can't diverge on scheme/encoding. + return dragonx::util::buildPaymentUri(s_address, s_amount, s_label, s_memo); } void RequestPaymentDialog::show(const std::string& address) diff --git a/src/util/bootstrap.cpp b/src/util/bootstrap.cpp index 2deb3fa..ec02456 100644 --- a/src/util/bootstrap.cpp +++ b/src/util/bootstrap.cpp @@ -724,9 +724,13 @@ bool Bootstrap::verifyChecksums(const std::string& zipPath, const std::string& b 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; + // Fail CLOSED: refuse to install an unverified multi-GB archive (mirrors the xmrig/daemon + // updaters, which refuse when no checksum is published). Trusting an unverified download of + // this size is a real integrity/security risk; a missing checksum is a server-side gap to fix. + DEBUG_LOGF("[Bootstrap] No checksum published — refusing to install an unverified archive\n"); + setProgress(State::Failed, "Verification failed: no checksum was published for the bootstrap. " + "Refusing to install an unverified archive."); + return false; } // Determine progress ranges: if both checksums exist, split 0-50% / 50-100% diff --git a/src/util/payment_uri.cpp b/src/util/payment_uri.cpp index bdcfd81..f7ef8bb 100644 --- a/src/util/payment_uri.cpp +++ b/src/util/payment_uri.cpp @@ -5,12 +5,58 @@ #include "payment_uri.h" #include +#include #include #include +#include namespace dragonx { namespace util { +std::string urlEncode(const std::string& value) +{ + std::string out; + out.reserve(value.size()); + for (unsigned char c : value) { + if (std::isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') { + out += static_cast(c); + } else if (c == ' ') { + out += "%20"; + } else { + char hex[4]; + std::snprintf(hex, sizeof(hex), "%%%02X", c); + out += hex; + } + } + return out; +} + +std::string buildPaymentUri(const std::string& address, double amount, + const std::string& label, const std::string& memo) +{ + if (address.empty()) return ""; + + std::ostringstream uri; + uri << "drgx:" << address; + + bool hasParams = false; + auto addParam = [&](const char* key, const std::string& value) { + if (value.empty()) return; + uri << (hasParams ? "&" : "?") << key << "=" << value; + hasParams = true; + }; + + if (amount > 0.0) { + std::ostringstream amt; + amt << std::fixed << std::setprecision(8) << amount; + addParam("amount", amt.str()); + } + addParam("label", urlEncode(label)); + addParam("memo", urlEncode(memo)); + + return uri.str(); +} + std::string urlDecode(const std::string& encoded) { std::string result; diff --git a/src/util/payment_uri.h b/src/util/payment_uri.h index 8927974..271d1f7 100644 --- a/src/util/payment_uri.h +++ b/src/util/payment_uri.h @@ -40,6 +40,21 @@ PaymentURI parsePaymentURI(const std::string& uri); */ std::string urlDecode(const std::string& encoded); +/** + * @brief URL-encode a string (RFC 3986 unreserved kept; ' ' -> %20; others -> %HH). + */ +std::string urlEncode(const std::string& value); + +/** + * @brief Build a canonical DragonX payment URI: drgx:
[?amount=..&label=..&memo=..], + * with query parameters URL-encoded. amount<=0 and empty label/memo are omitted. + * + * Single source of truth so the receive tab and the request-payment dialog can't diverge on the + * scheme or encoding (the receive tab previously emitted an unparseable "dragonx:" URI with a raw memo). + */ +std::string buildPaymentUri(const std::string& address, double amount = 0.0, + const std::string& label = "", const std::string& memo = ""); + /** * @brief Check if a string is a valid payment URI * @param str String to check