CWE-ID 100 Fix for MVC5

Question

Our application is being dinged several hundred times CWE-ID 100 "flaws" related to Technology-Specific Input Validation Problems according to Veracode.

According to their docs, the remediation is to check the ModelState.IsValid property on a model before using it. We do this on every controller action yet we are still dinged. An example controller action follows.

public async Task<ActionResult> DeliverySummary (ReportsViewModel Model)
{
    if (ModelState.IsValid)
    {
        /* Other processing occurs here */ 

        //finally return View
        return View(Model);
    }
    else 
    {
        return View();
    }
}

We have the System.ComponentModel.DataAnnotations on our model properties.

Has anyone ever encountered this?

Answer 1

I've been dealing with this myself. The basic culprit is you don't have [Bind] set on your argument, specifying the properties that are allowed.

My old sign-in controller action was like this

public ActionResult SignIn(SignInViewModel viewModel)

And to correct it, I need it to read like this

public ActionResult SignIn([Bind(Include = "Email,Password,UtcOffset")]SignInViewModel viewModel)

What this says to MVC is only the properties Email, Password and UtcOffset will be read from SignInViewModel, so if a hacker also set LastLogonTime it would be ignored.

As an aside, due to the security checks from Veracode, I'm thinking this kind of model-binding is now incredibly awkward, considering devs now have to keep strings in sync with prop names at the target. What a hassle.