From c7e3b46782e07e24e9c2f0d5264bc1f4481771bc Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 2 Feb 2026 20:03:18 +0700 Subject: [PATCH 1/5] refactor: SecureString already does zeroeing, remove duplicated code --- src/qt/mnemonicverificationdialog.cpp | 32 ++++----------------------- src/qt/mnemonicverificationdialog.h | 2 -- 2 files changed, 4 insertions(+), 30 deletions(-) diff --git a/src/qt/mnemonicverificationdialog.cpp b/src/qt/mnemonicverificationdialog.cpp index bf63222bd48c..0a7cf1489563 100644 --- a/src/qt/mnemonicverificationdialog.cpp +++ b/src/qt/mnemonicverificationdialog.cpp @@ -96,8 +96,6 @@ MnemonicVerificationDialog::MnemonicVerificationDialog(const SecureString& mnemo MnemonicVerificationDialog::~MnemonicVerificationDialog() { - clearWordsSecurely(); - clearMnemonic(); delete ui; } @@ -278,16 +276,10 @@ void MnemonicVerificationDialog::onHideMnemonicClicked() ui->hideMnemonicButton->hide(); ui->showMnemonicButton->show(); m_mnemonic_revealed = false; - // Clear words from non-secure memory immediately when hiding - clearWordsSecurely(); } void MnemonicVerificationDialog::reject() { - // Clear words when going back to step 1 (unless mnemonic is revealed) - if (!m_mnemonic_revealed) { - clearWordsSecurely(); - } // close dialog for step-1; return back to step-1 for step-2 if (ui->stackedWidget->currentIndex() == 0) { QDialog::reject(); @@ -361,12 +353,6 @@ void MnemonicVerificationDialog::accept() QDialog::accept(); } -void MnemonicVerificationDialog::clearMnemonic() -{ - clearWordsSecurely(); - m_mnemonic.assign(m_mnemonic.size(), 0); -} - std::vector MnemonicVerificationDialog::parseWords() { // If words are already parsed, reuse them (for step 2 validation or step 1 display) @@ -381,14 +367,15 @@ std::vector MnemonicVerificationDialog::parseWords() // Convert to SecureString vector for secure storage m_words.clear(); m_words.reserve(wordList.size()); + std::string wordStd; for (const QString& word : wordList) { - std::string wordStd = word.toStdString(); + wordStd = word.toStdString(); SecureString secureWord; secureWord.assign(std::string_view{wordStd}); m_words.push_back(secureWord); - // Clear temporary std::string - wordStd.assign(wordStd.size(), 0); } + // Clear temporary std::string + wordStd.assign(wordStd.size(), 0); // Clear the temporary QString immediately after parsing mnemonicStr.fill(QChar(0)); @@ -399,17 +386,6 @@ std::vector MnemonicVerificationDialog::parseWords() return m_words; } -void MnemonicVerificationDialog::clearWordsSecurely() -{ - // Securely clear each word string by overwriting before clearing - for (SecureString& word : m_words) { - // Overwrite with zeros before clearing - word.assign(word.size(), 0); - word.clear(); - } - m_words.clear(); -} - int MnemonicVerificationDialog::getWordCount() const { // Count words without parsing them into vector diff --git a/src/qt/mnemonicverificationdialog.h b/src/qt/mnemonicverificationdialog.h index c7026f2a71aa..299dd4bb184c 100644 --- a/src/qt/mnemonicverificationdialog.h +++ b/src/qt/mnemonicverificationdialog.h @@ -41,10 +41,8 @@ private Q_SLOTS: void generateRandomPositions(); void updateWordValidation(); bool validateWord(const QString& word, int position); - void clearMnemonic(); void buildMnemonicGrid(bool reveal); std::vector parseWords(); - void clearWordsSecurely(); int getWordCount() const; Ui::MnemonicVerificationDialog *ui; From f5d8b744692e3666f69dddefce2285ee429f875b Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 2 Feb 2026 20:07:18 +0700 Subject: [PATCH 2/5] fix: avoid extra allocation of secured data in temporary std::string for mnemonic --- src/qt/mnemonicverificationdialog.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qt/mnemonicverificationdialog.cpp b/src/qt/mnemonicverificationdialog.cpp index 0a7cf1489563..0975ebee2766 100644 --- a/src/qt/mnemonicverificationdialog.cpp +++ b/src/qt/mnemonicverificationdialog.cpp @@ -302,8 +302,8 @@ bool MnemonicVerificationDialog::validateWord(const QString& word, int position) return false; } // Convert SecureString to QString temporarily for comparison - QString secureWord = QString::fromStdString(std::string(words[position - 1].begin(), words[position - 1].end())); - bool result = word == secureWord.toLower(); + QString secureWord{QString::fromUtf8(words[position - 1].data(), words[position - 1].size()).toLower()}; + const bool result{word == secureWord.toLower()}; // Clear temporary QString immediately secureWord.fill(QChar(0)); secureWord.clear(); @@ -361,7 +361,7 @@ std::vector MnemonicVerificationDialog::parseWords() } // Parse words from secure mnemonic string - QString mnemonicStr = QString::fromStdString(std::string(m_mnemonic.begin(), m_mnemonic.end())); + QString mnemonicStr{QString::fromUtf8(m_mnemonic.data(), m_mnemonic.size())}; QStringList wordList = mnemonicStr.split(QRegularExpression("\\s+"), Qt::SkipEmptyParts); // Convert to SecureString vector for secure storage @@ -391,7 +391,7 @@ int MnemonicVerificationDialog::getWordCount() const // Count words without parsing them into vector // This avoids storing words in non-secure memory unnecessarily if (m_words.empty()) { - QString mnemonicStr = QString::fromStdString(std::string(m_mnemonic.begin(), m_mnemonic.end())); + QString mnemonicStr{QString::fromUtf8(m_mnemonic.data(), m_mnemonic.size())}; QStringList words = mnemonicStr.split(QRegularExpression("\\s+"), Qt::SkipEmptyParts); int count = words.size(); // Clear immediately @@ -456,7 +456,7 @@ void MnemonicVerificationDialog::buildMnemonicGrid(bool reveal) for (int c = 0; c < columns; ++c) { int idx = r * columns + c; if (idx >= n) break; // Convert SecureString to QString temporarily for display - QString wordStr = QString::fromStdString(std::string(words[idx].begin(), words[idx].end())); + QString wordStr{QString::fromUtf8(words[idx].data(), words[idx].size())}; const QString text = QString("%1. %2").arg(idx + 1, 2).arg(wordStr); QLabel* lbl = new QLabel(text); lbl->setTextInteractionFlags(Qt::TextSelectableByMouse); From 1edec05b6c0110288eea37283e0cd95045334af8 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 2 Feb 2026 19:51:47 +0700 Subject: [PATCH 3/5] fix: add mnemonicverificationdialog to non-backported list --- test/util/data/non-backported.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/util/data/non-backported.txt b/test/util/data/non-backported.txt index 8143c1559807..97566cf80518 100644 --- a/test/util/data/non-backported.txt +++ b/test/util/data/non-backported.txt @@ -33,6 +33,7 @@ src/qt/governancelist.* src/qt/guiutil_font.* src/qt/masternodelist.* src/qt/masternodemodel.* +src/qt/mnemonicverificationdialog.* src/qt/proposalmodel.* src/qt/proposalwizard.* src/rpc/coinjoin.cpp From b8b0c1cbe1bb54affebba6215b971182bf0efcf8 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 2 Feb 2026 19:52:02 +0700 Subject: [PATCH 4/5] fmt: fix formatting for mnemonicverificatinodialog --- src/qt/mnemonicverificationdialog.cpp | 4 +--- src/qt/mnemonicverificationdialog.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qt/mnemonicverificationdialog.cpp b/src/qt/mnemonicverificationdialog.cpp index 0975ebee2766..1a6cc6354610 100644 --- a/src/qt/mnemonicverificationdialog.cpp +++ b/src/qt/mnemonicverificationdialog.cpp @@ -78,7 +78,7 @@ MnemonicVerificationDialog::MnemonicVerificationDialog(const SecureString& mnemo // Connections connect(ui->showMnemonicButton, &QPushButton::clicked, this, &MnemonicVerificationDialog::onShowMnemonicClicked); - connect(ui->hideMnemonicButton, &QPushButton::clicked, this, &MnemonicVerificationDialog::onHideMnemonicClicked); + connect(ui->hideMnemonicButton, &QPushButton::clicked, this, &MnemonicVerificationDialog::onHideMnemonicClicked); if (!m_view_only) { connect(ui->writtenDownCheckbox, &QCheckBox::toggled, this, [this](bool checked) { @@ -470,5 +470,3 @@ void MnemonicVerificationDialog::buildMnemonicGrid(bool reveal) m_gridLayout->setRowMinimumHeight(rows, 12); } - - diff --git a/src/qt/mnemonicverificationdialog.h b/src/qt/mnemonicverificationdialog.h index 299dd4bb184c..ad3dec7426f0 100644 --- a/src/qt/mnemonicverificationdialog.h +++ b/src/qt/mnemonicverificationdialog.h @@ -57,4 +57,3 @@ private Q_SLOTS: }; #endif // BITCOIN_QT_MNEMONICVERIFICATIONDIALOG_H - From b847bae1ff23755b0f8114d944f069630f34bb17 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 11 Feb 2026 13:39:33 +0300 Subject: [PATCH 5/5] fix: restore eager clearing of parsed words and improve secure memory handling Restore m_words.clear() calls in onHideMnemonicClicked() and reject() to minimize the exposure window of sensitive mnemonic data in memory. While secure_allocator cleanses on deallocation, keeping data in memory longer than necessary increases the attack surface. Also fix redundant double .toLower() in validateWord() and replace std::string intermediate in parseWords() loop with QByteArray to avoid leaking word data through the default (non-secure) allocator. Co-Authored-By: Claude Opus 4.6 --- src/qt/mnemonicverificationdialog.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/qt/mnemonicverificationdialog.cpp b/src/qt/mnemonicverificationdialog.cpp index 1a6cc6354610..9bdb2b5094b4 100644 --- a/src/qt/mnemonicverificationdialog.cpp +++ b/src/qt/mnemonicverificationdialog.cpp @@ -276,10 +276,15 @@ void MnemonicVerificationDialog::onHideMnemonicClicked() ui->hideMnemonicButton->hide(); ui->showMnemonicButton->show(); m_mnemonic_revealed = false; + // Clear parsed words from memory immediately when hiding + m_words.clear(); } void MnemonicVerificationDialog::reject() { + // Eagerly clear parsed words to minimize exposure time in memory. + // They will be re-parsed on demand via parseWords() if needed. + m_words.clear(); // close dialog for step-1; return back to step-1 for step-2 if (ui->stackedWidget->currentIndex() == 0) { QDialog::reject(); @@ -303,7 +308,7 @@ bool MnemonicVerificationDialog::validateWord(const QString& word, int position) } // Convert SecureString to QString temporarily for comparison QString secureWord{QString::fromUtf8(words[position - 1].data(), words[position - 1].size()).toLower()}; - const bool result{word == secureWord.toLower()}; + const bool result{word == secureWord}; // Clear temporary QString immediately secureWord.fill(QChar(0)); secureWord.clear(); @@ -367,15 +372,13 @@ std::vector MnemonicVerificationDialog::parseWords() // Convert to SecureString vector for secure storage m_words.clear(); m_words.reserve(wordList.size()); - std::string wordStd; for (const QString& word : wordList) { - wordStd = word.toStdString(); + QByteArray utf8 = word.toUtf8(); SecureString secureWord; - secureWord.assign(std::string_view{wordStd}); - m_words.push_back(secureWord); + secureWord.assign(utf8.constData(), utf8.size()); + m_words.push_back(std::move(secureWord)); + utf8.fill(0); } - // Clear temporary std::string - wordStd.assign(wordStd.size(), 0); // Clear the temporary QString immediately after parsing mnemonicStr.fill(QChar(0));