From 8a30578872ef2cedf9ffcd0dd7919930efa8295e Mon Sep 17 00:00:00 2001 From: DanS Date: Mon, 13 Jul 2026 23:47:11 -0500 Subject: [PATCH] feat(settings): migrate Export Private Key modal + fix outside-click secret leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 (first security modal), presentation-only + the review's one finding. Migration: struct-form BlurFloat overlay (was legacy positional); full i18n (reusing export_private_key / key_export_private_warning / key_export_private_key / select_address / key_export_fetching / export / copy / close, plus one new key_export_failed); DialogWarningHeader; centered DialogActionFooter (Export + Close) + a Copy TactileButton; async UX (export_in_progress_ spinner, export_error_ line). The exported key stays in the char[256] buffer shown via a read-only InputTextMultiline (deliberately NOT AddressCopyField, which would copy the secret into a lingering std::string). exportPrivateKey is untouched. Security fix (found by the adversarial review): the 0a "outside-click/Esc wipe" via the Begin-returns-false early-return was UNREACHABLE — BeginOverlayDialog clears show_export_key_ mid-frame but still returns true, so an outside-click/Esc dismiss left the exported spending key un-wiped in export_result_. Add the in-body `if (!show_export_key_)` teardown before EndOverlayDialog (the proven import-dialog pattern), which is the real wipe point for every dismiss path. Co-Authored-By: Claude Opus 4.8 (1M context) --- res/lang/de.json | 1 + res/lang/es.json | 1 + res/lang/fr.json | 1 + res/lang/ja.json | 1 + res/lang/ko.json | 1 + res/lang/pt.json | 1 + res/lang/ru.json | 1 + res/lang/zh.json | 1 + src/app.cpp | 124 ++++++++++++++++++++++++++-------------------- src/app.h | 2 + src/util/i18n.cpp | 1 + 11 files changed, 82 insertions(+), 53 deletions(-) diff --git a/res/lang/de.json b/res/lang/de.json index a11b880..59576df 100644 --- a/res/lang/de.json +++ b/res/lang/de.json @@ -470,6 +470,7 @@ "keep": "Behalten", "keep_daemon": "Daemon weiterlaufen lassen", "key_export_click_retrieve": "Klicken Sie, um den Schlüssel aus Ihrer Wallet abzurufen", + "key_export_failed": "Schlüssel konnte nicht exportiert werden — entsperren Sie die Wallet (falls verschlüsselt) und versuchen Sie es erneut.", "key_export_fetching": "Schlüssel wird aus Wallet abgerufen...", "key_export_private_key": "Privater Schlüssel:", "key_export_private_warning": "Halten Sie diesen Schlüssel GEHEIM! Jeder mit diesem Schlüssel kann Ihre Gelder ausgeben. Teilen Sie ihn niemals online oder mit nicht vertrauenswürdigen Parteien.", diff --git a/res/lang/es.json b/res/lang/es.json index d81b4a5..35b01d5 100644 --- a/res/lang/es.json +++ b/res/lang/es.json @@ -470,6 +470,7 @@ "keep": "Mantener", "keep_daemon": "Mantener daemon activo", "key_export_click_retrieve": "Haga clic para recuperar la clave de su billetera", + "key_export_failed": "No se pudo exportar la clave: desbloquea el monedero (si está cifrado) e inténtalo de nuevo.", "key_export_fetching": "Obteniendo clave de la cartera...", "key_export_private_key": "Clave Privada:", "key_export_private_warning": "¡Mantén esta clave en SECRETO! Cualquiera con esta clave puede gastar tus fondos. Nunca la compartas en línea ni con personas no confiables.", diff --git a/res/lang/fr.json b/res/lang/fr.json index a97a864..9b38da2 100644 --- a/res/lang/fr.json +++ b/res/lang/fr.json @@ -470,6 +470,7 @@ "keep": "Conserver", "keep_daemon": "Garder le daemon en marche", "key_export_click_retrieve": "Cliquez pour récupérer la clé de votre portefeuille", + "key_export_failed": "Impossible d'exporter la clé — déverrouillez le portefeuille (s'il est chiffré) et réessayez.", "key_export_fetching": "Récupération de la clé depuis le portefeuille...", "key_export_private_key": "Clé privée :", "key_export_private_warning": "Gardez cette clé SECRÈTE ! Toute personne possédant cette clé peut dépenser vos fonds. Ne la partagez jamais en ligne ou avec des tiers non fiables.", diff --git a/res/lang/ja.json b/res/lang/ja.json index 8dbe0e5..2c2e592 100644 --- a/res/lang/ja.json +++ b/res/lang/ja.json @@ -470,6 +470,7 @@ "keep": "保持", "keep_daemon": "デーモンを実行し続ける", "key_export_click_retrieve": "クリックしてウォレットからキーを取得", + "key_export_failed": "鍵をエクスポートできませんでした。ウォレットのロックを解除して(暗号化されている場合)再試行してください。", "key_export_fetching": "ウォレットから鍵を取得中...", "key_export_private_key": "秘密鍵:", "key_export_private_warning": "この鍵は秘密にしてください!この鍵を持つ人は誰でもあなたの資金を使えます。オンラインや信頼できない相手と共有しないでください。", diff --git a/res/lang/ko.json b/res/lang/ko.json index 3f8f958..a245802 100644 --- a/res/lang/ko.json +++ b/res/lang/ko.json @@ -470,6 +470,7 @@ "keep": "유지", "keep_daemon": "데몬 계속 실행", "key_export_click_retrieve": "지갑에서 키를 가져오려면 클릭", + "key_export_failed": "키를 내보내지 못했습니다. 지갑 잠금을 해제하고(암호화된 경우) 다시 시도하세요.", "key_export_fetching": "지갑에서 키를 가져오는 중...", "key_export_private_key": "개인 키:", "key_export_private_warning": "이 키를 비밀로 유지하세요! 이 키를 가진 사람은 누구나 자금을 사용할 수 있습니다. 온라인이나 신뢰할 수 없는 사람과 공유하지 마세요.", diff --git a/res/lang/pt.json b/res/lang/pt.json index d7aac26..25e31c5 100644 --- a/res/lang/pt.json +++ b/res/lang/pt.json @@ -470,6 +470,7 @@ "keep": "Manter", "keep_daemon": "Manter daemon em execução", "key_export_click_retrieve": "Clique para recuperar a chave da sua carteira", + "key_export_failed": "Não foi possível exportar a chave — desbloqueie a carteira (se estiver criptografada) e tente novamente.", "key_export_fetching": "Buscando chave da carteira...", "key_export_private_key": "Chave Privada:", "key_export_private_warning": "Mantenha esta chave em SEGREDO! Qualquer pessoa com esta chave pode gastar seus fundos. Nunca a compartilhe online ou com terceiros não confiáveis.", diff --git a/res/lang/ru.json b/res/lang/ru.json index d212ccc..ae63892 100644 --- a/res/lang/ru.json +++ b/res/lang/ru.json @@ -470,6 +470,7 @@ "keep": "Сохранить", "keep_daemon": "Оставить daemon работающим", "key_export_click_retrieve": "Нажмите, чтобы получить ключ из вашего кошелька", + "key_export_failed": "Не удалось экспортировать ключ — разблокируйте кошелёк (если зашифрован) и повторите попытку.", "key_export_fetching": "Получение ключа из кошелька...", "key_export_private_key": "Приватный ключ:", "key_export_private_warning": "Держите этот ключ в ТАЙНЕ! Любой, кто владеет этим ключом, может потратить ваши средства. Никогда не делитесь им в интернете или с ненадёжными лицами.", diff --git a/res/lang/zh.json b/res/lang/zh.json index 6ba2683..fb1c7da 100644 --- a/res/lang/zh.json +++ b/res/lang/zh.json @@ -470,6 +470,7 @@ "keep": "保留", "keep_daemon": "保持守护进程运行", "key_export_click_retrieve": "点击从钱包中获取密钥", + "key_export_failed": "无法导出密钥——请解锁钱包(如已加密)后重试。", "key_export_fetching": "正在从钱包获取密钥...", "key_export_private_key": "私钥:", "key_export_private_warning": "请保密此密钥!任何拥有此密钥的人都可以花费您的资金。切勿在网上或与不可信的人分享。", diff --git a/src/app.cpp b/src/app.cpp index bc42625..6eaf98d 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -3192,81 +3192,99 @@ void App::renderImportKeyDialog() void App::renderExportKeyDialog() { - auto dlg = ui::schema::UI().drawElement("inline-dialogs", "export-key"); - auto dlgF = [&](const char* key, float fb) -> float { - auto it = dlg.extraFloats.find(key); - return it != dlg.extraFloats.end() ? it->second : fb; - }; - int btnFont = (int)dlgF("button-font", 1); - - if (!ui::material::BeginOverlayDialog("Export Private Key", &show_export_key_, dlgF("width", 600.0f), 0.94f)) { - // Closed via outside-click / Esc (the overload flips show_export_key_ then returns false here, - // so this is the only wipe point for that path) — scrub the exported secret. + namespace m = ui::material; + float dp = ui::Layout::dpiScale(); + + m::OverlayDialogSpec ov; + ov.title = TR("export_private_key"); + ov.p_open = &show_export_key_; + ov.style = m::OverlayStyle::BlurFloat; + ov.cardWidth = 600.0f; + ov.idSuffix = "exportkey"; + if (!m::BeginOverlayDialog(ov)) { + // Defensive scrub for a frame where the overlay renders nothing (Begin returns false). The + // actual dismiss-path wipe (Close / outside-click / Esc) is the in-body block near EndOverlayDialog. sodium_memzero(export_result_, sizeof(export_result_)); export_address_.clear(); + export_in_progress_ = false; + export_error_ = false; return; } - - ImGui::TextColored(ImVec4(0.9f, 0.4f, 0.4f, 1.0f), - "WARNING: Anyone with this key can spend your coins!"); + + m::DialogWarningHeader(TR("key_export_private_warning")); ImGui::Spacing(); - - // Address selector - ImGui::Text("Select Address:"); + + // Address selector (a legitimate need here, unlike the import dialog). std::vector all_addrs; for (const auto& a : state_.t_addresses) all_addrs.push_back(a.address); for (const auto& a : state_.z_addresses) all_addrs.push_back(a.address); - - int addrFrontLen = (int)dlgF("addr-front-len", 20); - int addrBackLen = (int)dlgF("addr-back-len", 8); - if (ImGui::BeginCombo("##exportaddr", export_address_.empty() ? "Select address..." : export_address_.c_str())) { + ImGui::SetNextItemWidth(-1); + const std::string comboLabel = export_address_.empty() ? std::string(TR("select_address")) + : util::truncateMiddle(export_address_, 24, 10); + if (ImGui::BeginCombo("##exportaddr", comboLabel.c_str())) { for (const auto& addr : all_addrs) { - bool selected = (export_address_ == addr); - std::string display = util::truncateMiddle(addr, addrFrontLen, addrBackLen); - if (ImGui::Selectable(display.c_str(), selected)) { + const bool selected = (export_address_ == addr); + if (ImGui::Selectable(util::truncateMiddle(addr, 28, 12).c_str(), selected)) { export_address_ = addr; - sodium_memzero(export_result_, sizeof(export_result_)); + sodium_memzero(export_result_, sizeof(export_result_)); // new address → old key stale + export_error_ = false; } } ImGui::EndCombo(); } - - ImGui::SameLine(); - if (ui::material::StyledButton("Export", ImVec2(0, 0), ui::material::resolveButtonFont(btnFont))) { - if (!export_address_.empty()) { - exportPrivateKey(export_address_, [this](const std::string& key) { - // Don't populate the secret buffer if the dialog was closed before the async - // export returned (it would otherwise linger un-wiped while the dialog is shut). - if (show_export_key_) - snprintf(export_result_, sizeof(export_result_), "%s", key.c_str()); - }); - } - } - ImGui::Spacing(); - - if (export_result_[0]) { - ImGui::Text("Private Key:"); + // Progress / result / error. The secret key is shown in a read-only char buffer (NOT + // widgets::AddressCopyField, which would copy it into a lingering std::string) + a Copy button. + if (export_in_progress_) { + ImGui::Spacing(); + ImGui::TextColored(ImGui::ColorConvertU32ToFloat4(m::Primary()), "%s%s", + TR("key_export_fetching"), m::LoadingDots()); + } else if (export_result_[0]) { + ImGui::Spacing(); + ImGui::TextColored(ImGui::ColorConvertU32ToFloat4(m::OnSurfaceMedium()), "%s", TR("key_export_private_key")); ImGui::InputTextMultiline("##exportresult", export_result_, sizeof(export_result_), - ImVec2(-1, dlgF("key-display-height", 60.0f)), ImGuiInputTextFlags_ReadOnly); - - if (ui::material::StyledButton("Copy to Clipboard", ImVec2(0, 0), ui::material::resolveButtonFont(btnFont))) { + ImVec2(-1, 60.0f * dp), ImGuiInputTextFlags_ReadOnly); + if (m::TactileButton(TR("copy"))) ImGui::SetClipboardText(export_result_); - } + } else if (export_error_) { + ImGui::Spacing(); + ImGui::PushTextWrapPos(0.0f); + ImGui::TextColored(ImVec4(0.8f, 0.3f, 0.3f, 1.0f), "%s", TR("key_export_failed")); + ImGui::PopTextWrapPos(); } - + ImGui::Spacing(); - ImGui::Separator(); - - int closeBtnFont = (int)dlgF("close-button-font", -1); - if (closeBtnFont < 0) closeBtnFont = btnFont; - if (ui::material::StyledButton("Close", ImVec2(dlgF("close-button-width", 120.0f), 0), ui::material::resolveButtonFont(closeBtnFont))) { - show_export_key_ = false; + + bool doExport = false, doClose = false; + const bool canExport = !export_address_.empty() && !export_in_progress_; + m::DialogActionFooter(TR("export"), canExport, TR("close"), doExport, doClose); + if (doClose) show_export_key_ = false; + if (doExport) { + export_in_progress_ = true; + export_error_ = false; + sodium_memzero(export_result_, sizeof(export_result_)); + exportPrivateKey(export_address_, [this](const std::string& key) { + // Don't populate the secret buffer if the dialog was closed before the async export + // returned (it would otherwise linger un-wiped while the dialog is shut). + export_in_progress_ = false; + if (!show_export_key_) return; + if (key.empty()) export_error_ = true; + else snprintf(export_result_, sizeof(export_result_), "%s", key.c_str()); + }); + } + + // Scrub the secret whenever the dialog closes THIS frame — Close button OR outside-click/Esc, which + // BeginOverlayDialog clears mid-frame while still returning true (so the early-return above is not + // reached on that path). This in-body teardown mirrors the import dialog and is the actual wipe + // point for the dismiss paths; the early-return only covers a Begin-returns-false frame. + if (!show_export_key_) { sodium_memzero(export_result_, sizeof(export_result_)); export_address_.clear(); + export_in_progress_ = false; + export_error_ = false; } - - ui::material::EndOverlayDialog(); + + m::EndOverlayDialog(); } void App::renderBackupDialog() diff --git a/src/app.h b/src/app.h index 4c99156..4bac1c1 100644 --- a/src/app.h +++ b/src/app.h @@ -809,6 +809,8 @@ private: // Export/Import state char export_result_[256] = {0}; // SECRET exported key — fixed buffer so it can be sodium_memzero'd + bool export_in_progress_ = false; // async key fetch running (spinner + disable Export) + bool export_error_ = false; // last export returned no key (locked wallet / failure) char import_key_input_[512] = {0}; std::string export_address_; std::string import_status_; diff --git a/src/util/i18n.cpp b/src/util/i18n.cpp index 89dde18..93f28a3 100644 --- a/src/util/i18n.cpp +++ b/src/util/i18n.cpp @@ -1393,6 +1393,7 @@ void I18n::loadBuiltinEnglish() // --- Key Export Dialog --- strings_["key_export_fetching"] = "Fetching key from wallet..."; + strings_["key_export_failed"] = "Couldn't export the key \xE2\x80\x94 unlock the wallet (if encrypted) and try again."; strings_["key_export_private_key"] = "Private Key:"; strings_["key_export_private_warning"] = "Keep this key SECRET! Anyone with this key can spend your funds. Never share it online or with untrusted parties."; strings_["key_export_reveal"] = "Reveal Key";