It feels like forever since SecureString has been introduced in the .Net Framework and yet I keep seeing it being mishandled, in particular by code that returns or receives passwords in clear text, so in this post I’ll show what you should be looking for when doing code reviews and finding conversions from SecureString to string.
The main idea with SecureString is that you would never store a password or other textual secret in plain text, unencrypted memory – at least not in a System.String instance. Unfortunately, SecureString was introduced in the framework only after plenty of APIs were built and shipped using passwords stored in String, such as System.Net.NetworkCredential, so an application that must use these APIs has no option but to convert SecureStrings to strings.
However, the SecureString class itself doesn’t provide any methods to get back a plain string with its content, exactly to discourage this type of usage. What a developer has to do is use functions from System.Runtime.InteropServices.Marshal to get a native buffer with the plain string, marshal the value into a managed string then – very importantly – free the native buffer.
And this is where I’ve seen people people making mistakes. You can certainly spot the problems in the following functions:
public static string BAD_ConvertToUnsecureString(this SecureString securePassword)
{
IntPtr unmanagedString = Marshal.SecureStringToGlobalAllocUnicode(securePassword);
var s = Marshal.PtrToStringUni(unmanagedString);
Marshal.ZeroFreeGlobalAllocUnicode(unmanagedString);
return s;
}
public static string REALLY_BAD_ConvertToUnsecureString(this SecureString securePassword)
{
IntPtr unmanagedString = Marshal.SecureStringToGlobalAllocUnicode(securePassword);
return Marshal.PtrToStringUni(unmanagedString);
}
The first one is almost ok, but if an exception happens after the native buffer is allocated then we leak that password. The second one is just plain wrong; the developer completely forgot to free the native buffer, introducing an unconditional leak of that password.
I’ve seen both mistakes being freely cut-and-paste because the original code was embedded in a larger function that was doing multiple things, so the details of the conversion were easy to overlook. Lacking a simple, reusable function leads to that, so don’t let these bugs creep into your project.
The correct implementation is to use a try/finally block to free the native buffer. Here’s an example, using SecureStringToGlobalAllocUnicode and ZeroFreeGlobalAllocUnicode:
public static string ConvertToUnsecureString(this SecureString securePassword)
{
if (securePassword == null)
throw new ArgumentNullException("securePassword");
IntPtr unmanagedString = IntPtr.Zero;
try
{
unmanagedString = Marshal.SecureStringToGlobalAllocUnicode(securePassword);
return Marshal.PtrToStringUni(unmanagedString);
}
finally
{
Marshal.ZeroFreeGlobalAllocUnicode(unmanagedString);
}
}
Please understand that a little village bursts on fire every time you call such a function, but if you do need it, write it like this, so there’s no excuse for anyone to write that code again.
The extension method syntax makes it easy to call the conversion as if it was part of SecureString itself:
SecureString password = someCodeToGetSecureString();
var credentials = new NetworkCredential(userName, password.ConvertToUnsecureString())
While at it you may want to provide the counterpart that converts a plain String to a SecureString. Following the same pattern as above we’ll have:
public static SecureString ConvertToSecureString(this string password)
{
if (password == null)
throw new ArgumentNullException("password");
unsafe
{
fixed (char* passwordChars = password)
{
var securePassword = new SecureString(passwordChars, password.Length);
securePassword.MakeReadOnly();
return securePassword;
}
}
}
The important point here is to mark the secure string as read-only before returning it – it is very unlikely that you’ll need to edit a secure string that was created from an existing string. You’ll also note that in the above example I used unsafe code to initialize the SecureString from its fixed/char* representation, which is about 10x faster than using AppendChar. If you are not allowed to use unsafe code, don’t worry, just write it using AppendChar and it works fine.
I’m not proud to have clear text passwords in my process memory, but we’ll have to live with it until the day somebody updates the framework to use SecureString everywhere, particularly in System.Net.
If you need to learn more about SecureString this post is a great start: http://blogs.msdn.com/shawnfa/archive/2004/05/27/143254.aspx.