svn commit: r1156279 - /struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java

[mcucchiara]

Author: mcucchiara
Date: Wed Aug 10 17:59:54 2011
New Revision: 1156279

URL: http://svn.apache.org/viewvc?rev=1156279&view=rev
Log:
WW-3668 - Vulnerability: User input is evaluated as an OGNL expression when 
there's a conversion error (a demonstrative patch).
Modified:
    
struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java

Modified: 
struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java
URL: 
http://svn.apache.org/viewvc/struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java?rev=1156279&r1=1156278&r2=1156279&view=diff
==============================================================================
--- 
struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java
 (original)
+++ 
struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java
 Wed Aug 10 17:59:54 2011
@@ -35,13 +35,13 @@ public class ConversionErrorInterceptorT
     protected ActionContext context;
     protected ActionInvocation invocation;
     protected ConversionErrorInterceptor interceptor;
-    protected Map conversionErrors;
+    protected Map<String,Object> conversionErrors;
     protected Mock mockInvocation;
     protected ValueStack stack;
 
 
     public void testFieldErrorAdded() throws Exception {
-        conversionErrors.put("foo", new Long(123));
+        conversionErrors.put("foo", 123L);
 
         SimpleAction action = new SimpleAction();
         mockInvocation.expectAndReturn("getAction", action);
@@ -69,19 +69,12 @@ public class ConversionErrorInterceptorT
     public void testWithPreResultListener() throws Exception {
         conversionErrors.put("foo", "Hello");
 
-        ActionContext ac = new ActionContext(stack.getContext());
-        ac.setConversionErrors(conversionErrors);
-        ac.setValueStack(stack);
+        ActionContext ac = createActionContext();
+        MockActionInvocation mai = createActionInvocation(ac);
+        SimpleAction action = createAction(mai);
 
-        MockActionInvocation mai = new MockActionInvocation();
-        mai.setInvocationContext(ac);
-        mai.setStack(stack);
-        SimpleAction action = new SimpleAction();
-        action.setFoo(55);
-        mai.setAction(action);
-        stack.push(action);
         assertNull(action.getFieldErrors().get("foo"));
-        assertEquals(new Integer(55), stack.findValue("foo"));
+        assertEquals(55, stack.findValue("foo"));
 
         interceptor.intercept(mai);
 
@@ -91,6 +84,51 @@ public class ConversionErrorInterceptorT
         assertEquals("Hello", stack.findValue("foo")); // assume that the 
original value is reset
     }
 
+    /**
+     * See WW-3668
+     * @throws Exception
+     */
+    public void testWithPreResultListenerAgainstMaliciousCode() throws 
Exception {
+        conversionErrors.put("foo", "' + #root + '");
+
+        ActionContext ac = createActionContext();
+
+        MockActionInvocation mai = createActionInvocation(ac);
+
+        SimpleAction action = createAction(mai);
+        assertNull(action.getFieldErrors().get("foo"));
+        assertEquals(55, stack.findValue("foo"));
+
+        interceptor.intercept(mai);
+
+        assertTrue(action.hasFieldErrors());
+        assertNotNull(action.getFieldErrors().get("foo"));
+
+        assertEquals("' + #root + '", stack.findValue("foo"));
+    }
+
+    private MockActionInvocation createActionInvocation(ActionContext ac) {
+        MockActionInvocation mai = new MockActionInvocation();
+        mai.setInvocationContext(ac);
+        mai.setStack(stack);
+        return mai;
+    }
+
+    private SimpleAction createAction(MockActionInvocation mai) {
+        SimpleAction action = new SimpleAction();
+        action.setFoo(55);
+        mai.setAction(action);
+        stack.push(action);
+        return action;
+    }
+
+    private ActionContext createActionContext() {
+        ActionContext ac = new ActionContext(stack.getContext());
+        ac.setConversionErrors(conversionErrors);
+        ac.setValueStack(stack);
+        return ac;
+    }
+
     @Override
     protected void setUp() throws Exception {
         super.setUp();
@@ -99,7 +137,7 @@ public class ConversionErrorInterceptorT
         invocation = (ActionInvocation) mockInvocation.proxy();
         stack = ActionContext.getContext().getValueStack();
         context = new ActionContext(stack.getContext());
-        conversionErrors = new HashMap();
+        conversionErrors = new HashMap<String,Object>();
         context.setConversionErrors(conversionErrors);
         mockInvocation.matchAndReturn("getInvocationContext", context);
         mockInvocation.expect("addPreResultListener", 
C.isA(PreResultListener.class));