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.

1 files changed, 4 insertions, 10 deletions

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");