From 658c0f355bc8f9856b1465fbb86de2a6c236f336 Mon Sep 17 00:00:00 2001 From: DanS Date: Sun, 5 Jul 2026 15:08:59 -0500 Subject: [PATCH] fix(ui): validate and clamp dialog input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tighten input handling across several dialogs so bad/edge input can't produce a doomed request or a confusing display: - app_security: passphrase strength meter now factors character-class diversity — an all-one-class string downgrades one tier so "aaaaaaaa" no longer scores as high as a mixed one. - block_info: clamp the height field to the chain tip and hide/deny "Next" at the tip (was only yielding a raw RPC error). - address_label: trim surrounding whitespace before saving; a whitespace-only label clears it (mirrors clearing the icon). - send_tab: re-clamp the amount when a Max send has its fee bumped in the confirm popup (kept the total within budget) and NUL-terminate the strncpy'd address/memo when a payment URI overwrites the form. - settings_page: reject an empty/whitespace-only lite wallet path before dispatching an unusable open/restore request. - peers_tab: guard ExtractIP against an empty address. - transaction_details: disable "View in explorer" when the configured explorer URL is empty/whitespace so we never open a garbage link. - app: seed-backup "Skip" now requires a second, deliberate click with a fund-loss warning before it dismisses. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app.cpp | 43 ++++++++++++++++--- src/app_security.cpp | 24 +++++++++-- src/ui/pages/settings_page.cpp | 13 ++++++ src/ui/windows/address_label_dialog.h | 14 +++++- src/ui/windows/block_info_dialog.cpp | 13 +++++- src/ui/windows/peers_tab.cpp | 1 + src/ui/windows/send_tab.cpp | 17 ++++++++ src/ui/windows/transaction_details_dialog.cpp | 7 ++- 8 files changed, 118 insertions(+), 14 deletions(-) diff --git a/src/app.cpp b/src/app.cpp index b8d1a86..726fb02 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -2278,6 +2278,7 @@ void App::renderLiteFirstRunPrompt() static std::vector> chips; // shuffled (word, consumed) static int progress = 0; // # words confirmed in order static double wrongFlashUntil = 0.0; // brief "not the next word" hint + static bool skipConfirm = false; // "Skip" backup requires a second confirming click static bool creating = false; // async create (with failover) in flight // Restore-from-seed step (step 3). The seed buffer is SECRET — wiped in finish() and on Back. static char restoreSeed[512] = {}; @@ -2305,6 +2306,7 @@ void App::renderLiteFirstRunPrompt() chips.clear(); progress = 0; step = 0; + skipConfirm = false; creating = false; lite_firstrun_dismissed_ = true; ImGui::CloseCurrentPopup(); @@ -2415,14 +2417,28 @@ void App::renderLiteFirstRunPrompt() std::mt19937 rng{std::random_device{}()}; std::shuffle(chips.begin(), chips.end(), rng); progress = 0; + skipConfirm = false; step = 2; } ImGui::SameLine(); if (ui::material::TactileButton("Copy", ImVec2(80, 0))) copySecretToClipboard(seed); ImGui::SameLine(); - if (ui::material::TactileButton("Skip", ImVec2(80, 0))) { - ui::Notifications::instance().success(TR("lite_welcome_created"), 6.0f); - finish(); + if (ui::material::TactileButton(skipConfirm ? "Skip anyway" : "Skip", ImVec2(120, 0))) { + if (!skipConfirm) { + skipConfirm = true; // require a second, deliberate click + } else { + ui::Notifications::instance().success(TR("lite_welcome_created"), 6.0f); + finish(); + } + } + if (skipConfirm) { + ImGui::Spacing(); + ImGui::PushTextWrapPos(ImGui::GetCursorPosX() + 380.0f); + ImGui::PushStyleColor(ImGuiCol_Text, ui::material::Error()); + ImGui::TextUnformatted("You have not backed up your seed — funds could be lost. " + "Skip anyway?"); + ImGui::PopStyleColor(); + ImGui::PopTextWrapPos(); } } else if (step == 2) { // ── Verify: tap the words in order ────────────────────────────────────── @@ -2471,11 +2487,24 @@ void App::renderLiteFirstRunPrompt() } if (!verified) ImGui::EndDisabled(); ImGui::SameLine(); - if (ui::material::TactileButton("Back", ImVec2(80, 0))) step = 1; + if (ui::material::TactileButton("Back", ImVec2(80, 0))) { skipConfirm = false; step = 1; } ImGui::SameLine(); - if (ui::material::TactileButton("Skip", ImVec2(80, 0))) { - ui::Notifications::instance().success(TR("lite_welcome_created"), 6.0f); - finish(); + if (ui::material::TactileButton(skipConfirm ? "Skip anyway" : "Skip", ImVec2(120, 0))) { + if (!skipConfirm) { + skipConfirm = true; // require a second, deliberate click + } else { + ui::Notifications::instance().success(TR("lite_welcome_created"), 6.0f); + finish(); + } + } + if (skipConfirm) { + ImGui::Spacing(); + ImGui::PushTextWrapPos(ImGui::GetCursorPosX() + 380.0f); + ImGui::PushStyleColor(ImGuiCol_Text, ui::material::Error()); + ImGui::TextUnformatted("You have not backed up your seed — funds could be lost. " + "Skip anyway?"); + ImGui::PopStyleColor(); + ImGui::PopTextWrapPos(); } } else if (step == 3) { // ── Restore from an existing seed phrase ──────────────────────────────── diff --git a/src/app_security.cpp b/src/app_security.cpp index 60b3898..4c109dd 100644 --- a/src/app_security.cpp +++ b/src/app_security.cpp @@ -1146,12 +1146,30 @@ void App::renderEncryptWalletDialog() { // Strength meter bar { size_t len = strlen(encrypt_pass_buf_); + // Character-class diversity: an all-digit or single-class + // string shouldn't score as high as a mixed one. + bool hasDigit = false, hasLower = false, hasUpper = false, hasSymbol = false; + for (const char* c = encrypt_pass_buf_; *c; ++c) { + unsigned char uc = static_cast(*c); + if (uc >= '0' && uc <= '9') hasDigit = true; + else if (uc >= 'a' && uc <= 'z') hasLower = true; + else if (uc >= 'A' && uc <= 'Z') hasUpper = true; + else hasSymbol = true; + } + int classes = (int)hasDigit + (int)hasLower + (int)hasUpper + (int)hasSymbol; + const char* strengthLabel = "Weak"; ImVec4 strengthCol(0.9f, 0.2f, 0.2f, 1.0f); float strengthPct = 0.25f; - if (len >= 16) { strengthLabel = "Strong"; strengthCol = ImVec4(0.3f,0.9f,0.5f,1); strengthPct = 1.0f; } - else if (len >= 12) { strengthLabel = "Good"; strengthCol = ImVec4(0.3f,0.9f,0.5f,1); strengthPct = 0.75f; } - else if (len >= 8) { strengthLabel = "Fair"; strengthCol = ImVec4(1,0.7f,0.3f,1); strengthPct = 0.5f; } + int tier = 0; // 0=Weak, 1=Fair, 2=Good, 3=Strong + if (len >= 16) tier = 3; + else if (len >= 12) tier = 2; + else if (len >= 8) tier = 1; + // Downgrade one tier when only a single character class is used. + if (classes <= 1 && tier > 0) tier -= 1; + if (tier == 3) { strengthLabel = "Strong"; strengthCol = ImVec4(0.3f,0.9f,0.5f,1); strengthPct = 1.0f; } + else if (tier == 2) { strengthLabel = "Good"; strengthCol = ImVec4(0.3f,0.9f,0.5f,1); strengthPct = 0.75f; } + else if (tier == 1) { strengthLabel = "Fair"; strengthCol = ImVec4(1,0.7f,0.3f,1); strengthPct = 0.5f; } float barW = ImGui::GetContentRegionAvail().x; float barH = 4.0f; diff --git a/src/ui/pages/settings_page.cpp b/src/ui/pages/settings_page.cpp index c17ff8c..58f28ff 100644 --- a/src/ui/pages/settings_page.cpp +++ b/src/ui/pages/settings_page.cpp @@ -287,6 +287,19 @@ static void evaluateLiteLifecycleRequestFromPageState(App* app) { } } liteSecretScrubber{input}; + // Open/Restore both target a wallet path; reject an empty/whitespace-only one early so we + // never dispatch an unusable request (the scrubber above still wipes secrets on this return). + if (input.request.operation == wallet::LiteWalletLifecycleOperation::OpenExisting || + input.request.operation == wallet::LiteWalletLifecycleOperation::RestoreFromSeed) { + std::string trimmedPath(s_settingsState.lite_wallet_path); + const auto first = trimmedPath.find_first_not_of(" \t\r\n"); + if (first == std::string::npos) { + s_settingsState.lite_lifecycle_status = "Enter a wallet path"; + s_settingsState.lite_lifecycle_summary.clear(); + return; + } + } + // Restore needs a complete 24-word seed; reject early (the scrubber above still wipes the // entered secret on this return path). if (input.request.operation == wallet::LiteWalletLifecycleOperation::RestoreFromSeed) { diff --git a/src/ui/windows/address_label_dialog.h b/src/ui/windows/address_label_dialog.h index dcb92a1..8f5e429 100644 --- a/src/ui/windows/address_label_dialog.h +++ b/src/ui/windows/address_label_dialog.h @@ -221,7 +221,19 @@ public: ImGui::SameLine(0, Layout::spacingMd()); if (TactileButton(saveLabel, ImVec2(saveW, 0), buttonFont)) { // Apply changes - s_app->setAddressLabel(s_address, s_label); + // Trim surrounding whitespace; a whitespace-only label clears it + // (mirroring how an empty icon selection clears the icon). + std::string trimmedLabel(s_label); + { + std::size_t b = trimmedLabel.find_first_not_of(" \t\r\n"); + if (b == std::string::npos) { + trimmedLabel.clear(); + } else { + std::size_t e = trimmedLabel.find_last_not_of(" \t\r\n"); + trimmedLabel = trimmedLabel.substr(b, e - b + 1); + } + } + s_app->setAddressLabel(s_address, trimmedLabel); if (s_selectedIcon >= 0) s_app->setAddressIcon(s_address, material::project_icons::walletIconName(s_selectedIcon)); else diff --git a/src/ui/windows/block_info_dialog.cpp b/src/ui/windows/block_info_dialog.cpp index c2fc9b1..45b2de8 100644 --- a/src/ui/windows/block_info_dialog.cpp +++ b/src/ui/windows/block_info_dialog.cpp @@ -115,6 +115,11 @@ void BlockInfoDialog::render(App* app) ImGui::SetNextItemWidth(heightInput.width); ImGui::InputInt("##Height", &s_height); if (s_height < 1) s_height = 1; + // Clamp to the chain tip so navigation/typing can't request a height + // above the tip (which would only yield a raw RPC error). + if (state.sync.blocks > 0 && s_height > state.sync.blocks) { + s_height = state.sync.blocks; + } ImGui::SameLine(); @@ -287,7 +292,8 @@ void BlockInfoDialog::render(App* app) if (ImGui::IsItemHovered()) { material::Tooltip("%s", TR("block_click_next")); } - if (ImGui::IsItemClicked()) { + if (ImGui::IsItemClicked() && + (state.sync.blocks <= 0 || s_height < state.sync.blocks)) { s_height++; s_has_data = false; } @@ -308,7 +314,10 @@ void BlockInfoDialog::render(App* app) ImGui::SameLine(); } - if (!s_next_hash.empty()) { + // Only offer "Next" when below the chain tip (the tip block has no + // nextblockhash, so this stays hidden there). + if (!s_next_hash.empty() && + (state.sync.blocks <= 0 || s_height < state.sync.blocks)) { if (material::StyledButton(TR("block_nav_next"), ImVec2(0,0), S.resolveFont(closeBtn.font))) { s_height++; s_has_data = false; diff --git a/src/ui/windows/peers_tab.cpp b/src/ui/windows/peers_tab.cpp index d8ed638..6d0c5c3 100644 --- a/src/ui/windows/peers_tab.cpp +++ b/src/ui/windows/peers_tab.cpp @@ -37,6 +37,7 @@ static int s_selected_banned_idx = -1; // Helper: Extract IP without port static std::string ExtractIP(const std::string& addr) { + if (addr.empty()) return addr; std::string ip = addr; if (ip[0] == '[') { auto pos = ip.rfind("]:"); diff --git a/src/ui/windows/send_tab.cpp b/src/ui/windows/send_tab.cpp index afba9cf..4f7ad95 100644 --- a/src/ui/windows/send_tab.cpp +++ b/src/ui/windows/send_tab.cpp @@ -725,7 +725,17 @@ void RenderSendConfirmPopup(App* app) { { Type().textColored(TypeStyle::Overline, OnSurfaceMedium(), TR("send_network_fee")); ImGui::Dummy(ImVec2(0, Layout::spacingSm())); + double feeBefore = s_fee; RenderFeeTierSelector("##confirm"); + // If Max was selected, a fee bump here would push total over the available + // balance (s_amount was clamped to available - old fee). Re-clamp so the send + // stays within budget. + if (s_send_max && s_fee != feeBefore) { + double avail = GetAvailableBalance(app); + double maxAmount = avail - s_fee; + if (maxAmount < 0) maxAmount = 0; + s_amount = maxAmount; + } // Recalculate total after potential fee change total = s_amount + s_fee; ImGui::Dummy(ImVec2(0, Layout::spacingMd())); @@ -1129,9 +1139,16 @@ void RenderSendTab(App* app) // Handle pending payment from URI if (app->hasPendingPayment()) { + // A URI arriving mid-compose would silently overwrite the user's in-progress form. + // Capture an undo snapshot first so "Undo" can restore what they were typing. + if (FormHasData()) { + SaveFormSnapshot(); + } strncpy(s_to_address, app->getPendingToAddress().c_str(), sizeof(s_to_address) - 1); + s_to_address[sizeof(s_to_address) - 1] = '\0'; s_amount = app->getPendingAmount(); strncpy(s_memo, app->getPendingMemo().c_str(), sizeof(s_memo) - 1); + s_memo[sizeof(s_memo) - 1] = '\0'; app->clearPendingPayment(); } diff --git a/src/ui/windows/transaction_details_dialog.cpp b/src/ui/windows/transaction_details_dialog.cpp index 1209002..031b337 100644 --- a/src/ui/windows/transaction_details_dialog.cpp +++ b/src/ui/windows/transaction_details_dialog.cpp @@ -179,10 +179,15 @@ void TransactionDetailsDialog::render(App* app) float start_x = (ImGui::GetWindowWidth() - total_width) / 2.0f; ImGui::SetCursorPosX(start_x); + // Guard against an empty/whitespace explorer URL so we never open a garbage link. + std::string explorerBase = app->settings()->getTxExplorerUrl(); + bool explorerValid = explorerBase.find_first_not_of(" \t\r\n") != std::string::npos; + if (!explorerValid) ImGui::BeginDisabled(); if (material::StyledButton(TR("tx_view_explorer"), ImVec2(button_width, 0), S.resolveFont(bottomBtn.font))) { - std::string url = app->settings()->getTxExplorerUrl() + tx.txid; + std::string url = explorerBase + tx.txid; util::Platform::openUrl(url); } + if (!explorerValid) ImGui::EndDisabled(); ImGui::SameLine();