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.
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.
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.
Why do TFVC users hate Git
When I start a new development engagement or TFS implementation with a client, I prefer to use and promote Git for the codebase. I have been faced with a lot of resistance from developers to use Git instead of Team Foundation Version Control (TFVC) or another centralized version control. In this post, I will have a list of the common complaints I hear when I propose using Git.
Concept of a local repository
The first thing a developer must do after creating the remote repository is cloning the remote repo. By cloning a remote repo, we are creating a local copy of the repository. For new Git users, this is a new concept. I don’t like to say that it is like “Get latest” for the first time because it is fundamentally different. Cloning a remote repo means you are getting a copy of the remote repo, which includes files, branches and history, on your computer. With that copy on your disk, you can modify files, commit changes, create a new branch, merge a branch, view history and compare files with their previous versions, all that while being offline. With TFVC, you can only modify files while you are offline.
Branching
Some centralized version control users often hate branching. It is their nightmare. They often only branch the code for releases or bug fixes in a release. In Git, branching is a first class citizen. It is the norm and not the exception. Centralized VC users get intimidated by that fact. Technically speaking, your local code is considered a branch of the remote code.
Terminology
The terminology to work with Git VC is different than TFVC. In TFVC, 90% of the time developers use two commands which are Get latest and Checkin. In Git, developers often deal with more commands on daily bases such has Fetch, Pull, Commit, Push, branch, merge, and Checkout. There are other commands in Git but they are used less often than the commands listed in the previous sentence. Some TFVC developers who are new Git find that overwhelming.
Command line
TFVC version control users interact with VC using GUI. Git is integrated with Visual Studio, Eclipse and other IDEs. But sometimes you must roll up your sleeves and use command line.
I can’t Pull the content of one folder
In TFVC, it is easy to get the latest code of a folder. In Git, you can only pull the changes for current branch from the remote repo. You can’t pull the changes of one folder.
Too many repositories
In TFVC, you can have one code repository per Team Project. We usually separate projects by adding them into separate folders. In Git, we create a repo per project. Keep in mind a project doesn’t necessary map to one .NET project or solution. I often hear complains that developers have to switch from one repo to another because they work on multiple projects. This forces them to either open multiple instances of the IDE and each instance is pointing to a repo or keep switching from one repo another.
Those are the top complaint I hear from client when switching to Git. Do you have other complaint? I would love to hear them.
About Lajak Technologies
A consulting firm in Ottawa, Ontario that provides services related to Microsoft technologies, Team Foundation Server, DevOps practices, security and more. Contact us today to help you in solving your complex software problems.
Microsoft Makes MSTest v2 Open Source
In case you have missed the news. Microsoft has opened sourced the MSTest v2 framework. MSTest v2 is the latest version of MsTest Framework. It is cross-platform which means it can be executed on Windows, Linux, and Mac. It can be used to test desktop, mobile, web, or library .NET applications. And the best part, it is distributed though Nuget as a package.
The advantages of this approach are we
- no long must wait for a new release or update for the .NET framework nor Visual Studio;
- we don’t have to wait for Microsoft to fix an issue. It is open source; we can submit a pull request on GitHub with the fix;
- Code and roadmap are transparent
For more info visit the following links
https://github.com/Microsoft/testfx-docs/blob/master/roadmap.md
https://github.com/Microsoft/testfx
About Lajak Technologies
A consulting firm in Ottawa, Ontario that provides services related to Microsoft technologies, Team Foundation Server, DevOps practices, security and more. Contact us today to help you solving your complex software problems.