Pass all characters to SecureString including nulls

`SecureString` is a `std::string` specialization with
a secure allocator. However, it's treated like a C-
string (no explicit length and null-terminated). This
can cause unexpected behavior. For instance, if a user
enters a passphrase with an embedded null character
(which is possible through Qt and the JSON-RPC), it will
ignore any characters after the null, giving the user
a false sense of security.

Instead of assigning `SecureString` via `std::string::c_str()`,
assign it via a `std::string_view` of the original. This
explicitly captures the size and doesn't make any extraneous
copies in memory.

4 files changed, 10 insertions, 16 deletions

diff --git a/src/qt/askpassphrasedialog.cpp b/src/qt/askpassphrasedialog.cpp
index d15aba5cdd..d437e92a93 100644
--- a/src/qt/askpassphrasedialog.cpp
+++ b/src/qt/askpassphrasedialog.cpp
@@ -89,11 +89,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->passEdit1->text().toStdString().c_str());
-    newpass1.assign(ui->passEdit2->text().toStdString().c_str());
-    newpass2.assign(ui->passEdit3->text().toStdString().c_str());
+
+    oldpass.assign(std::string_view{ui->passEdit1->text().toStdString()});
+    newpass1.assign(std::string_view{ui->passEdit2->text().toStdString()});
+    newpass2.assign(std::string_view{ui->passEdit3->text().toStdString()});
 
     secureClearPassFields();
 
diff --git a/src/support/allocators/secure.h b/src/support/allocators/secure.h
index c6bd685189..a0918bf463 100644
--- a/src/support/allocators/secure.h
+++ b/src/support/allocators/secure.h
@@ -56,6 +56,7 @@ struct secure_allocator : public std::allocator<T> {
 };
 
 // This is exactly like std::string, but with a custom allocator.
+// TODO: Consider finding a way to make incoming RPC request.params[i] mlock()ed as well
 typedef std::basic_string<char, std::char_traits<char>, secure_allocator<char> > SecureString;
 
 #endif // BITCOIN_SUPPORT_ALLOCATORS_SECURE_H
diff --git a/src/wallet/rpc/encrypt.cpp b/src/wallet/rpc/encrypt.cpp
index 38960bda7b..b9271e8dd1 100644
--- a/src/wallet/rpc/encrypt.cpp
+++ b/src/wallet/rpc/encrypt.cpp
@@ -49,9 +49,7 @@ RPCHelpMan walletpassphrase()
         // Note that the walletpassphrase is stored in request.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 request.params[0] mlock()'d to begin with.
-        strWalletPass = request.params[0].get_str().c_str();
+        strWalletPass = std::string_view{request.params[0].get_str()};
 
         // Get the timeout
         nSleepTime = request.params[1].getInt<int64_t>();
@@ -132,15 +130,13 @@ RPCHelpMan walletpassphrasechange()
 
     LOCK2(pwallet->m_relock_mutex, pwallet->cs_wallet);
 
-    // TODO: get rid of these .c_str() calls by implementing SecureString::operator=(std::string)
-    // Alternately, find a way to make request.params[0] mlock()'d to begin with.
     SecureString strOldWalletPass;
     strOldWalletPass.reserve(100);
-    strOldWalletPass = request.params[0].get_str().c_str();
+    strOldWalletPass = std::string_view{request.params[0].get_str()};
 
     SecureString strNewWalletPass;
     strNewWalletPass.reserve(100);
-    strNewWalletPass = request.params[1].get_str().c_str();
+    strNewWalletPass = std::string_view{request.params[1].get_str()};
 
     if (strOldWalletPass.empty() || strNewWalletPass.empty()) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase cannot be empty");
@@ -241,11 +237,9 @@ RPCHelpMan encryptwallet()
 
     LOCK2(pwallet->m_relock_mutex, pwallet->cs_wallet);
 
-    // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string)
-    // Alternately, find a way to make request.params[0] mlock()'d to begin with.
     SecureString strWalletPass;
     strWalletPass.reserve(100);
-    strWalletPass = request.params[0].get_str().c_str();
+    strWalletPass = std::string_view{request.params[0].get_str()};
 
     if (strWalletPass.empty()) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase cannot be empty");
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index 23a88cd51b..a6cdad6642 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -359,7 +359,7 @@ static RPCHelpMan createwallet()
     passphrase.reserve(100);
     std::vector<bilingual_str> warnings;
     if (!request.params[3].isNull()) {
-        passphrase = request.params[3].get_str().c_str();
+        passphrase = std::string_view{request.params[3].get_str()};
         if (passphrase.empty()) {
             // Empty string means unencrypted
             warnings.emplace_back(Untranslated("Empty string given as passphrase, wallet will not be encrypted."));