0

My MVC5 application is being scanned/spammed, and I'm getting 1,000's of exceptions in my logs. How can I resolve this?

Here's my model:

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

Here's my view:

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

Here's my action:

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

  return View(model);
}

When a spammer submits manually, a value such as "IsRememberMe=foo" in the POST, ModelState.IsValid==false, as expected and model.IsRememberMe==false as expected. However, when rendering the resulting view, Html.CheckBoxFor(m => m.IsRememberMe) throws an exception:

System.InvalidOperationException: The parameter conversion from type 'System.String' to type 'System.Boolean' failed.

Inside the controller, if I add:

string value = ModelState["IsRememberMe"].Value.AttemptedValue;

then value equals foo (the input value).

Note that under normal conditions, everything is working correctly.

It looks like Html.CheckBoxFor() is pulling the value from ModelState rather than model.IsRememberMe.

What is the best way to resolve this?

Update

Based on the comments and feedback, it looks like the use of data from ModelState is by design. I'm OK with that under normal circumstances.

Some comments have suggested this is a duplicate. While the solution may be similar, I argue that it is not a duplicate since the problem I am trying to solve is different: I am not actively trying to change the submitted value in my controller.

I am trying to prevent an exception in MVC caused by the submission of invalid data. I am posting my own answer below.

Matt Houser
  • 33,983
  • 6
  • 70
  • 88
  • `PostRedirectPattern`, redirect back to the GET action. – mxmissile Feb 20 '19 at 15:06
  • @mxmissile under what conditions do I perform the redirect? The model has valid values. Must I iterate through `ModelState` and perform my own model validation to know whether to redirect or not? – Matt Houser Feb 20 '19 at 15:15
  • `IsValid == false` if the other fields on the form failed validation. In that case, I don't want to redirect to back to GET. There, I want to return `View(model)` along with the model state errors so the user can retry. – Matt Houser Feb 20 '19 at 15:26
  • Got it, this is indeed interesting, are you using AntiForgery? – mxmissile Feb 20 '19 at 15:26
  • Yes I am. I did not include it above, but I am using `[ValidateAntiForgeryToken]` on the action along with `@Html.AntiForgeryToken()` in the view. That part is working fine. This is not a forgery case. I can replicate this problem using PostMan or curl. – Matt Houser Feb 20 '19 at 15:28
  • Check this... https://stackoverflow.com/questions/48204441/why-does-mvc-use-the-modelstate-over-a-supplied-model-on-a-get – mxmissile Feb 20 '19 at 15:32
  • Possible duplicate of [ASP.NET MVC 3 HtmlHelper.Textbox Uses ModelState as a source for value?](https://stackoverflow.com/questions/7640987/asp-net-mvc-3-htmlhelper-textbox-uses-modelstate-as-a-source-for-value) – Owen Pauling Feb 20 '19 at 15:44
  • https://blogs.msdn.microsoft.com/simonince/2010/05/05/asp-net-mvcs-html-helpers-render-the-wrong-value/ – Owen Pauling Feb 20 '19 at 15:45

1 Answers1

-1

I think this is a bug in the implementation of Html.CheckBoxFor(). Pulling the data from ModelState is fine, however, if the data cannot be converted, then it should fallback to using the value from the model instead of throwing an exception that halts rendering of the view.

I cannot think of a use-case where it's desirable to throw an exception (preventing rendering of the view) if the value in ModelState cannot be converted to a checked/unchecked for a checkbox. I'd be glad to hear from anyone who has such a use-case.

Solution

Due to the simplicity of the data type involved (bool), we can remove the value from ModelState if we are going to re-render the view. This will remove the offending value.

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

  if (ModelState.Keys.Contains("IsRememberMe"))
  {
    ModelState.Remove("IsRememberMe");
  }

  return View(model);
}
Matt Houser
  • 33,983
  • 6
  • 70
  • 88