Is ModelDriven's interface a security hack in struts2?

Question

background: I encoded the ActionSupport struts2 class with ModelDriven. This is a hibernate / spring web application using OSIV and attached objects in a view (JSP).

Today I received this letter from an architect “punishing” me for placing an object that had a link to an attached object on struts2 rack through ModelDriven<E> . Is he right or what? Obviously, this is a serious thing that I am doing, but I do not follow what he says, and I really do not want to accept his offer and visit him at my table after that. Oh boy. Time to change your career.

--- from the architect ---

Billy, as we said earlier, you still make the same mistakes in your code over and over again. This is the fourth time you have made this mistake, and I am concerned about the quality of your work. It is one thing to do this once or even twice, but after the fourth time, I wonder if you can understand what I am saying. The following will set this out for you. If you do not receive it after reading this letter, go to my desk and we will consider it. This needs to be stopped immediately, and I want all your code reorganized by the end of the day, fixing this error. If any code bleeds into production, we will have a serious security problem at our fingertips. Also note that I am copying Dave to this so that a proper reprimand can be issued. I will also recommend Dave that you move from a Level III developer to Level II. Read the following and please study it and reformat all your code as I pointed out.

About binding objects:

When the Struts2 action class is marked with the ModelDriven interface, the model will be bound to form elements in the HTML page. For example, if the HTML form has a field called userName, and the action class is defined as:

public class UserAction extends ActionSupport implements ModelDriven

And the UserModel is a POJO as follows:

 public class UserModel { private String userName; public String getUserName() { return userName; } public void setUserName(String userName) { this.userName = userName; } } 

When the form is submitted while the action contains an instance of UserModel, struts2 will associate the userName field with UserModel.userName, automatically filling in the value.

However, this simplicity is expensive for malicious users. If an object is declared as ModelDriven, the end user viewing the user who has access to the model graph through model installers. Take this example, for example:

public class UserAction extends ActionSupport implements ModelDriven

and...

 public class UserModel { private String userName; private UserEntity userEntity; public String getUserName() { return userName; } public void setUserName(String userName) { this.userName = userName; } pubic UserEntity getUserEntity() { return userEntity; } } 

and...

 @Entity public class UserEntity { private String password; public String getPassword() { return password; } public void setPassword(String password) { this.password = password; } } 

assuming an OSIV pattern is being used, and a UserEntity object is attached.

A damned user with little knowledge or time on hand can:

 /myform?userName=billy&userEntity.password=newpassword 

Assuming that the Entity is saved at the end of the session, the above results change the Billy password.

Point, graph objects available!

When using ModelDriven and using an alternative is a terrible approach, you have to identify fine-grained models that fit in the cost price and then copy from the model to the target before sending a response and resolving the transaction.

Answer 1

Your rights architect, placing objects with access to confidential information in ValueStack poses a potential security risk. A malicious user can indeed reset the password with the aforementioned attack.

BUT:

Since he is an architect, he had to develop ways to properly validate / limit input parameters. Using ParamsInterceptor in Struts2, it’s quite simple to allow the passing of certain parameters to the action. Thus, it is not your job that sucks, it is your system architecture. Developers should be able to focus on implementing business logic. Infrastructure must be provided by the architect.

Greetings

w

Answer 2

ModelDriven interceptor blinded

Yes The model interface can be the source of a security problem, if you do not process incoming parameters, you will need security holes.

You need to use a parameter hook.

In struts.xml, change your params interceptor as:

 <interceptor-ref name="params"> <param name="excludeParams">\w+((\.\w+)|(\[\d+\])|(\(\d+\))|(\['\w+'\])|(\('\w+'\)))*</param> </interceptor-ref> 

Then in your action we implement ParameterNameAware and write acceptableParameterName .

 public class sample implements ParameterNameAware(){ public boolean acceptableParameterName(String parameterName) { if (("username".equals(parameterName) || "firstname".equals(parameterName) || "lastname".equals(parameterName)) return true; else return false; } } 

The above is important if your User pojo has many other properties and only some of them must be obtained from the user.

If you use a lot of ModelDriven actions, you can make it general.

Create a basic action that extends ParameterNameAware . Then try to develop a general approach to have a list of your actions and valid parameters:

We used spring to read the list of actions and their acceptable parameters. In spring xml we added:

 <util:properties id="actionsValidParameters" location="classpath:/configs/actions-valid-parameters.properties" /> 

actions-valid-parameters.properties :

 save-user=username,description,firstname,lastname save-address=zipcode,city,detail,detail.addLine1,detail.addLine2,detail.no 

Hint. If there is a Detail object in the address object, and you want to fill in some properties in the Detail object, make sure that you include the "detail" in the list above.

Action is like

 public class BaseActionSupport extends ActionSupport implements ParameterNameAware { @Resource(name = "actionsValidParameters") public Properties actionsValidParameters; @Override public boolean acceptableParameterName(String parameterName) { String actionName = ActionContext.getContext().getName(); String validParams = (String) actionsValidParameters.get(actionName); //If the action is not defined in the list, it is assumed that the action can accept all parameters. You can return false so if the action is not in the list no parameter is accepeted. It is up to you! if(StringUtils.isBlank(validParams)) return true; // Search all the list of parameters. //You can split the validParams to array and search array. Pattern pattern = Pattern.compile("(?<=^|,)" + parameterName + "(?=,|$)"); Matcher matcher = pattern.matcher(validParams); boolean accepeted = matcher.find(); LOG.debug( "The {} parameter is {} in action {}, Position (excluding the action name) {} , {} , mathced {} ", parameterName, accepeted, actionName, matcher.start(), matcher.end(), matcher.group()); return accepeted; } } 

Now open your actions as

  public class UserAction extends BaseActionSupport implements ModelDriven<User>{ }