Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

wallet: SecureString to allow null characters #2690

Merged
merged 2 commits into from
Jul 4, 2023
Merged

wallet: SecureString to allow null characters #2690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57af7fa
Pass all characters to SecureString including nulls
barton2526 Jun 5, 2023
ca934df
Detailed error message for passphrases with null chars
barton2526 Jun 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions src/qt/askpassphrasedialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,10 @@ void AskPassphraseDialog::accept()
oldpass.reserve(MAX_PASSPHRASE_SIZE);
newpass1.reserve(MAX_PASSPHRASE_SIZE);
newpass2.reserve(MAX_PASSPHRASE_SIZE);
// TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string)
// Alternately, find a way to make this input mlock()'d to begin with.
oldpass.assign(ui->oldPassphraseEdit->text().toStdString().c_str());
newpass1.assign(ui->newPassphraseEdit->text().toStdString().c_str());
newpass2.assign(ui->repeatNewPassphraseEdit->text().toStdString().c_str());

oldpass.assign(std::string_view{ui->oldPassphraseEdit->text().toStdString()});
newpass1.assign(std::string_view{ui->newPassphraseEdit->text().toStdString()});
newpass2.assign(std::string_view{ui->repeatNewPassphraseEdit->text().toStdString()});

secureClearPassFields();

Expand Down Expand Up @@ -135,10 +134,20 @@ void AskPassphraseDialog::accept()
} break;
case UnlockStaking:
case Unlock:
if(!model->setWalletLocked(false, oldpass))
{
QMessageBox::critical(this, tr("Wallet unlock failed"),
tr("The passphrase entered for the wallet decryption was incorrect."));
if(!model->setWalletLocked(false, oldpass)) {
// Check if the passphrase has a null character
if (oldpass.find('\0') == std::string::npos) {
QMessageBox::critical(this, tr("Wallet unlock failed"),
tr("The passphrase entered for the wallet decryption was incorrect."));
} else {
QMessageBox::critical(this, tr("Wallet unlock failed"),
tr("The passphrase entered for the wallet decryption is incorrect. "
"It contains a null character (ie - a zero byte). "
"If the passphrase was set with a version of this software prior to 5.4.6, "
"please try again with only the characters up to — but not including — "
"the first null character. If this is successful, please set a new "
"passphrase to avoid this issue in the future."));
}
}
else
{
Expand All @@ -157,8 +166,18 @@ void AskPassphraseDialog::accept()
}
else
{
QMessageBox::critical(this, tr("Wallet encryption failed"),
tr("The passphrase entered for the wallet decryption was incorrect."));
// Check if the old passphrase had a null character
if (oldpass.find('\0') == std::string::npos) {
QMessageBox::critical(this, tr("Passphrase change failed"),
tr("The passphrase entered for the wallet decryption was incorrect."));
} else {
QMessageBox::critical(this, tr("Passphrase change failed"),
tr("The old passphrase entered for the wallet decryption is incorrect. "
"It contains a null character (ie - a zero byte). "
"If the passphrase was set with a version of this software prior to 5.4.6, "
"please try again with only the characters up to — but not including — "
"the first null character."));
}
}
}
else
Expand Down
37 changes: 25 additions & 12 deletions src/wallet/rpcwallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2186,16 +2186,24 @@ UniValue walletpassphrase(const UniValue& params, bool fHelp)
// Note that the walletpassphrase is stored in params[0] which is not mlock()ed
SecureString strWalletPass;
strWalletPass.reserve(100);
// Get rid of this .c_str() by implementing SecureString::operator=(std::string)
// Alternately, find a way to make params[0] mlock()'d to begin with.
strWalletPass = params[0].get_str().c_str();
strWalletPass = std::string_view{params[0].get_str()};

if (strWalletPass.length() > 0)
{
LOCK2(cs_main, pwalletMain->cs_wallet);

if (!pwalletMain->Unlock(strWalletPass))
throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect.");
// Check if the passphrase has a null character
if (strWalletPass.find('\0') == std::string::npos) {
throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect.");
} else {
throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered is incorrect. "
"It contains a null character (ie - a zero byte). "
"If the passphrase was set with a version of this software prior to 5.4.6, "
"please try again with only the characters up to — but not including — "
"the first null character. If this is successful, please set a new "
"passphrase to avoid this issue in the future.");
}
}
else
throw runtime_error(
Expand Down Expand Up @@ -2229,15 +2237,13 @@ UniValue walletpassphrasechange(const UniValue& params, bool fHelp)
if (!pwalletMain->IsCrypted())
throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrasechange was called.");

// Get rid of these .c_str() calls by implementing SecureString::operator=(std::string)
// Alternately, find a way to make params[0] mlock()'d to begin with.
SecureString strOldWalletPass;
strOldWalletPass.reserve(100);
strOldWalletPass = params[0].get_str().c_str();
strOldWalletPass = std::string_view{params[0].get_str()};

SecureString strNewWalletPass;
strNewWalletPass.reserve(100);
strNewWalletPass = params[1].get_str().c_str();
strNewWalletPass = std::string_view{params[1].get_str()};

if (strOldWalletPass.length() < 1 || strNewWalletPass.length() < 1)
throw runtime_error(
Expand All @@ -2247,7 +2253,16 @@ UniValue walletpassphrasechange(const UniValue& params, bool fHelp)
LOCK2(cs_main, pwalletMain->cs_wallet);

if (!pwalletMain->ChangeWalletPassphrase(strOldWalletPass, strNewWalletPass))
throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect.");
// Check if the old passphrase had a null character
if (strOldWalletPass.find('\0') == std::string::npos) {
throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect.");
} else {
throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The old wallet passphrase entered is incorrect. "
"It contains a null character (ie - a zero byte). "
"If the old passphrase was set with a version of this software prior to 5.4.6, "
"please try again with only the characters up to — but not including — "
"the first null character.");
}

return NullUniValue;
}
Expand Down Expand Up @@ -2373,11 +2388,9 @@ UniValue encryptwallet(const UniValue& params, bool fHelp)
if (pwalletMain->IsCrypted())
throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an encrypted wallet, but encryptwallet was called.");

// Get rid of this .c_str() by implementing SecureString::operator=(std::string)
// Alternately, find a way to make params[0] mlock()'d to begin with.
SecureString strWalletPass;
strWalletPass.reserve(100);
strWalletPass = params[0].get_str().c_str();
strWalletPass = std::string_view{params[0].get_str()};

if (strWalletPass.length() < 1)
throw runtime_error(
Expand Down