Commits

Anonymous committed 7bbf8f3

XW-641 XWork ParameterInterceptors bypass (OGNL statement execution)

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

Comments (0)

Files changed (4)

src/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java

  */
 package com.opensymphony.xwork2.interceptor;
 
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.ActionInvocation;
 import com.opensymphony.xwork2.ValidationAware;
 import com.opensymphony.xwork2.conversion.impl.InstantiatingNullHandler;
 import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
 import com.opensymphony.xwork2.inject.Inject;
+import com.opensymphony.xwork2.util.ClearableValueStack;
 import com.opensymphony.xwork2.util.LocalizedTextUtil;
 import com.opensymphony.xwork2.util.TextParseUtil;
 import com.opensymphony.xwork2.util.ValueStack;
+import com.opensymphony.xwork2.util.ValueStackFactory;
 import com.opensymphony.xwork2.util.logging.Logger;
 import com.opensymphony.xwork2.util.logging.LoggerFactory;
 import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
 
-import java.util.*;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
 
 /**
  * <!-- START SNIPPET: description -->
     private String acceptedParamNames = "[\\p{Graph}&&[^,#:=]]*";
     private Pattern acceptedPattern = Pattern.compile(acceptedParamNames);
 
+    private ValueStackFactory valueStackFactory;
+
+    @Inject
+    public void setValueStackFactory(ValueStackFactory valueStackFactory) {
+        this.valueStackFactory = valueStackFactory;
+    }
+
     @Inject("devMode")
     public static void setDevMode(String mode) {
         devMode = "true".equals(mode);
             }
         }
 
+        ValueStack newStack = valueStackFactory.createValueStack(stack);
+        if (newStack instanceof ClearableValueStack) {
+            //if the stack's context can be cleared, do that to prevent OGNL
+            //from having access to objects in the stack, see XW-641
+            ((ClearableValueStack)newStack).clearContextValues();
+            Map<String, Object> context = newStack.getContext();
+            ReflectionContextState.setCreatingNullObjects(context, true);
+            ReflectionContextState.setDenyMethodExecution(context, true);
+            ReflectionContextState.setReportingConversionErrors(context, true);
+        }
+
         for (Map.Entry<String, Object> entry : acceptableParameters.entrySet()) {
             String name = entry.getKey();
             Object value = entry.getValue();
             try {
-                stack.setValue(name, value);
+                newStack.setValue(name, value);
             } catch (RuntimeException e) {
                 if (devMode) {
                     String developerNotification = LocalizedTextUtil.findText(ParametersInterceptor.class, "devmode.notification", ActionContext.getContext().getLocale(), "Developer Notification:\n{0}", new Object[]{
                 }
             }
         }
+
         addParametersToContext(ActionContext.getContext(), acceptableParameters);
     }
 

src/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java

 import com.opensymphony.xwork2.inject.Container;
 import com.opensymphony.xwork2.inject.Inject;
 import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
+import com.opensymphony.xwork2.util.ClearableValueStack;
 import com.opensymphony.xwork2.util.CompoundRoot;
 import com.opensymphony.xwork2.util.ValueStack;
 import com.opensymphony.xwork2.util.logging.Logger;
  * @author tm_jee
  * @version $Date$ $Id$
  */
-public class OgnlValueStack implements Serializable, ValueStack {
+public class OgnlValueStack implements Serializable, ValueStack, ClearableValueStack {
 
     private static final long serialVersionUID = 370737852934925530L;
 
     }
 
 
+    public void clearContextValues() {
+        //this is an OGNL ValueStack so the context will be an OgnlContext
+        //it would be better to make context of type OgnlContext
+       ((OgnlContext)context).getValues().clear();
+    }
+
+
 }

src/java/com/opensymphony/xwork2/util/ClearableValueStack.java

+/*
+ * $Id$
+ *
+ * Copyright 2003-2004 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.opensymphony.xwork2.util;
+
+/**
+ * ValueStacks implementing this interface provide a way to remove values from
+ * their contexts.
+ */
+public interface ClearableValueStack {
+    /**
+     * Remove all values from the context
+     */
+    void clearContextValues();
+}

src/test/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java

  */
 package com.opensymphony.xwork2.interceptor;
 
-import com.opensymphony.xwork2.*;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import ognl.PropertyAccessor;
+
+import com.opensymphony.xwork2.Action;
+import com.opensymphony.xwork2.ActionContext;
+import com.opensymphony.xwork2.ActionProxy;
+import com.opensymphony.xwork2.ModelDrivenAction;
+import com.opensymphony.xwork2.SimpleAction;
+import com.opensymphony.xwork2.TestBean;
+import com.opensymphony.xwork2.TextProvider;
+import com.opensymphony.xwork2.XWorkTestCase;
+import com.opensymphony.xwork2.config.entities.ActionConfig;
 import com.opensymphony.xwork2.config.providers.MockConfigurationProvider;
 import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider;
 import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
 import com.opensymphony.xwork2.mock.MockActionInvocation;
 import com.opensymphony.xwork2.ognl.OgnlValueStack;
+import com.opensymphony.xwork2.ognl.OgnlValueStackFactory;
 import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
 import com.opensymphony.xwork2.util.CompoundRoot;
 import com.opensymphony.xwork2.util.ValueStack;
-import ognl.PropertyAccessor;
-
-import java.util.*;
+import com.opensymphony.xwork2.util.ValueStackFactory;
 
 
 /**
         ParametersInterceptor pi = new ParametersInterceptor();
         container.inject(pi);
         final Map actual = new HashMap();
+        pi.setValueStackFactory(createValueStackFactory(actual));
         ValueStack stack = createStubValueStack(actual);
         final Map expected = new HashMap() {
             {
         params.put("blah", "This is blah");
         params.put("#session.foo", "Foo");
         params.put("\u0023session[\'user\']", "0wn3d");
+        params.put("\\u0023session[\'user\']", "0wn3d");
         params.put("\u0023session.user2", "0wn3d");
+        params.put("\\u0023session.user2", "0wn3d");
         params.put("('\u0023'%20%2b%20'session[\'user3\']')(unused)", "0wn3d");
         params.put("('\\u0023' + 'session[\\'user4\\']')(unused)", "0wn3d");
 
         proxy.execute();
         assertEquals("This is blah", ((SimpleAction) proxy.getAction()).getBlah());
     }
-    
+
     public void testParametersNotAccessPrivateVariables() throws Exception {
         Map<String, Object> params = new HashMap<String, Object>();
         params.put("protectedMap.foo", "This is blah");
         assertNotNull(action.getTheProtectedMap().get("boo"));
         assertNull(action.getTheProtectedMap().get("foo"));
     }
-    
+
     public void testParametersNotAccessProtectedMethods() throws Exception {
         Map<String, Object> params = new HashMap<String, Object>();
         params.put("theSemiProtectedMap.foo", "This is blah");
     }
 
     public void testNonexistentParametersGetLoggedInDevMode() throws Exception {
-        loadConfigurationProviders(new XmlConfigurationProvider("xwork-test-beans.xml"), 
+        loadConfigurationProviders(new XmlConfigurationProvider("xwork-test-beans.xml"),
                 new MockConfigurationProvider(Collections.singletonMap("devMode", "true")));
         Map<String, Object> params = new HashMap<String, Object>();
         params.put("not_a_property", "There is no action property named like this");
         extraContext.put(ActionContext.PARAMETERS, params);
         ParametersInterceptor.setDevMode("true");
 
+        ActionConfig config = configuration.getRuntimeConfiguration().getActionConfig("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME);
+        container.inject(config.getInterceptors().get(0).getInterceptor());
         ActionProxy proxy = actionProxyFactory.createActionProxy("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME, extraContext);
         proxy.execute();
         final String actionMessage = "" + ((SimpleAction) proxy.getAction()).getActionMessages().toArray()[0];
     }
 
     public void testNonexistentParametersAreIgnoredInProductionMode() throws Exception {
-        loadConfigurationProviders(new XmlConfigurationProvider("xwork-test-beans.xml"), 
+        loadConfigurationProviders(new XmlConfigurationProvider("xwork-test-beans.xml"),
                 new MockConfigurationProvider(Collections.singletonMap("devMode", "false")));
         Map<String, Object> params = new HashMap<String, Object>();
         params.put("not_a_property", "There is no action property named like this");
         HashMap<String, Object> extraContext = new HashMap<String, Object>();
         extraContext.put(ActionContext.PARAMETERS, params);
 
+        ActionConfig config = configuration.getRuntimeConfiguration().getActionConfig("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME);
+        container.inject(config.getInterceptors().get(0).getInterceptor());
         ActionProxy proxy = actionProxyFactory.createActionProxy("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME, extraContext);
         proxy.execute();
         assertTrue(((SimpleAction) proxy.getAction()).getActionMessages().isEmpty());
         ParametersInterceptor pi = new ParametersInterceptor();
         container.inject(pi);
         final Map<String, Object> actual = new LinkedHashMap<String, Object>();
+        pi.setValueStackFactory(createValueStackFactory(actual));
         ValueStack stack = createStubValueStack(actual);
 
         Map<String, Object> parameters = new HashMap<String, Object>();
         pi.setOrdered(true);
         container.inject(pi);
         final Map<String, Object> actual = new LinkedHashMap<String, Object>();
+        pi.setValueStackFactory(createValueStackFactory(actual));
         ValueStack stack = createStubValueStack(actual);
 
         Map<String, Object> parameters = new HashMap<String, Object>();
         pi.setOrdered(true);
         assertEquals(true, pi.isOrdered());
     }
-    
+
     public void testExcludedParametersAreIgnored() throws Exception {
         ParametersInterceptor pi = new ParametersInterceptor();
         container.inject(pi);
         pi.setExcludeParams("dojo\\..*");
         final Map actual = new HashMap();
+        pi.setValueStackFactory(createValueStackFactory(actual));
         ValueStack stack = createStubValueStack(actual);
         container.inject(stack);
 
         assertEquals(expected, actual);
     }
 
+    private ValueStackFactory createValueStackFactory(final Map<String, Object> context) {
+        OgnlValueStackFactory factory = new OgnlValueStackFactory() {
+            @Override
+            public ValueStack createValueStack(ValueStack stack) {
+                return createStubValueStack(context);
+            }
+        };
+        container.inject(factory);
+        return factory;
+    }
+
     private ValueStack createStubValueStack(final Map<String, Object> actual) {
         ValueStack stack = new OgnlValueStack(
                 container.getInstance(XWorkConverter.class),
         container.inject(stack);
         return stack;
     }
-    
+
     /*
     public void testIndexedParameters() throws Exception {
         Map params = new HashMap();
     protected void setUp() throws Exception {
         super.setUp();
         loadConfigurationProviders(new XmlConfigurationProvider("xwork-test-beans.xml"), new MockConfigurationProvider());
+
+        ActionConfig config = configuration.getRuntimeConfiguration().getActionConfig("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME);
+        container.inject(config.getInterceptors().get(0).getInterceptor());
     }
 }