OWASP O2 Platform Blog

Fixing one of JPetStore’s AutoBinding Vulnerabilities (changing the purchase price)

JPetStore (test application included with the Spring Framework) has a couple Autobinding vulnerabilities. The objective of this post is to present a solution for fixing them that meets the following criteria:

  • minumal code changes on the controllers
  • no code changes on the views
  • dramatically reduction of the size of the binded object (the ModelAttribute, CommandClass),
  • matches the expected web-layer inputs (i.e. the attack surface)
  • supports UnitTesting
  • doesn’t rely on Spring Framework’s recommended solution (which is based on the SetAllowedFields and SetDisallowedFields)

Here are a number of posts that give the brackground to this problem and document the O2 Script I’m using to test the fix:

The vulnerability we are going to fix is the one that allows (amongst other things) the modification of the TotalPrice value of our shopping cart (which is not a good thing 🙂 )

This vulnerability is graphically described in this video:

… and it is fundamentally created by the fact that the setTotalPrice(…) method is exposed  in the Order class which is exposed in the OrderForm class (which is used as the command class for the OrderFormController).

So here is the fix.

1) The first step is to create a class that can be used on as the CommandClass. Trying to give it a name that made sense for what is was going to do, I called it OrderData and place it in a package called …jpetstore.domain.fromWeb (vs the original OrderForm class in the package … jpetstore.domain)

package org.springframework.samples.jpetstore.domain.fromWeb;
import org.springframework.samples.jpetstore.domain.Order;
import org.springframework.samples.jpetstore.web.spring.OrderForm;

public class OrderData {

private OrderForm orderForm;

public OrderData(OrderForm orderForm)
{
this.orderForm = orderForm;
}

public static OrderForm getOrderForm(Object command)
{
OrderData orderData = (OrderData)command;
return orderData.orderForm;
}
//bindable values
public OrderData getOrder()
{
return this;
}

....

}

This expects to receive an OrderForm object in its contructor which it will store in a private field.

The static getOrderForm() method is there to help the casting of the object given to us by the Spring Framework

2) On the FormBackingObject method return an OrderData object instead of an OrderForm object (which is vulnerable to the AutoBinding injection) and put on the Http Request attributes the original orderForm  object (this will allow the views to have access to the original orderForm , which they expec,t, and prevent us from needing to make code changes to the view (the fact that views should have access to massive domain objects is a topic for another time))

protected Object formBackingObject(HttpServletRequest request) throws ModelAndViewDefiningException {


UserSession userSession = (UserSession) request.getSession().getAttribute("userSession");
Cart cart = (Cart) request.getSession().getAttribute("sessionCart");
if (cart != null) {
// Re-read account from DB at team's request.
Account account = this.petStore.getAccount(userSession.getAccount().getUsername());
OrderForm orderForm = new OrderForm();
orderForm.getOrder().initOrder(account, cart);

request.setAttribute("orderForm", orderForm);     // AutoBinding fix
return new OrderData(orderForm);                // AutoBinding fix
}
else {
ModelAndView modelAndView = new ModelAndView("Error");
modelAndView.addObject("message", "An order could not be created because a cart could not be found.");
throw new ModelAndViewDefiningException(modelAndView);
}
}

3) modify all locations that originally received the OrderFrom object from SpringFramework in order to handle the fact that it is now an OrderData object (this uses the static method getOrderForm in order to make the code change short and sweet)

> onBindAndValidate

protected void onBindAndValidate(HttpServletRequest request, Object command, BindException errors, int page) {
if (page == 0 && request.getParameter("shippingAddressRequired") == null) {

//OrderForm orderForm = (OrderForm) command;            // AutoBinding fix
OrderForm orderForm = OrderData.getOrderForm(command);     // AutoBinding fix
orderForm.setShippingAddressRequired(false);
}
}

> on getTargetPage:

protected int getTargetPage(HttpServletRequest request, Object command, Errors errors, int currentPage) {
//OrderForm orderForm = (OrderForm) command;                    // AutoBinding fix
OrderForm orderForm = OrderData.getOrderForm(command);            // AutoBinding fix
if (currentPage == 0 && orderForm.isShippingAddressRequired()) {
return 1;
}
else {
return 2;
}
}

>processFinish

protected ModelAndView processFinish(
HttpServletRequest request, HttpServletResponse response, Object command, BindException errors) {
//OrderForm orderForm = (OrderForm) command;                    // AutoBinding fix
OrderForm orderForm = OrderData.getOrderForm(command);            // AutoBinding fix
this.petStore.insertOrder(orderForm.getOrder());
request.getSession().removeAttribute("sessionCart");
Map model = new HashMap();
model.put("order", orderForm.getOrder());
model.put("message", "Thank you, your order has been submitted.");
return new ModelAndView("ViewOrder", model);
}

4) finally add to the FormData object the getters and setters of the variables that we want to expose.

A good place to find out which variables to expose is to look at the values that are submited from the web pages, in this case:

Here is the full code of this class:

package org.springframework.samples.jpetstore.domain.fromWeb;
import org.springframework.samples.jpetstore.domain.Order;
import org.springframework.samples.jpetstore.web.spring.OrderForm;

public class OrderData {

private OrderForm orderForm;

public OrderData(OrderForm orderForm)
{
this.orderForm = orderForm;
}

public static OrderForm getOrderForm(Object command)
{
OrderData orderData = (OrderData)command;
return orderData.orderForm;
}

//bindable values
public void setShippingAddressRequired    (boolean shippingAddressRequired)
{
this.orderForm.setShippingAddressRequired(shippingAddressRequired);
}

public OrderData getOrder()
{
return this;
}

public void setCardType            (String cardType)             { this.orderForm.getOrder().setCardType            (cardType);}
public void setCreditCard        (String creditCard)         { this.orderForm.getOrder().setCreditCard        (creditCard); }
public void setExpiryDate        (String expiryDate)            { this.orderForm.getOrder().setExpiryDate        (expiryDate); }
public void setBillToFirstName    (String billToFirstName)     { this.orderForm.getOrder().setBillToFirstName    (billToFirstName); }
public void setBillToLastName    (String billToLastName)     { this.orderForm.getOrder().setBillToLastName    (billToLastName); }
public void setBillAddress1        (String billAddress1)         { this.orderForm.getOrder().setBillAddress1        (billAddress1); }
public void setBillAddress2        (String billAddress1)         { this.orderForm.getOrder().setBillAddress2        (billAddress1); }
public void setBillCity            (String billCity)             { this.orderForm.getOrder().setBillCity            (billCity); }
public void setBillState        (String billState)             { this.orderForm.getOrder().setBillState        (billState); }
public void setBillZip            (String billZip)            { this.orderForm.getOrder().setBillZip            (billZip); }
public void setBillCountry        (String billCountry)         { this.orderForm.getOrder().setBillCountry        (billCountry); }

public String getCardType        ()             { return this.orderForm.getOrder().getCardType         ();}
public String getCreditCard        ()             { return this.orderForm.getOrder().getCreditCard     ();}
public String getExpiryDate        ()             { return this.orderForm.getOrder().getExpiryDate     ();}
public String getBillToFirstName()             { return this.orderForm.getOrder().getBillToFirstName();}
public String getBillToLastName    ()             { return this.orderForm.getOrder().getBillToLastName ();}
public String getBillAddress1    ()             { return this.orderForm.getOrder().getBillAddress1     ();}
public String getBillAddress2    ()             { return this.orderForm.getOrder().getBillAddress2     ();}
public String getBillCity        ()             { return this.orderForm.getOrder().getBillCity         ();}
public String getBillState        ()             { return this.orderForm.getOrder().getBillState         ();}
public String getBillZip        ()             { return this.orderForm.getOrder().getBillZip         ();}
public String getBillCountry    ()             { return this.orderForm.getOrder().getBillCountry     ();}
}

The getOrder() method which returns the OrderData object, is a cool trick to trick the StringFramwork (which will come  looking for an  getOrder() method) and prevent us from creating multiple class files (which is where blind spots tend to occur). I like the fact that I can put all bindable variables in one location/file. There is a small side effect that now we can provide multiple order.* values, ie.  order.order.order.order.order.billCity (I wonder if there is DoS angle here)

And that’s it (this example is a bit simpler since JPetStore doesn’t use a very complex binding wiring (for example no config files to change),  but I will provide more examples later that handle the other variations.

Now, with this fix in place we can verify the fix by running the web automation script that previously allowed us to control the purchase price, and now it doesn’t 🙂

exploit variation #1

exploit variation #2

November 9, 2011 Posted by | Fixing Code, Java, JPetStore, Spring MVC | Leave a comment