Hashing a SecureString in .NET

Question

In .NET, we have the SecureString class, which is all very well until you come to try and use it, as to (for example) hash the string, you need the plaintext. I've had a go here at writing a function that will hash a SecureString, given a hash function that takes a byte array and outputs a byte array.

private static byte[] HashSecureString(SecureString ss, Func<byte[], byte[]> hash) {     // Convert the SecureString to a BSTR     IntPtr bstr = Marshal.SecureStringToBSTR(ss);      // BSTR contains the length of the string in bytes in an     // Int32 stored in the 4 bytes prior to the BSTR pointer     int length = Marshal.ReadInt32(bstr, -4);      // Allocate a byte array to copy the string into     byte[] bytes = new byte[length];      // Copy the BSTR to the byte array     Marshal.Copy(bstr, bytes, 0, length);      // Immediately destroy the BSTR as we don't need it any more     Marshal.ZeroFreeBSTR(bstr);      // Hash the byte array     byte[] hashed = hash(bytes);      // Destroy the plaintext copy in the byte array     for (int i = 0; i < length; i++) { bytes[i] = 0; }      // Return the hash     return hashed; } 

I believe this will correctly hash the string, and will correctly scrub any copies of the plaintext from memory by the time the function returns, assuming the provided hash function is well behaved and doesn't make any copies of the input that it doesn't scrub itself. Have I missed anything here?

Answer 1

Have I missed anything here?

Yes, you have, a rather fundamental one at that. You cannot scrub the copy of the array left behind when the garbage collector compacts the heap. Marshal.SecureStringToBSTR(ss) is okay because a BSTR is allocated in unmanaged memory so will have a reliable pointer that won't change. In other words, no problem scrubbing that one.

Your byte[] bytes array however contains the copy of the string and is allocated on the GC heap. You make it likely to induce a garbage collection with the hashed[] array. Easily avoided but of course you have little control over other threads in your process allocating memory and inducing a collection. Or for that matter a background GC that was already in progress when your code started running.

The point of SecureString is to never have a cleartext copy of the string in garbage collected memory. Copying it into a managed array violated that guarantee. If you want to make this code secure then you are going to have to write a hash() method that takes the IntPtr and only reads through that pointer.

Beware that if your hash needs to match a hash computed on another machine then you cannot ignore the Encoding that machine would use to turn the string into bytes.

Answer 2

There's always the possibility of using the unmanaged CryptoApi or CNG functions. Bear in mind that SecureString was designed with an unmanaged consumer which has full control over memory management in mind.

If you want to stick to C#, you should pin the temporary array to prevent the GC from moving it around before you get a chance to scrub it:

private static byte[] HashSecureString(SecureString input, Func<byte[], byte[]> hash) {     var bstr = Marshal.SecureStringToBSTR(input);     var length = Marshal.ReadInt32(bstr, -4);     var bytes = new byte[length];      var bytesPin = GCHandle.Alloc(bytes, GCHandleType.Pinned);     try {         Marshal.Copy(bstr, bytes, 0, length);         Marshal.ZeroFreeBSTR(bstr);          return hash(bytes);     } finally {         for (var i = 0; i < bytes.Length; i++) {              bytes[i] = 0;          }          bytesPin.Free();     } }