Problem
Encrypting a wallet through JSON RPC or Qt appears to allow a user to include null characters in the passphrase, but silently ignores everything including and after the first null character.
For instance (on regtest), trying to set a passphrase of “a{null character}b”:
curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "encryptwallet", "params": ["a\u0000b"]}' -H 'content-type: text/plain;' http://127.0.0.1:18443/
This will succeed, but allow the user to unlock with the passphrase “a”, instead of the expected full passphrase (which also works):
curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "walletpassphrase", "params": ["a",10]}' -H 'content-type: text/plain;' http://127.0.0.1:18443/
I am also able to replicate it in Qt on my macOS machine by running printf 'a\0b' | pbcopy and pasting the result into the passphrase dialog.
My main concern is a user thinking that they’re generating, say, 32 random bytes as a passphrase, and if they’re unlucky and get a zero in the first few bytes, it unexpectedly cuts their security down to almost nothing.
Root Cause
The reason is due to our SecureString type. SecureString is a std::string specialization with a secure allocator. However, when assigned, it’s treated like a C- string (no explicit length and null-terminated). See the original PR for more details.
Potential Solutions
I think there are two plausible approaches to take. The first (and my preference) is to allow and support null characters, and I will submit a PR that enables that (by making SecureString use the entire string). The second is to explicitly reject any passphrases that contain null characters.
One significant complication may be that, if anyone is already using a passphrase with a null, then my first solution would stop their wallet from unlocking. However, it would still be unlockable just by trimming the null and any subsequent characters.
