Addressing Mass Assignment vulnerabilities with @NoBind annotation for domain objects [SPR-13835]

[spring-projects-issues]

Anton Abashkin opened SPR-13835 and commented

Most modern frameworks provide a convenience mechanism for binding form data to a domain object. Spring MVC is no exception.

While such a function is certainly indispensable, it exposes the application to the risk of a mass assignment vulnerability http://www.hpenterprisesecurity.com/vulncat/en/vulncat/java/mass_assignment_sensitive_field_exposure.html

A very serious vulnerability, this is actually how GitHub got hacked back in 2012 http://www.infoq.com/news/2012/03/GitHub-Compromised

Currently, Spring MVC does address this issue by providing two methods:

http://docs.spring.io/autorepo/docs/spring-framework/3.1.x/javadoc-api/org/springframework/validation/DataBinder.html#setAllowedFields%28java.lang.String...%29

http://docs.spring.io/autorepo/docs/spring-framework/3.1.x/javadoc-api/org/springframework/validation/DataBinder.html#setDisallowedFields%28java.lang.String...%29

While obviously a step in the right direction, this approach is not always optimal, the reason being the developer must set this in each controller using @InitBinder. This duplicates effort and is error prone.

One alternative for the developer is to create Data Transfer Objects (DTOs). While this does indeed provide protection, as well as other architectural benefits, it is not practiced by many developers.

I believe the optimal way to address this is to do something similar to what the the Play Framework does by providing a @NoBind annotation: https://www.playframework.com/documentation/1.4.x/controllers#nobinding

public class User extends Model {

  @NoBinding("profile") public boolean isAdmin;

  @As("dd, MM yyyy") Date birthDate;

  public String name;
}

public static void editProfile(@As("profile") User user) {
  // ...
}

Expressing these constraints directly in the domain object using annotations seems optimal from both a development and security auditing point of view

Thoughts?

---

Issue Links:

• Provide support for configuring the bindable properties of a form-backing object using field-level annotations [SPR-7747] #12403 Provide support for configuring the bindable properties of a form-backing object using field-level annotations
• Ability to suppress "rejectedValue" in error responses [SPR-14771] #19337 Ability to suppress "rejectedValue" in error responses

2 votes, 5 watchers

[spring-projects-issues]

Anton Abashkin commented

More info: https://cwe.mitre.org/data/definitions/915.html

[spring-projects-issues]

Juergen Hoeller commented

We have been considering such a mechanism before - either whitelisting bindable fields or blacklisting non-binding fields through an annotation as you suggest - but never got around to work out the details. I'll take your request as an opportunity to revisit this topic for 4.3...

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Technically, we would implement such an auto-disallow for specifically annotated fields at the DataBinder level, which is also where the present "allowedFields" / "disallowedFields" mechanism resides. We can easily make the annotation type configurable and simply provide a convenience @NoBind (whatever the name) annotation that Spring MVC configures by default.

The lower-level BeanWrapper has a different mission and should remain capable of binding to any property.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Unfortunately, this turns out out to be non-trivial to implement at the DataBinder level, in particular with respect to the traversal of nested paths. We'll have to revisit the BeanWrapper versus DataBinder relatonship in a more comprehensive fashion, so I'm afraid this is rather a 5.0 topic in general... and we're very close to our 4.3 deadline already.

Juergen

[rstoyanchev]

For 6.1 we've added enhanced support for constructor binding in DataBinder including support for nested constructors (#20806). It is also possible to set a declarativeBinding flag (#30948) through an @InitBinder method locally on a controller, or globally in a controller advice, or via ConfigurableWebBindingInitializer. The declarative mode ensures that data binding is limited to constructor parameters, and properties explicitly allowed through the allowedFields property on DataBinder.

This provides an option to use data binding such that the request parameters used are determined by the Object structure, and not the other way around.

I am closing this as superseded for now, but we are open to further feedback.