Is checkmarx mark properties named Password as Medium vulnerability regardless to the underlining implementation?

Question

I'm a little bit confused about why checkmarx mark the below public property Password as medium vulnerability of type Heap Inspection.

Any idea why checkmarx mark this line as vulnerability ? Any recommendations/improvement ideas for the code below ?

SecureString password;

public String Password
{
    get { return SecureStringToString(password); }
    set
    {
        if (value != null)
        {
            password = new SecureString();
            foreach (char c in value) password.AppendChar(c);
        }
    }
}

String SecureStringToString(SecureString value)
{
    IntPtr valuePtr = IntPtr.Zero;
    try
    {
        valuePtr = Marshal.SecureStringToGlobalAllocUnicode(value);
        return Marshal.PtrToStringUni(valuePtr);
    }
    finally
    {
        Marshal.ZeroFreeGlobalAllocUnicode(valuePtr);
    }
}

Answer 1

The relevant Checkmarx query does care about the underlying implementation, it precisely looks for password fields/variables of type String. Since you allow your SecureString to be returned as a regular String, as soon as that happens it is once again exposed and becomes vulnerable to "heap inspection" (i.e. the plaintext password is stored in an immutable string variable in memory).

In order to prevent this, you would need to redesign your application so that a plain text password is never needed - it should always be hashed (or rather bcrypt/scrypt/PBKDF2'd) right away.
In the rare cases that a plaintext password is required, store it in a byte array, and then zeroize the array when you are done with it - this doesn't completely solve the issue, but it does allow you to minimize the exposure window, and control the password from being passed around the heap too much.