c# - 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.