Add MakeSecureString helper, eliminate .c_str() in password paths
- Add MakeSecureString(const std::string&) in allocators.h - Replace .c_str() shims in walletpassphrase, walletpassphrasechange, encryptwallet RPCs and askpassphrasedialog - Update TODO_DOCUMENTATION.md to mark issue as resolved Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -51,14 +51,13 @@ nOrderPos = -1; // TODO: calculate elsewhere
|
||||
**Status:** Deferred - no functional issue.
|
||||
**Fix:** Move calculation to WalletDB when transaction is added.
|
||||
|
||||
### src/rpcwallet.cpp - SecureString Operator
|
||||
```cpp
|
||||
// Lines 1474, 1513, 1569: "TODO: get rid of this .c_str()"
|
||||
```
|
||||
**Issue:** SecureString missing operator=(std::string).
|
||||
**Impact:** Forced to use .c_str() which exposes password temporarily.
|
||||
**Status:** Deferred - would require SecureString class modification.
|
||||
**Fix:** Add `SecureString& operator=(const std::string&)` method.
|
||||
### src/rpcwallet.cpp / src/qt/askpassphrasedialog.cpp - SecureString Conversion
|
||||
**Issue:** Password-handling paths were converting through `.c_str()` because `SecureString`
|
||||
did not have a convenient conversion helper from `std::string`.
|
||||
**Impact:** Unnecessary C-string shims in sensitive code paths.
|
||||
**Status:** Resolved.
|
||||
**Fix:** Added `MakeSecureString(const std::string&)` in `src/allocators.h` and updated
|
||||
the wallet RPC and passphrase dialog call sites to use it directly.
|
||||
|
||||
## Low Priority (Nice-to-Have)
|
||||
|
||||
|
||||
@@ -254,4 +254,9 @@ struct zero_after_free_allocator : public std::allocator<T>
|
||||
// This is exactly like std::string, but with a custom allocator.
|
||||
typedef std::basic_string<char, std::char_traits<char>, secure_allocator<char> > SecureString;
|
||||
|
||||
static inline SecureString MakeSecureString(const std::string& value)
|
||||
{
|
||||
return SecureString(value.begin(), value.end());
|
||||
}
|
||||
|
||||
#endif
|
||||
|
||||
@@ -99,11 +99,9 @@ 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->passEdit1->text().toStdString().c_str());
|
||||
newpass1.assign(ui->passEdit2->text().toStdString().c_str());
|
||||
newpass2.assign(ui->passEdit3->text().toStdString().c_str());
|
||||
oldpass = MakeSecureString(ui->passEdit1->text().toStdString());
|
||||
newpass1 = MakeSecureString(ui->passEdit2->text().toStdString());
|
||||
newpass2 = MakeSecureString(ui->passEdit3->text().toStdString());
|
||||
|
||||
switch(mode)
|
||||
{
|
||||
|
||||
+4
-10
@@ -1471,9 +1471,7 @@ Value walletpassphrase(const Array& params, bool fHelp)
|
||||
// Note that the walletpassphrase is stored in params[0] which is not mlock()ed
|
||||
SecureString strWalletPass;
|
||||
strWalletPass.reserve(100);
|
||||
// TODO: 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 = MakeSecureString(params[0].get_str());
|
||||
|
||||
if (strWalletPass.length() > 0)
|
||||
{
|
||||
@@ -1510,15 +1508,13 @@ Value walletpassphrasechange(const Array& params, bool fHelp)
|
||||
if (!pwalletMain->IsCrypted())
|
||||
throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrasechange was called.");
|
||||
|
||||
// TODO: 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 = MakeSecureString(params[0].get_str());
|
||||
|
||||
SecureString strNewWalletPass;
|
||||
strNewWalletPass.reserve(100);
|
||||
strNewWalletPass = params[1].get_str().c_str();
|
||||
strNewWalletPass = MakeSecureString(params[1].get_str());
|
||||
|
||||
if (strOldWalletPass.length() < 1 || strNewWalletPass.length() < 1)
|
||||
throw runtime_error(
|
||||
@@ -1566,11 +1562,9 @@ Value encryptwallet(const Array& params, bool fHelp)
|
||||
if (pwalletMain->IsCrypted())
|
||||
throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an encrypted wallet, but encryptwallet was called.");
|
||||
|
||||
// TODO: 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 = MakeSecureString(params[0].get_str());
|
||||
|
||||
if (strWalletPass.length() < 1)
|
||||
throw runtime_error(
|
||||
|
||||
Reference in New Issue
Block a user