1

Is it necessary to add [Required] annotation to either foreign key or virtual navigation property of the Entity framework model in order to prevent under posting attack?

I have the below models. CompanyId is the foreign key and Company model has CompanyName as the [Required] property. Minding that foreign key is a not nullable int Is there a risk of under posting attack, either if the foreign key or virtual navigation property of Employee model is not marked [Required]. If there is a risk, does add [Required] on either of them have any negative impact? Because I have enabled Lazy loading and adding [Required] on virtual navigation property necessitates initialising them before saving Employee model.

[Table("Employee")]
pulic partial class Employee : Model
{
  public override int Id {get;set;}
  public int CompanyId {get;set;}
  public string Name {get;set;}
  [Required]
  public virtual Company Company {get;set;}
}

[Table("Company")]
public partial class Company : Model
{
  public override int Id {get;set;}
  [Required]
  public string CompanyName {get;set;}
  public virtual Icollection<Employee> Employees {get;set;}
}

This Entity framework model has a corresponding view model called EmployeeViewModel.

public class EmployeeViewModel
{
 public int Id {get;set;}
 public string Name {get;set;}
 public int CompanyId {get;set;}
}
Useme Alehosaini
  • 2,998
  • 6
  • 18
  • 26
user6754
  • 71
  • 1
  • 8
  • Why are you giving your database models to the outside world? – nvoigt Apr 17 '20 at 07:04
  • I am not exposing database models to the world, I have viewmodels for that. But according to Security scan(Fortify on demand) hackers are able to manipulate the request to do some underposting and cause unexpected results. – user6754 Apr 17 '20 at 08:01
  • I don't understand... you say you don't give your database models to the public, but yet your scan shows you do. Is your scan wrong? why would your model even have [Required] attributes if you don't make them publicly accessible? – nvoigt Apr 17 '20 at 08:05
  • [Required] annotation can be used for both client and server side validation https://learn.microsoft.com/en-us/aspnet/mvc/overview/older-versions/getting-started-with-aspnet-mvc4/adding-validation-to-the-model – user6754 Apr 17 '20 at 14:09

1 Answers1

0

If your model is not exposed directly through your API then it will not specifically prevent an Under-Post, where this property is not included in a post unless your API also validates this constraint.

Adding [Required] to your navigation properties will however set the underlying ForeignKey column in the database as Not Nullable if you are using the Code First data schema management approach. The added protection of the underlying FK does mean that the data cannot be saved without a value for this field, which will prevent an Under-Post of a PUT (SQL INSERT) but not specifically a POST (SQL UPDATE).

You mentioned in comments that this model is not exposed directly through your API, but contradicted that with a report from a Security scan that indicates that it is, to accurately identify your specific issues would require more information about your actual API.

In MVC the, the controllers will validate that all members annotated with the RequiredAttribute are provided as the input for POST operations. If a property is not provided, then the call will fail. This is described in this post What does it mean for a property to be [Required] and nullable? with some background from Brad Wilson from the ASP.Net team responsible for this feature.

Is it necessary to add [Required] annotation to either foreign key or virtual navigation property of the Entity framework model in order to prevent under posting attack?

Only if the model is exposed to the API and only if you do not want to allow the client to POST without the required FK value or the related Entity. This is highly subjective to your implementation but if your default handling of an omitted navigation property value on a POST is to delete the existing data, or to clear the FK link, then yes, you would want to ensure that the FK or the navigation property was annotated as [Required].

  • It would be smarter to ignore the navigation properties that are not posted instead of setting the FKs to null.

If there is a risk, does add [Required] on either of them have any negative impact?

The negative impact is that it will require all POSTs on the type to include this information, even if the interface does not allow the selection or manipulation of the related data.

There is a Risk that this can actually create a paradoxical Over-Post scenario where the client holds older/stale data and sends that after the API has already received an updated version of the same data from another user or view that did allow access to the extra information.

To mitigate this risk, make sure that you have implemented concurrency tokens or some other solution to manage concurrency.


One alternative (potentially radical) solution to this is to not allow or not support POST of nested data at all and only operate on the top level of data that is provided. This would still allow you to specify [Required] on the FK properties but not allow or require the whole linked entity to be provided as well. This will require a greater degree of calls from the client to the API to manage the state of richly nested objects but it can also simplify the client and reduce the bytes going across the wire.

Chris Schaller
  • 13,704
  • 3
  • 43
  • 81