Jaa

How to properly convert SecureString to String

  • Artikkeli

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: https://blogs.msdn.com/shawnfa/archive/2004/05/27/143254.aspx .

Comments

  • Anonymous
    June 12, 2009
    PingBack from http://microsoft-sharepoint.simplynetdev.com/how-to-properly-convert-securestring-to-string/

  • Anonymous
    November 23, 2009
    Thank you for this: I was using a different version I found some time ago:            IntPtr ptr = Marshal.SecureStringToBSTR(ss);            string s = Marshal.PtrToStringAuto(ptr); // this now makes it insecure and held in regular managed memory            Marshal.ZeroFreeBSTR(ptr); but recently found this doesn't work under Mono! Your solution does.

  • Anonymous
    October 31, 2010
    What does this mean?! In the method declaration ... public static string ConvertToUnsecureString(this SecureString ...) why does the word this preceed the type! I tried googling the answer but apparently the word 'this' is quite popular outside the realm of computing... Cheers!

  • Anonymous
    October 31, 2010
    The 'this' keyword in the method declaration makes the static method  be considered an extension method (see msdn.microsoft.com/.../bb383977.aspx). Then you can invoke the method as if it was part of the original type. For example:  string password = "SecretPassword";  SecureString securePassword = password.ConvertToSecureString();  string password2 = securePassword.ConvertToUnsecureString(); Enjoy!

  • Anonymous
    November 01, 2010
    fpintos, Sadly enough, i probably will enjoy knowing that. It gives me yet another subject not to discuss with my girlfriend :0).

  • Anonymous
    June 04, 2013
    How about this extension method for reading a value from AppSettings into a SecureString?  Did I get anything wrong?        public static void ReadFromAppSetting(this SecureString value, string key)        {            char[] appSettingValueCharacters;            if (ConfigurationManager.AppSettings[key] == null)                appSettingValueCharacters = new char[] { };            else                appSettingValueCharacters = ConfigurationManager.AppSettings[key].ToCharArray();            foreach (char c in appSettingValueCharacters)                value.AppendChar(c);        }

  • Anonymous
    June 04, 2013
    Since the appSettings returns a string you can then simply call ConvertToSecureString on the returned string. ReadFromAppSetting as written above is leaving the secure string unlocked; you might want to lock it down.

  • Anonymous
    June 04, 2013
    Good points.  Thanks.

  • Anonymous
    June 04, 2013
    The reason I am not using ConvertToSecureString is because it requires me to add the /unsafe compiler switch to my code, and my organization does not allow unsafe code blocks.

  • Anonymous
    June 04, 2013
    Revised.        public static void ReadFromAppSetting(this SecureString value, string key)        {            char[] appSettingValueCharacters;            if (ConfigurationManager.AppSettings[key] == null)                appSettingValueCharacters = new char[] { };            else                appSettingValueCharacters = ConfigurationManager.AppSettings[key].ToCharArray();            foreach (char c in appSettingValueCharacters)                value.AppendChar(c);            value.MakeReadOnly();        }

  • Anonymous
    June 04, 2013
    "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." Why?  Because it's using Interop Services?  Because it's doing pointer stuff and marshalling in code NOT marked as unsafe?

  • Anonymous
    June 04, 2013
    You can replace the part of ConvertToSecureString that uses pointer and use the foreach on the char array if you want to avoid the /unsafe flag. Using the char* ctor was just a perf optimization. As for torching a village when ConvertToUnsecureString is called, it is because if you are taking a secure string and converting to a plain string then all benefits of having a secure string go out the window. The whole idea of SecureString was to NEVER have a plain text password in System.String. The marshaling functions allows SecureString to play with native functions, which is OK, because you can force the system to blot that memory after using it, so the risk of having the plain text password is present only while actually using that password in native code, but if you put the plain text password in System.String you give up control of when and how that password will be gone from the process memory. You will have a garbage-collected string with the plain text password lying around in memory for an undetermined amount of time. If you process happens to crash during this time and saves a dump, the password will be on that dump. If that dump gets transmitted to a crash analysis service, someone not authorized might find it. It gets worse if the dump contains enough information that allows one to link that password to a user name.

  • Anonymous
    November 05, 2013
    According to MSDN, PtrToStringUni creates a COPY of the string - so doesn't this make everything you've done pointless? - you still end up with a System.String with the password (or whatever) in it to be disposed of at the system's leisure?

  • Anonymous
    November 06, 2013
    Yes, converting the secure string to a plain string will leave you with the password in memory for an unknown amount of time, which is why one should avoid the conversion in the first place. The idea here is that if you need to convert, then at least convert in a way that avoids the memory leaks.

  • Anonymous
    December 21, 2013
    This code is completely redundant most of the time since NetworkCredential does it for you: it has constructors that accept both String and SecureString, but it stores only SecureStrings in memory. So: var cred = new NetworkCredential("U", "P"); // creates a SecureString cred.SecurePassword // returns the SecureString cred.Password // converts the SecureString to a String

  • Anonymous
    January 29, 2014
    Very good to point that out John. It is nice to see that the .net framework 4 updated NetworkCredential with SecureString, making functions like these even less necessary.

  • Anonymous
    December 16, 2014
    Hello, could anybody explain me, where the security is? Everybody can convert the secured String to a plain String, so the text is not really encrypted for spying?!? Thank you

  • Anonymous
    December 16, 2014
    The idea of SecureString is to avoid keeping a password in plain-text in memory, as a defense in-depth mechanism. Security comes from SecureString using RtlEncryptMemory to encrypt the buffer in a way that only the same process can decrypt it. If another user/process/machine gets a hold of the secured buffer, it will not have the key to decrypt it. Some scenarios this helps with:

  • suppose the process crashes and generates a memory dump which gets copied around for analysis; if passwords are stored in plain text, scanning the dump will reveal them. With SecureString, they will not be readable by the dump analyst.
  • suppose the process has a vulnerability that allows an attacker to read memory from the process, along the lines of Hearthbleed. With SecureString, passwords will not be readable by the attacker.

  • Anonymous
    May 27, 2015
    Microsoft really needs to add something to the CLR and BCL that will cause a particular value of a string to be stored in secure memory.  Similar to String.Intern but in a differect and more secure pool.  The CLR could pull the value from this other pool when needed and anytime the same string value is used from that point on would be in secure memory away from dumps. It would get rid of the limitations of SecureString and these unsafe code blocks as well and make MVVM login screens just as secure as everything else.  (which is totally pointless anyways because it has to be used as string to do anything except create network credentials in the full framework.)

  • Anonymous
    October 12, 2015
    try this . string password = new System.Net.NetworkCredential(string.Empty, securePassword).Password;