Skip to content

Commit

Permalink
Merge pull request gridcoin-community#2690 from barton2526/SecurerSec…
Browse files Browse the repository at this point in the history 
…ureString

wallet: SecureString to allow null characters
  • Loading branch information
@jamescowens
jamescowens authored Jul 4, 2023
2 parents aa12215 + ca934df commit dad1bd2
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 23 deletions.
41 changes: 30 additions & 11 deletions src/qt/askpassphrasedialog.cpp
View file
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
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

0 comments on commit dad1bd2

Please sign in to comment.