WW-4805 Blocks ognl access to class members of Spring proxy

xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java

@@ -15,6 +15,7 @@
  */
 package com.opensymphony.xwork2.ognl;
 
+import com.opensymphony.xwork2.util.ProxyUtil;
 import com.opensymphony.xwork2.util.logging.Logger;
 import com.opensymphony.xwork2.util.logging.LoggerFactory;
 import ognl.DefaultMemberAccess;
@@ -93,6 +94,11 @@ public class SecurityMemberAccess extends DefaultMemberAccess {
             return false;
         }
 
+        if (ProxyUtil.isProxyMember(member, target)) {
+            LOG.warn("Access to proxy [#0] is blocked!", member);
+            return false;
+        }
+
         boolean allow = true;
         if (!checkStaticMethodAccess(member)) {
             if (LOG.isTraceEnabled()) {

xwork-core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java

@@ -0,0 +1,151 @@
+/*
+ * Copyright 2017 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;
+
+import org.apache.commons.lang3.reflect.ConstructorUtils;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.commons.lang3.reflect.MethodUtils;
+
+import java.lang.reflect.*;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * <code>ProxyUtil</code>
+ * <p>
+ * Various utility methods dealing with proxies
+ * </p>
+ *
+ */
+public class ProxyUtil {
+    private static final String SPRING_ADVISED_CLASS_NAME = "org.springframework.aop.framework.Advised";
+    private static final String SPRING_SPRINGPROXY_CLASS_NAME = "org.springframework.aop.SpringProxy";
+    private static final String SPRING_TARGETCLASSAWARE_CLASS_NAME = "org.springframework.aop.TargetClassAware";
+
+    private static final Map<Class<?>, Boolean> isProxyCache =
+            new ConcurrentHashMap<Class<?>, Boolean>(256);
+    private static final Map<Member, Boolean> isProxyMemberCache =
+            new ConcurrentHashMap<Member, Boolean>(256);
+
+    /**
+     * Check whether the given member is a proxy member of a proxy object.
+     * @param member the member to check
+     * @param object the object to check
+     */
+    public static boolean isProxyMember(Member member, Object object) {
+        if (!isProxy(object))
+            return false;
+
+        Boolean flag = isProxyMemberCache.get(member);
+        if (flag != null) {
+            return flag;
+        }
+
+        boolean isProxyMember = isSpringProxyMember(member);
+
+        isProxyMemberCache.put(member, isProxyMember);
+        return isProxyMember;
+    }
+
+    /**
+     * Check whether the given object is a proxy.
+     * @param object the object to check
+     */
+    private static boolean isProxy(Object object) {
+        Class<?> clazz = object.getClass();
+        Boolean flag = isProxyCache.get(clazz);
+        if (flag != null) {
+            return flag;
+        }
+
+        boolean isProxy = isSpringAopProxy(object);
+
+        isProxyCache.put(clazz, isProxy);
+        return isProxy;
+    }
+
+    /**
+     * Check whether the given object is a Spring proxy.
+     * @param object the object to check
+     */
+    private static boolean isSpringAopProxy(Object object) {
+        Class<?> clazz = object.getClass();
+        return (implementsInterface(clazz, SPRING_SPRINGPROXY_CLASS_NAME) && (Proxy.isProxyClass(clazz)
+                || isCglibProxyClass(clazz)));
+    }
+
+    /**
+     * Check whether the given member is a member of a spring proxy.
+     * @param member the member to check
+     */
+    private static boolean isSpringProxyMember(Member member) {
+        try {
+            Class<?> clazz = ClassLoaderUtil.loadClass(SPRING_ADVISED_CLASS_NAME, ProxyUtil.class);
+            if (hasMember(clazz, member))
+                return true;
+            clazz = ClassLoaderUtil.loadClass(SPRING_TARGETCLASSAWARE_CLASS_NAME, ProxyUtil.class);
+            if (hasMember(clazz, member))
+                return true;
+            clazz = ClassLoaderUtil.loadClass(SPRING_SPRINGPROXY_CLASS_NAME, ProxyUtil.class);
+            if (hasMember(clazz, member))
+                return true;
+        } catch (ClassNotFoundException ignored) {
+        }
+
+        return false;
+    }
+
+    /**
+     * Check whether the specified class is a CGLIB-generated class.
+     * @param clazz the class to check
+     */
+    private static boolean isCglibProxyClass(Class<?> clazz) {
+        return (clazz != null && clazz.getName().contains("$$"));
+    }
+
+    /**
+     * Check whether the given class implements an interface with a given class name.
+     * @param clazz the class to check
+     * @param ifaceClassName the interface class name to check
+     */
+    private static boolean implementsInterface(Class<?> clazz, String ifaceClassName) {
+        try {
+            Class<?> ifaceClass = ClassLoaderUtil.loadClass(ifaceClassName, ProxyUtil.class);
+            return ifaceClass.isAssignableFrom(clazz);
+        } catch (ClassNotFoundException e) {
+            return false;
+        }
+    }
+
+    /**
+     * Check whether the given class has a given member.
+     * @param clazz the class to check
+     * @param member the member to check
+     */
+    private static boolean hasMember(Class<?> clazz, Member member) {
+        if (member instanceof Method) {
+            return null != MethodUtils.getMatchingAccessibleMethod(clazz, member.getName(), ((Method) member).getParameterTypes());
+        }
+        if (member instanceof Field) {
+            return null != FieldUtils.getField(clazz, member.getName(), true);
+        }
+        if (member instanceof Constructor) {
+            return null != ConstructorUtils.getMatchingAccessibleConstructor(clazz, ((Constructor) member).getParameterTypes());
+        }
+
+        return false;
+    }
+}

xwork-core/src/test/java/com/opensymphony/xwork2/spring/ActionsFromSpringTest.java

@@ -5,8 +5,12 @@ package com.opensymphony.xwork2.spring;
 
 import com.opensymphony.xwork2.*;
 import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider;
+import org.apache.commons.lang3.reflect.MethodUtils;
 import org.springframework.context.ApplicationContext;
 
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * Test loading actions from the Spring Application Context.
  *
@@ -77,4 +81,26 @@ public class ActionsFromSpringTest extends XWorkTestCase {
     	        assertTrue(springResult.isInitialize());
     	        assertNotNull(springResult.getStringParameter());
     }
+
+    public void testProxiedActionIsNotAccessible() throws Exception {
+        // given
+        Map<String, Object> params = new HashMap<String, Object>();
+        params.put("exposeProxy", "true");
+        params.put("blah", "S2-047");
+
+        HashMap<String, Object> extraContext = new HashMap<String, Object>();
+        extraContext.put(ActionContext.PARAMETERS, params);
+
+        ActionProxy proxy = actionProxyFactory.createActionProxy(null,
+                "paramsAwareProxiedAction", null, extraContext);
+
+        // when
+        proxy.execute();
+        Object action = proxy.getAction();
+
+        //then
+        assertEquals("S2-047", ((SimpleAction) action).getBlah());
+        assertFalse("proxied action is accessible!",
+                (Boolean) MethodUtils.invokeMethod(action, "isExposeProxy"));
+    }
 }

xwork-core/src/test/resources/com/opensymphony/xwork2/spring/actionContext-xwork.xml

@@ -8,6 +8,11 @@
             <result-type name="springResult" class="springResult" />
         </result-types>
 
+        <interceptors>
+            <interceptor name="params"
+                         class="com.opensymphony.xwork2.interceptor.ParametersInterceptor"/>
+        </interceptors>
+
 		<action name="simpleAction" class="simple-action"/>
 
         <action name="dependencyAction" class="dependency-action"/>
@@ -19,5 +24,10 @@
         <action name="simpleActionSpringResult" class="simple-action">
 			<result name="error" type="springResult"/>
 		</action>
+
+        <action name="paramsAwareProxiedAction" class="proxied-action">
+            <interceptor-ref name="params" />
+            <result name="input" type="null"/>
+        </action>
     </package>
 </xwork>