Commits

davenewton  committed 50a6ba0

Fixing huge OGNL security holes with malicious parameter names
XW-641

git-svn-id: http://svn.opensymphony.com/svn/xwork/branches/xwork_1-0-3@1845e221344d-f017-0410-9bd5-d282ab1896d7

  • Participants
  • Parent commits a811ae1
  • Branches CONF-27294, xwork_1-0-3 1
    1. xwork_1-0-3_branch

Comments (0)

Files changed (2)

File src/java/com/opensymphony/xwork/interceptor/ParametersInterceptor.java

 public class ParametersInterceptor extends AroundInterceptor {
     //~ Methods ////////////////////////////////////////////////////////////////
 
+    protected boolean acceptableName(String name) {
+        if ((name.indexOf('=') != -1) || (name.indexOf(',') != -1) || (name.indexOf('#') != -1) || (name.indexOf(':') != -1) || (name.indexOf("\\u0023") != -1)) {
+            return false;
+        } else {
+            return true;
+        }
+    }
+
     protected void after(ActionInvocation dispatcher, String result) throws Exception {
     }
 
                     for (Iterator iterator = parameters.entrySet().iterator();
                             iterator.hasNext();) {
                         Map.Entry entry = (Map.Entry) iterator.next();
-                        stack.setValue(entry.getKey().toString(), entry.getValue());
+                        String name = entry.getKey().toString();
+
+                        if (acceptableName(name)) {
+                            stack.setValue(name, entry.getValue());
+                        }
                     }
                 }
             } finally {

File src/test/com/opensymphony/xwork/interceptor/ParametersInterceptorTest.java

 import com.opensymphony.xwork.TestBean;
 import com.opensymphony.xwork.config.ConfigurationManager;
 import com.opensymphony.xwork.config.providers.MockConfigurationProvider;
+import com.opensymphony.xwork.util.OgnlValueStack;
 
 import junit.framework.TestCase;
 
     public void testParameters() {
         Map params = new HashMap();
         params.put("blah", "This is blah");
+        params.put("#session.foo", "Foo");
+        params.put("\u0023session[\'user\']", "0wn3d");
+        params.put("\u0023session.user2", "0wn3d");
+        params.put("('\u0023'%20%2b%20'session[\'user3\']')(unused)", "0wn3d");
+        params.put("('\\u0023' + 'session[\\'user4\\']')(unused)", "0wn3d");
 
         HashMap extraContext = new HashMap();
         extraContext.put(ActionContext.PARAMETERS, params);
 
         try {
             ActionProxy proxy = ActionProxyFactory.getFactory().createActionProxy("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME, extraContext);
+            OgnlValueStack stack = proxy.getInvocation().getStack();
+            HashMap session = new HashMap();
+            stack.getContext().put("session", session);
             proxy.execute();
             assertEquals("This is blah", ((SimpleAction) proxy.getAction()).getBlah());
+            assertNull(session.get("foo"));
+            assertNull(session.get("user"));
+            assertNull(session.get("user2"));
+            assertNull(session.get("user3"));
+            assertNull(session.get("user4"));
         } catch (Exception e) {
             e.printStackTrace();
             fail();