Spring seems to bind all form fields : security?

[Sebastian]

Hi all,

i wondered how secure the automatic data binding is, when using domain objects directly as command objects (a very usefull feature together with hibernate).

If i have a jsp-form which uses <bind>-tags to bind certain fields of the command object to form fields, then all the command objects properties will be set automatically upon form submit. Thats fine so far.

But what happens if a user manipulates the form submit, e.g. by adding additional form fields in the html code (which try to set properties on the command object, which were not bound with the <bind>-tag)?

As i could not find definitive information on this, i made an experiment with the petclinic sample from the current Spring 1.1.1:

The ownerFrom.jsp for adding a new owner includes the firstName.jsp for the respective form field.

firstName.jsp (with binding):

<%@ include file="/WEB-INF/jsp/includes.jsp" %>

<B>First Name&#58;</B>
<spring&#58;bind path="command.firstName">
  <FONT color="red">
    <B><c&#58;out value="$&#123;status.errorMessage&#125;"/></B>
  </FONT>
  <BR><INPUT type="text" maxlength="30" size="30" name="firstName" value="<c&#58;out value="$&#123;status.value&#125;"/>" >
</spring&#58;bind>
<P>

As expected the first name is set in the command object upon submission.

If i remove the <bind>-tags the jsp looks like this:

firstName.jsp (without binding):

<%@ include file="/WEB-INF/jsp/includes.jsp" %>

<B>First Name&#58;</B>

  <FONT color="red">
    <B><c&#58;out value="$&#123;status.errorMessage&#125;"/></B>
  </FONT>
  <BR><INPUT type="text" maxlength="30" size="30" name="firstName" value="<c&#58;out value="$&#123;status.value&#125;"/>" >

<P>

When submitting this form the firstName is still set on the command object. The only (expected) change i noticed is that on invalid submits the current value of firstName is not kept in the form.


So, using this example, my question is: Shouldn't Spring check that only those form fields are set on the command object, which were bound with the bind-tag?

If i use my domain objects as command object, then this seems to me like a big security hole, as any user could potentially change any property that can be reached by the command objects associations.

The DataBinder class has a method setAllowedFields() which could be used, but shouldn't Spring do this automatically?

I am looking forward to your answers, am i wrong with my assumptions?

Thanks a lot!
Sebastian

BTW: setting sessionForm=true did not change this... just an idea...

[davison]

So, using this example, my question is: Shouldn't Spring check that only those form fields are set on the command object, which were bound with the bind-tag?

It's not a requirement to use a bind tag (or equivalent for non-JSP views) in order to submit a form POST, so this wouldn't be a complete solution.

If i use my domain objects as command object, then this seems to me like a big security hole, as any user could potentially change any property that can be reached by the command objects associations.

Without having tried it, it looks likely yes. Setting a nested path in carefully crafted HTTP packets could have severe consequences.

The DataBinder class has a method setAllowedFields() which could be used, but shouldn't Spring do this automatically?

Certainly if your command objects have properties or associations that should be immutable by a form POST, then use this method to set the names of the permitted fields and Spring will ignore any others that are posted. I think however that the logging could be improved in this area and that POSTed fields not meeting the isAllowed() criteria should be noted in the log. It may also be worth logging a WARNing message when the DataBinder is created if the allowedFields array is null.

The safest way to handle it would be to make allowedFields mandatory and throw an exception if null, but this would probably break the vast majority of existing applications.

Maybe raise a JIRA issue about the logging..?

[cmgharris]

I haven't used setAllowedFields up to now, but it looks like this is important - I need to revisit in some places.
In some cases there will be a large number of fields and only a few are sensitive - would it make sense to also have a setDisallowedFields method as an alternative?

[davison]

would it make sense to also have a setDisallowedFields method as an alternative?

I think it would lead to a potentially dangerous false sense of security. If you disallow fields and then later refactor your command object to include additional fields that should also be disallowed, you have to remember to update the list. By setting allowedFields only, you don't since if you refactor the other way and add fields that should be allowed, then your code fails in testing which is good. However it does mean that sensible logging of the workflow needs to occur and currently that won't happen.

I'll add a JIRA issue for this and take a look at it.

Regards,

[dwsmith75]

When using setAllowedFields()

How do you it set collection properties e.g.

class Person{
 List address
}

class Address{
  String addressLine1
}

*xxx and xxx* patterns used in isAllowed() seems pretty weak.

e.g.
*addressLine1 would allow binding of anything regardless of it not being in Address class

Maybe the default implementation should be able to handle collection properties accepting something like: person.address[*].addressLine1

Should be a breeze to implement

[paulf]

Hello,

Can you tell me what the latest thinking on this thread is. I have to build a secure application and realised a clever user could add a another field to a form and modify a field in my command object that is not expected to be modified.

I searched for "setAllowed" in the reference documentation and it is not mentioned anywhere so I'm wondering if Spring is doing something in the background to ensure security?

[Fokko Breden]

You must use setAllowedFields inside initBinder of your Controller. Example:
...
private static final String[] ALLOWED_PROPERTIES = {"password", "name", "userInfo.phone", "userInfo.email"};
...
public EditUserController() {
setCommandName("user");
...
}
...
protected void initBinder(HttpServletRequest request, ServletRequestDataBinder binder) {
super.initBinder(request, binder);
binder.setAllowedFields(ALLOWED_PROPERTIES);
}
...

http://www.springframework.org/docs/...ataBinder.html