wallet: SecureString to allow null characters

[barton2526]

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

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

Note to reviewers, the following all compile identically in recent GCC (x86-64 and ARM64) with -O2 (and -std=c++17):

std::string orig_string;
std::cin >> orig_string;
SecureString s;
s.reserve(100);
// The following all compile identically
s = orig_string;
s = std::string_view{orig_string};
s.assign(std::string_view{orig_string});
s.assign(orig_string.data(), orig_string.size());

So it's largely a matter of preference. However, one thing to keep in mind is that we want to avoid making unnecessary copies of any sensitive data in memory.

Something like SecureString s{orig_string}; is still invalid and probably unwanted in our case, since it'd get treated as a short string and optimized away from the secure allocator. I presume that's the reason for the reserve() calls.

Ref: bitcoin/bitcoin#27068

[barton2526]

I need help testing this.

[div72]

The diff looks fine, the std::string_view casts are completely useless but keeping them would be better to remain closer to upstream.

I haven't tested if it worked previously but I can confirm the GUI properly accepts the null with this PR, the problem is there's no sane way to pass a password with null for daemon users. You cannot pass it through gridcoinresearchd as the arguments are parsed as C-strings will be just cut off at null. Even if you could (or use curl etc.) the RPC interface chokes when null is passed and does not seem to properly interpret \0 or \u0000 in the JSON.

[jamescowens]

Based on my testing, I think the chances of a problem here are minimal.