Struts 2 insecure direct object reference

There is a type of vulnerability which seems peculiar to Struts 2/WebWork applications and therefore may not be widely known. (It may exist in other frameworks as well, but I haven’t personally used any that have it.) The vulnerability is not part of Struts 2, but it enables it in the same sense that a SQL database enables SQL injection.

The attack probably falls under insecure direct object reference, although it can bear similarities to OGNL injection. To demonstrate it, look at the following code:

class BadAction extends ActionSupport implements Preparable, ModelDriven<Address> {
  private Address address;

  public void prepare() {
    address = (Address) ActionContext.getContext().getSession().get("address");
  }

  public String execute() {
    return SUCCESS;
  }

  public Address getModel() {
    return address;
  }
}

// Address class contains street, city, state, zip, etc.

The intent is that the Address class may have many fields, so we use the ModelDriven interface instead of duplicating all the getters and setters. The problem in this case is that the object in the session is being directly modified before validation. Note that the method calls in order are prepare, getters/setters, validate, and execute. (Of course, validate may use a config file instead of being a method on the action.) Validation occurs after the session is modified in this case, so even if validation fails, the session is poisoned.

The attack does not require the action to implement ModelDriven. The main point is that the action returns a sensitive object which the attacker can modify. Remember, attackers can call all public getters and setters on an action, all public getters and setters on objects returned by getters, and so on. The action above could be easily rewritten to avoid this problem, but in more complex scenarios, a way to avoid such attacks is to store cloned objects in the action and return those clones from the getters.

Now let’s say we learned from BadAction, but now we need to validate the new address based on the old one. We decide to use a validation XML file instead of a validate method, so we need a public getter for the old address. Here is our new code:

class BadAction2 extends ActionSupport implements Preparable, ModelDriven<Address> {
  private Address oldAddress;
  private Address address = new Address();

  public void prepare() {
    oldAddress = ((Address) ActionContext.getContext().getSession().get("address")).clone();
  }

  public String execute() {
    ActionContext.getContext().getSession().put("address", address);
    return SUCCESS;
  }

  public Address getModel() {
    return address;
  }

  public Address getOldAddress() {
    return oldAddress;
  }
}

// Address class contains street, city, state, zip, etc.

The old address from the session is cloned, so the session can’t be modified without the address passing validation. However, nothing prevents the user from modifying the old address, simply by passing request parameters such as oldAddress.city. Since this happens before validation, the attacker could place values in the old address that will let the new address pass that part of the validation. In this case, the best thing to do is move the validation to a validate method and get rid of the getter for the old address.

The one thing you should keep in mind regarding this attack is that the attacker can modify anything the action returns from a getter before validation.

Post a Comment

Your email is never shared. Required fields are marked *

*
*