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
):
0std::string orig_string;
1std::cin >> orig_string;
2SecureString s;
3s.reserve(100);
4// The following all compile identically
5s = orig_string;
6s = std::string_view{orig_string};
7s.assign(std::string_view{orig_string});
8s.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.
Fixes #27067.