How to resolve CWE-316 in .NET using SecureString in a neat and easy way.
According to CWE-316, we should not be storing sensitive information in cleartext in memory.
Example way of exposure:
When your computer writes memory content to swap, sensitive information will be left on disk.
There is a neat solution on StackOverflow, Convert String to SecureString.
Quoting the contribution by Hossein Narimani Rad below:
SecureString ss = new NetworkCredential("", "myPass").SecurePassword;
string s = new NetworkCredential("", ss).Password;
I like this solution because it makes use of .NET’s library and it accomplishes it in a single line.
There are a few considerations when using this in a class.
A sample class with comments. Do read them to understand my reasonings:
public sealed class Person : BaseModel
{
// The backing store has to be SecureString.
private SecureString password;
public Person(string name, string password)
{
Name = name;
Password = password;
}
/*
* Typically, we should opt for deterministic disposal of resources,
* i.e. file streams.
* This is so that your application can release locks allowing
* other threads or applications to get access to them.
*
* In SecureString however, MSDN states that its Dispose method
* "writes binary zeroes to the allocated memory that contains the value of
* this SecureString object, then frees the allocated memory."
*
* Should I be worried?
* It is encrypted, I would not be too worried about its exposure,
* so I leave this class without an IDisposable implementation.
* This is especially since this would otherwise be the only model class
* throughout the application that would require IDisposable implementation.
* For this specific reason, I would rather not have IDisposable.
*
* Why?
* Imagine if this class inherits a Base class.
* The Base class however does not implement IDisposable.
* As such, the caller application then needs to determine if child classes
* can be casted to IDisposable in order to perform deterministic cleanup.
*/
~Person()
{
password?.Dispose();
}
public string Name { get; }
// I would like the public property to interact in string however.
// The caller should not be keeping a copy of this password in plain string.
// Reference this property directly to minimise exposure of the plain string.
public string Password
{
get
{
// Simplest way to convert string to SecureString.
return password == null ? string.Empty : new NetworkCredential(string.Empty, password).Password;
}
// If you would like to allow changing password, remove the `private` keyword
private set
{
// If not null, dispose.
// Here, we can and should dispose before replacing it.
password?.Dispose();
password = new NetworkCredential(string.Empty, value).SecurePassword;
}
}
}
Agree or disagree? Do drop me a comment below.