Commits

Anonymous committed be6b756

XW-254: security problem fixed

git-svn-id: http://svn.opensymphony.com/svn/xwork/trunk@389e221344d-f017-0410-9bd5-d282ab1896d7

  • Participants
  • Parent commits 30f91cd

Comments (0)

Files changed (2)

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

  * {@link OgnlValueStack#setValue(java.lang.String, java.lang.Object) setValue} on
  * the value stack with the property name being the key in the map, and the value
  * being the associated value in the map.
- *
+ * <p/>
  * This interceptor sets up a few special conditions before setting the values on
  * the stack:
- *
+ * <p/>
  * <ul>
- *  <li>It turns on null object handling, meaning if the property "foo" is null and
- *      value is set on "foo.bar", then the foo object will be created as explained
- *      in {@link InstantiatingNullHandler}.</li>
- *  <li>It also turns off the ability to allow methods to be executed, which is done
- *      as a security protection. This includes both static and non-static methods,
- *      as explained in {@link XWorkMethodAccessor}.</li>
- *  <li>Turns on reporting of type conversion errors, which are otherwise not normally
- *      reported. It is important to report them here because this input is expected
-*       to be directly from the user.</li>
+ * <li>It turns on null object handling, meaning if the property "foo" is null and
+ * value is set on "foo.bar", then the foo object will be created as explained
+ * in {@link InstantiatingNullHandler}.</li>
+ * <li>It also turns off the ability to allow methods to be executed, which is done
+ * as a security protection. This includes both static and non-static methods,
+ * as explained in {@link XWorkMethodAccessor}.</li>
+ * <li>Turns on reporting of type conversion errors, which are otherwise not normally
+ * reported. It is important to report them here because this input is expected
+ * to be directly from the user.</li>
  * </ul>
  *
  * @author Patrick Lightbody
                     final OgnlValueStack stack = ActionContext.getContext().getValueStack();
 
                     for (Iterator iterator = parameters.entrySet().iterator();
-                            iterator.hasNext();) {
+                         iterator.hasNext();) {
                         Map.Entry entry = (Map.Entry) iterator.next();
-                        stack.setValue(entry.getKey().toString(), entry.getValue());
+                        String name = entry.getKey().toString();
+                        if (acceptableName(name)) {
+                            Object value = entry.getValue();
+                            stack.setValue(name, value);
+                        }
                     }
                 }
             } finally {
             }
         }
     }
+
+    protected boolean acceptableName(String name) {
+        if (name.indexOf('=') != -1 || name.indexOf(',') != -1 || name.indexOf('#') != -1) {
+            return false;
+        } else {
+            return true;
+        }
+    }
 }

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

  */
 package com.opensymphony.xwork.interceptor;
 
-import com.opensymphony.xwork.Action;
-import com.opensymphony.xwork.ActionContext;
-import com.opensymphony.xwork.ActionProxy;
-import com.opensymphony.xwork.ActionProxyFactory;
-import com.opensymphony.xwork.ModelDrivenAction;
-import com.opensymphony.xwork.SimpleAction;
-import com.opensymphony.xwork.TestBean;
+import com.opensymphony.xwork.*;
 import com.opensymphony.xwork.config.ConfigurationManager;
 import com.opensymphony.xwork.config.providers.MockConfigurationProvider;
-
+import com.opensymphony.xwork.util.OgnlValueStack;
 import junit.framework.TestCase;
 
 import java.util.HashMap;
 
 /**
  * ParametersInterceptorTest
- *
+ * <p/>
  * Created : Jan 15, 2003 8:49:15 PM
  *
  * @author Jason Carreira
         }
     }
 
+    public void testParametersDoesNotAffectSession() {
+        Map params = new HashMap();
+        params.put("blah", "This is blah");
+        params.put("#session.foo", "Foo");
+
+        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"));
+        } catch (Exception e) {
+            e.printStackTrace();
+            fail();
+        }
+    }
+
     public void testParameters() {
         Map params = new HashMap();
         params.put("blah", "This is blah");