Archive

Archive for March, 2018

Preventing over posting or mass assignments vulnerability in ASP.Net MVC


In the previous post, I showed how the over posting or mass assignments vulnerability works in ASP.NET MVC. In this post, I will show how to prevent this vulnerability.

Never use your database object as a parameter for in an action method

Developers may get tempted to use the object they pass to the database. Here is an example

[HttpPost]
[ValidateAntiForgeryToken]
public async Task IActionResult  Create(User user)
{
    if (ModelState.IsValid)
    {
	_context.Add(userl);
	await _context.SaveChangesAsync();
	return RedirectToAction(nameof(Index));
    }
    return View(user);
} 

This is dangerous. With the above code, you will end up hurting the integrity of the data in your database. You should never, in my opinion, expose your database object to the outside world.

Recently, when scaffolding the Controller, the code generator adds a comment to warn developers about the overposting attack I don’t recall seeing it in ASP.NET MVC 4. It may have been added in ASP.NET MVC 5 or ASP.NET Core. The scaffolder also using the BindAttribute when generating the code which will discuss later on in this post.

Use different mode/view model for the Get and Post action methods

I like to use a model or view model which contains a subset of the properties that are in the database model. I use a different view model for the Get and Post request depending on the fields I want to show on the screen or capture from the user. I added new properties to the User model. I also created two view models. One for creating and updating a user (you can different VM for create and update if you want) and one for viewing a user.

public class CreateUserViewModel
    {
	public string Firstname { get; set; }
	public string Lastname { get; set; }
	public string Role { get; set; }
    }
public class UserViewModel
    {
	public int Id { get; set; }
	public string Firstname { get; set; }
	public string Lastname { get; set; }
	public string Role { get; set; }
	public DateTime LastUpdatedDate { get; set; }
    }

I created a branch in the code repository to demonstrate that https://github.com/lajak/SecureAspMvc/tree/OverPostingPreventionSeparateModel

With using the above code, we guarantee that the binder will only limited to the properties we have in the CreateUserViewModel for post request while for get request we have exposing more properties. Btw, creating a separate view model for get is not really necessary if your action method returns a view because your action method is returning the html which will be rendered by the browser. It becomes more crucial to have separate view model for read if your action returns data in json format.

In the controller, we need to map the values from CreateserViewModel to the DB user object.

  public async Task IActionResult Create(CreateUserViewModel user)
{
	var dbUser = new User();
	if (!await TryUpdateModelAsync(dbUser))
	    return NotFound();

	if (ModelState.IsValid)
	{
	    dbUser.LastUpdatedDate = DateTime.Now;
	    dbUser.CreatedDate = dbUser.LastUpdatedDate;
	    _context.Add(dbUser);
	    await _context.SaveChangesAsync();
	    return RedirectToAction(nameof(Index));
	}
	return View(dbUser);
}

Decorate properties with attributes

If you want to use one model, you can decorate the properties you don’t want the binder to bind with the [BindNever] attribute.

public class User
    {
	public int Id { get; set; }
	public string Firstname { get; set; }
	public string Lastname { get; set; }
	public string Role { get; set; }
	[BindNever]
	public DateTime CreatedDate { get; set; }
	[BindNever]
	public DateTime LastUpdatedDate { get; set; }
    }

Properties that are decorated with the BindNever attribute will not be assigned the value that is posted to the server. In other words, the binder will ignore it. There are few potential issues with using BindNever attribute.

  • High maintenance: developers need to remember adding BindNever property when adding new attribute if required and developers need to make sure that BindNever is not removed by mistake from one of the attribute
  • TryUpdateModelAsync: if you decide to have let’s say an admin page where you want administrator to be able to modify those attributes and you create a view model. The TryUpdateModelAsync will still ignore those properties that are decorated with BindNever and you need to assign those properties manually
  • I have read few reported issues that the Binder doesn’t respect BindNever or BindRequired when using the FromBody attribute

Use BindAttribute

One other option is to use the Bind attribute on the action method as such

public async Task IActionResult> Create([Bind("Firstname", "Lastname", "Role")] User user)

With this approach, you need to add the Bind attribute on every action method you want to prevent the over posting attack. You may have to revisit all your action methods every time you add or remove a property to the model.

Conclusion

As we have seen, there are few ways to protect our application from the overposting attack. I prefer to have view models. I feel it is the most flexible option as I can have a different view model per view or action method if required.

 

Advertisements
Categories: Security, Uncategorized Tags: ,

Over posting or mass assignments vulnerability in ASP.Net MVC


In Visual Studio, ASP.NET MVC comes with a great productivity feature called scaffolding. It is a code generation feature that generates a controller, view and the database calls if you are using Entity Framework to communicate to the database. If developers are not careful, they may expose the application to the over posting or mass assignment security vulnerability.

To understand the vulnerability, you need to understand how the Model Binder works in ASP.NET MVC.

The source code for this article is at https://github.com/lajak/SecureAspMvc/tree/OverPosting.

Let’s start with creating some artifacts

Create a new ASP.NET MVC project

Create a new model

        public int Id { get; set; }
        public string Firstname { get; set; }
        public string Lastname { get; set; }
        public string Role { get; set; }

Create UserController

  • Right click on the Controllers folder
  • Click on Add and select Controller
  • On the dialog, select MVC controller with read/write (lets keep it simple and not over complicate the example)
  • In the Create Action method, right click on the View function and click on Add View menu item

  • Fill the dialog as shown below.

On clicking the Add button, Visual Studio will generate a for creating a user. The view will be named create.cshtml and will be located in the Views\Users folder

Run the application and browse to http://localhost/user/create

  • Go back to the UserController
  • On Create Action method that is decorated with the HttpPost attribute, change the parameter type to UserModel and the parameter name to model as shown on the picture below. Also place a breakpoint at the return statement. We don’t have a view for the Index page. But we don’t need for this post.

Run the website in debug mode

Browse to http://localhost/user/create

Fill the form

Click the Create button

The code should break at the breakpoint

Hover over the model parameter and expand the properties

Notice that the Binder mapped the values that you entered in the form to the properties in the object

Now, stop the debugger and go to the Create.cshtml page. Let’s assume that the requirement was to hide the id field. The developer goes and delete the highlighted code

Run the code again and browse to the create method

Notice that the Id field is no longer showing up

If we run the code and populate the fields, the Id property won’t be populated.

As someone who wants to exploit this vulnerability, I can inject that id parameter into the request.

It is easier to use a tool like Fiddler or Burp to intercept the request and add the extra attributes to the payload but we can use the F12 developers tools as well

Click F12

Right click on

<form action="/user/Create" method="post">

Edit as Html and add the following line

<input name="Id" class="form-control" id="Id" type="number" value="111111111" data-val-required="The Id field is required." data-val="true">

Click outside the form attribute

Notice that we can see the field

Fill the other fields and hit create

Notice that the Binder binds the value of the id field to the Id property

Conclusion

We showed a trivial example for the over posting vulnerability. A more realistic example would be showing more or less fields based on the user role. If the developer only uses an if statement in the cshtml code to show fields based on the user role, the developer might be exposing the application to the over posting vulnerability. In the next post, I will show how to protect your application from this vulnerability.

Categories: Security, Uncategorized Tags: ,

Security tips: Record level authorization in your .NET application


Record level authorization is to verify that users are permitted to access the records they want to view, update or delete. I should say that not all applications I worked on or reviewed had that requirement. The requirement for some of the applications I had worked with was to implement role based authorization where users with certain roles can access certain areas of the application while other applications I had worked on required record level authorization.

Example of a record based authorization would be a bank account, only the account holders should be able to access the account and manage the account. Another example, in an insurance system, the policy holder should be the only one who can submit a claim and accept a settlement.

How to test if the application is vulnerable?

If you are a tester and doing white box testing, there isn’t a recipe that tells you that the application is 100% coded correctly. Let’s show a simple example using ASP.NET MVC on how you can test your application. Let’s say we have a URL such as http://hostname/users/Details/1. Given that the default route is {controller}/{action}/{id}, we can tell that the controller name is UsersController, the action method name is Details and the record Id is 1.

If the application is supposed to implement record level authorization, the system shouldn’t allow you to access record id 100 if you don’t have access to it. This can easily be tested by replace 1 with 100 in the url (e.g. http://hostname/users/Details/100)

How about POST requests? You can test POST request using the edit action method (i.e. http://hostname/users/Edit/1). The record id is usually passed in the payload. You can use the F12 developer tools to see the parameters passed to the server.

You can try to modify the id in the DOM if you don’t want to use other tools. It is easier to use a tool like Burp or Fiddler to intercept the request and modify parameters.

If you were able to modify a record that your user shouldn’t have access to, the application should fail the test case.

Conclusion

Record level authorization is a must for some systems but it is not necessary a requirement for other systems. You need to clarify the requirement with client to make sure your test cases are based on the requirements. The vulnerability is catastrophic for high profile applications if a hacker can exploit it. As a tester, your test cases should cover record level authorization related requirements.

Categories: Security Tags: , ,