SecureString as SqlParameter value without GC concerns

The purpose here is to make it easy to use sensitive data that is already in the form of a SecureString (example) without converting it to a String object and risking more leaks than necessary.

SecureString isn't about total security, but it is about reducing attack surface. For example, when you call SecureString.AppendChar there is a brief flash where it decrypts the contents, adds your character, and reencrypts. This is still better than storing your password in the clear on the heap for any amount of time.

So in a similar vein, if I'm to use a SecureString as a SqlParameter value, it's best to do as little as possible with the contents in the clear and erase it as soon as possible. This isn't about transport security to SQL server, just C# process memory that has the potential to be paged to disk and end up unerased, in the clear, for years.

Usage:

var secureString = new SecureString();
secureString.AppendChar('a');
secureString.AppendChar('q');
secureString.AppendChar('1');

using (var command = new SqlCommand("select case when @secureParam = 'aq1' then 'yes' else 'no' end", connection))
{
    object returnValue;

    using (command.Parameters.AddSecure("secureParam", secureString))
    {
        // At this point no copies exist in the clear
        returnValue = (string)command.ExecuteScalar();
        // Now one pinned String object exists in the clear (referenced at the internal property command.Parameters[0].CoercedValue)
    }
    // At this point no copies exist in the clear
}

Code:

```
public static class SecureSqlParameterExtensions
{
[DllImport("kernel32.dll", EntryPoint = "CopyMemory")]
private static extern void CopyMemory(IntPtr dest, IntPtr src, IntPtr count);
[DllImport("kernel32.dll", EntryPoint = "RtlZeroMemory")]
private static extern void ZeroMemory(IntPtr ptr, IntPtr count);

///
/// You must dispose the return value as soon as SqlCommand.Execute* is called.
///
public static IDisposab

I don't think you should do this.

public string ToString(IFormatProvider provider)

SecureString doesn't override ToString. It just returns it's own name. I would recommend doing the same here. It seems to defeat the purpose of using it, if you're going to such extreme measures to then retrieve the value via a ToString method.

I mean, yes. I understand that you do eventually have to retrieve the actual string, but this is not the place to do that. There's a reason SecureString doesn't provide a method to get to the plain text value. It's supposed to be difficult to get out. Allowing people to call secureParameterValue.ToString() removes all of the benefits of using SecureString.

private sealed class SecureStringParameterValue : IConvertible, IDisposable
{
    private readonly SecureString secureString;
    private int length;
    private string insecureManagedCopy;
    private GCHandle insecureManagedCopyGcHandle;

I love that this is sealed. That's great. Security sensitive things should be sealed. But again, storing a insecureManagedCopy for the life of the class defeats the purpose. Now you're at the mercy of the garbage collector.

When you do need to access the actual value, you want to make sure that the unmanaged copy is removed as quickly as possible. Here is an example implementation that I submitted to the LibGit2Sharp project.

public sealed class SecureUsernamePasswordCredentials : Credentials
{
    /// 
    /// Callback to acquire a credential object.
    /// 
    /// The newly created credential object.
    /// 0 for success, < 0 to indicate an error, > 0 to indicate no credential was acquired.
    protected internal override int GitCredentialHandler(out IntPtr cred)
    {
        if (Username == null || Password == null)
        {
            throw new InvalidOperationException("UsernamePasswordCredentials contains a null Username or Password.");
        }

        IntPtr passwordPtr = IntPtr.Zero;

        try
        {
            passwordPtr = Marshal.SecureStringToGlobalAllocUnicode(Password);

            return NativeMethods.git_cred_userpass_plaintext_new(out cred, Username, Marshal.PtrToStringUni(passwordPtr));
        }
        finally
        {
            Marshal.ZeroFreeGlobalAllocUnicode(passwordPtr);
        }

    }

Notice how the consumer is the one responsible for decoding and using it? Also note that the unmanaged string is always released from memory before the method exits. My implementation is faced with many of the same issues. It still has to decode the string, but it never exposes a manage string to anyplace it could be copied. It gets sent to the external library with no room for anyone to ever make copies of the password. You should strive a little more towards that goal.

To make things clear, there's no reason a developer couldn't do this with your implementation.

using (var command = new SqlCommand("select case when @secureParam = 'aq1' then 'yes' else 'no' end", connection))
{
    object returnValue;
    string pswd;
    using (command.Parameters.AddSecure("secureParam", secureString))
    {
        // At this point no copies exist in the clear
        returnValue = (string)command.ExecuteScalar();
        pswd = command.Parameters[0].CoercedValue;
    }

    // At this point the password is stored in a plain string.
}

Or, a developer doesn't use a using block. That would also lead to having the plain string in memory. Even worse, someone could forget to Dispose() it.

One last note. Once you've removed the ToString method, there's no longer a reason to implement IConvertible and those nasty unimplmented methods can be removed. Actually, I'm not sure why they were there to begin with. You didn't need to implement IConvertible in order to override ToString.

StackExchange Code Review Q#87300, answer score: 5