CODEWITHSUNDEEP
c#asp.net-mvc
Rodolfo
Rodolfo

Reputation: 247

Users modify hidden value of model in View (@Html.HiddenFor)

I edit a existing object with this View:

@using (Html.BeginForm("EditPat","bff"))
{
    @Html.AntiForgeryToken()
    @Html.ValidationSummary(true)
    @Html.HiddenFor(m => m.IdNumPart)
    ...more code

Actually the IdNumPart field is not visible for common users, but if they use Chrome DevTools they can modify the hidden value:

<input data-val="true" data-val-number="The field IdNumPart must be a number." data-val-required="The IdNumPart field is required." id="IdNumPart" name="IdNumPart" type="hidden" value="13">

And when they submit that will affect the logic in my controller, in way that they can modify any record changing the IdNumPart

[HttpPost]
public ActionResult EditPat(inspListaNoParte model)
{
    try
    {
        var oldModel = bff.inspListaNoPartes.Find(model.IdNumPart);
        bff.Entry(oldModel).State = EntityState.Modified;
        oldModel.UMC = model.UMC;
        bff.SaveChanges();
        TempData["AlertMessage"] = "Success";
    }
    catch (Exception ex)
    {
        TempData["ErrorMessage"] = "Error";
        throw;
    }
    return View();
}

So, I'm looking for a way that I can protect me from this issues.

Upvotes: 1

Views: 1197

Answers (4)

Zuzlx
Zuzlx

Reputation: 1266

One other way to do it is to hash the value of IdNumPart and send it as a second hidden field. You should hash it with some sort of server key though. On post back, the hash value of the second field must match the IdNumPart. If any one changes even one char of the hash or the IdNumPart, you will know and you should reject the change. Another advice would be that never take a security suggestion from someone on the internet who, from top of his head, comes up with a security solution.

Upvotes: 1

Senad Meškin
Senad Meškin

Reputation: 13756

You can use attributes to forbid access to methods for certain users

[HttpPost]
[Authorize(Roles = "Administrator,Editors")]
public ActionResult EditPat(inspListaNoParte model)
{
    try
    {
        var oldModel = bff.inspListaNoPartes.Find(model.IdNumPart);
        bff.Entry(oldModel).State = EntityState.Modified;
        oldModel.UMC = model.UMC;
        bff.SaveChanges();
        TempData["AlertMessage"] = "Success";
    }
    catch (Exception ex)
    {
        TempData["ErrorMessage"] = "Error";
        throw;
    }
    return View();
}

or you can check if user is inside group using method

if(User.IsInRole("Administrator"))

and add field to your db table "ModifiedBy" so you can know who was the last person who modified the record.

oldModel.LastModifiedBy= WebSecurity.CurrentUserId;

If user has access to edit mode for certain products then just check if that product belongs to him

if(oldModel.OwnerId == WebSercurity.CurrentUserId)

I hope this helps.

Upvotes: 2

Berin Loritsch
Berin Loritsch

Reputation: 11473

This dilemma is common to all web applications. There's a few approaches with varying tradeoffs.

  • Force any editing to be authenticated (good practice). This doesn't prevent the problem, but at least you have a record of who is being a bad actor and you can disable their account. (other answers have more detail on this)
  • Keep hidden fields in some form of session storage. Advantage is that the only info in the client is the session id. The session Id is used to bring back the user's information. You'll have to reassemble the record yourself, and deal with the possibility that the information won't be there because the user changed the session id stored in a cookie. NOTE: ViewBag and TempData are specific types of session data. You'll need something that lasts from displaying the edit page to the action that handles it. I believe TempData was designed for that purpose, but double check.

There can be other fairly complicated ways involving JSON serialization, and code obfuscation, but the best thing you can do is never trust data coming from a browser at face value.

Combining the hidden fields in session data with authenticated interaction, you can compare hidden field values with session values and if they don't match, flag the user account. Just be careful if you introduce JavaScript to do those modifications yourself.

Upvotes: 2

Matthew Haugen
Matthew Haugen

Reputation: 13286

The simple answer is to authorize access on edit, which is something you should be doing anyway.

Whenever you get a POST, run the same authorization code as you have to ensure the person is able to view the item. If they don't, 403 them.

It's true that HTTP isn't particularly "secure" in this way, because it's stateless. You can't assume that someone won't change things. Even worse, someone could open cURL or Fiddler and send a request directly to you, without even having to use dev tools.

It's best practice to always recheck authentication (something your framework will likely do for you anyway) and authorization for every request, be it a viewing or editing one.

Of course, if you're using roles-based authorization (like with the Authorize attribute, for instance), MVC will handle that all for you.

Upvotes: 1

Related Questions