Search code examples
c#asp.net-mvcsecuritymodel-bindingcheckmarx

Keep Sensitive Data Off Heap


While scanning an ASP.net MVC application using Checkmarx, I regularly see heap inspection vulnerabilities. So I started to wonder if I could use a custom model binder or the built-in ByteArrayModelBinder to keep passwords and other sensitive strings out of the heap, for controlled disposal. I came up with the following solution which posts and displays the sensitive data via a byte array, but I am wondering if this data is still making its way into the heap through a string somewhere. (Note: The display action is just for debugging.)

ViewModel

public class ByteArrayViewModel
{
    public byte[] SensitiveData { get; set; }        
}

Input View

@model MvcHandlingSensativeStrings.Models.ByteArrayViewModel

@{
    ViewBag.Title = "byte[] Post";
    Layout = "~/Views/Shared/_Layout.cshtml";
}

<h2>@ViewBag.Title</h2>
@using (Html.BeginForm("Post", "ByteArray", FormMethod.Post))
{
    @Html.TextBoxFor(m=>m.SensitiveData);
    <button type="submit">Send</button>
}

enter image description here

Controller

public class ByteArrayController : Controller
{
    public ActionResult Index()
    {
        return View(new ByteArrayViewModel());
    }

    [HttpPost]
    public ActionResult Post(ByteArrayViewModel viewModel)
    {
        try
        {
            // Handle sensitive data here
            return View("Display", viewModel);
        }
        catch
        {
            return View("Index");
        }
        finally
        {
            // Clear sensitive data from memory
            //Array.Clear(viewModel.SensitiveData, 0, viewModel.SensitiveData.Leng
        }
    }

    public ActionResult Display(ByteArrayViewModel viewModel)
    {
        return View(viewModel);
    }
}

Display View

@model MvcHandlingSensativeStrings.Models.ByteArrayViewModel

@{
    ViewBag.Title = "byte[] Display";
    Layout = "~/Views/Shared/_Layout.cshtml";
    string s = Convert.ToBase64String(Model.SensitiveData);
}

<h2>@ViewBag.Title</h2>
<p>@s</p>
<p>@Model.SensitiveData.GetType().ToString()</p>

Display Output

enter image description here

* Update *

This shows that before ByteArrayModelBinder or any other model binder executes, form parameters are stored in an array of strings and are thus susceptible to heap inspection.

enter image description here

* Update 2 *

If you peek at Microsoft's implementation of NetworkCredential, you will noticed that even though the Password property is a string, SecureString is used underneath for storage.


Solution

  • The answer is no, you are not making this one bit more secure.

    To avoid storing secrets in memory that could be recovered, you need to use the SecureString class in C# (or the underlying crypto API in Windows). This will go through some effort to make the secrets harder to recover by erasing them on dispose, but also making sure they never end up in the pagefile.

    But more importantly, printing the secrets out to the web page (even if using TLS) obviously exposes the secret to all layers of the web stack, making it vulnerable to multiple local attacks, as well as browser attacks on the client.

    The solution is to never ever store passwords in clear text, and instead use salted hashes. Don't try to invent stuff like this yourself, use proven solutions for verifying and storing passwords securely. It is very easy to overlook something that completely voids the security of your application. For example, the crypto API has a facility for this. See for example here: How to store and retrieve credentials on windows using C#

    If you absolutely need to accept "secret" user data coming from a form, and you want to protect against attacks on the server:

    1. Protect the process against remote exploits in other processes on the same machine (run in isolated VM, run in Docker, run everything as separate (non-root) users
    2. To protect against local attacks, make sure that the sensitive data does not end up in persistent storage (i.e. SecureString). This will be hard to do using your current implementation as already pointed out, as you need to harden all layers of your application, from SSL/TLS termination and down to the controller.