Apache/struts
1
0
Fork 0
mirror of https://github.com/apache/struts.git synced 2026-08-31 19:35:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

Security patch applied

Browse Source 
git-svn-id: https://svn.apache.org/repos/asf/struts/struts2/branches/STRUTS_2_2_3_1@1164516 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Maurizio Cucchiara
2011-09-02 12:54:49 +00:00
parent babac6c2bf
commit 25e50069d6
6 changed files with 128 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/org/apache/struts2/interceptor/StrutsConversionErrorInterceptor.java
+1 -1
View File
@@ -80,7 +80,7 @@ public class StrutsConversionErrorInterceptor extends ConversionErrorInterceptor
try {
stack.push(value);
return "'" + stack.findValue("top", String.class) + "'";
return escape(stack.findString("top"));
} finally {
stack.pop();
}
core/src/main/resources/template/simple/text.ftl
+2 -2
View File
@@ -29,7 +29,7 @@
maxlength="${parameters.maxlength?html}"<#rt/>
</#if>
<#if parameters.nameValue??>
value="<@s.property value="parameters.nameValue"/>"<#rt/>
value="${parameters.nameValue?html}"<#rt/>
</#if>
<#if parameters.disabled?default(false)>
disabled="disabled"<#rt/>
@@ -50,4 +50,4 @@
<#include "/${parameters.templateDir}/simple/scripting-events.ftl" />
<#include "/${parameters.templateDir}/simple/common-attributes.ftl" />
<#include "/${parameters.templateDir}/simple/dynamic-attributes.ftl" />
/>
/>
core/src/test/java/org/apache/struts2/interceptor/StrutsConversionErrorInterceptorTest.java
+8 -10
View File
@@ -21,11 +21,6 @@
package org.apache.struts2.interceptor;
import java.util.HashMap;
import java.util.Map;
import org.apache.struts2.StrutsTestCase;
import com.mockobjects.dynamic.C;
import com.mockobjects.dynamic.Mock;
import com.opensymphony.xwork2.Action;
@@ -33,7 +28,10 @@ import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.ActionInvocation;
import com.opensymphony.xwork2.ActionSupport;
import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.ValueStackFactory;
import org.apache.struts2.StrutsTestCase;
import java.util.HashMap;
import java.util.Map;
/**
@@ -44,14 +42,14 @@ public class StrutsConversionErrorInterceptorTest extends StrutsTestCase {
protected ActionContext context;
protected ActionInvocation invocation;
protected Map conversionErrors;
protected Map<String, Object> conversionErrors;
protected Mock mockInvocation;
protected ValueStack stack;
protected StrutsConversionErrorInterceptor interceptor;
public void testEmptyValuesDoNotSetFieldErrors() throws Exception {
conversionErrors.put("foo", new Long(123));
conversionErrors.put("foo", 123L);
conversionErrors.put("bar", "");
conversionErrors.put("baz", new String[]{""});
@@ -70,7 +68,7 @@ public class StrutsConversionErrorInterceptorTest extends StrutsTestCase {
}
public void testFieldErrorAdded() throws Exception {
conversionErrors.put("foo", new Long(123));
conversionErrors.put("foo", 123L);
ActionSupport action = new ActionSupport();
mockInvocation.expectAndReturn("getAction", action);
@@ -89,7 +87,7 @@ public class StrutsConversionErrorInterceptorTest extends StrutsTestCase {
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.expectAndReturn("invoke", Action.SUCCESS);
xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptor.java
+6 -1
View File
@@ -20,6 +20,7 @@ import com.opensymphony.xwork2.ActionInvocation;
import com.opensymphony.xwork2.ValidationAware;
import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
import com.opensymphony.xwork2.util.ValueStack;
import org.apache.commons.lang.StringEscapeUtils;
import java.util.HashMap;
import java.util.Map;
@@ -84,7 +85,11 @@ public class ConversionErrorInterceptor extends AbstractInterceptor {
public static final String ORIGINAL_PROPERTY_OVERRIDE = "original.property.override";
protected Object getOverrideExpr(ActionInvocation invocation, Object value) {
return "'" + value + "'";
return escape(value);
}
protected String escape(Object value) {
return "\"" + StringEscapeUtils.escapeJava(String.valueOf(value)) + "\"";
}
@Override
xwork-core/src/main/java/com/opensymphony/xwork2/validator/validators/RepopulateConversionErrorFieldValidatorSupport.java
+58 -49
View File
@@ -22,58 +22,59 @@ import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.logging.Logger;
import com.opensymphony.xwork2.util.logging.LoggerFactory;
import com.opensymphony.xwork2.validator.ValidationException;
import org.apache.commons.lang.StringEscapeUtils;
import java.util.LinkedHashMap;
import java.util.Map;
/**
*
*
*
*
* An abstract base class that adds in the capability to populate the stack with
* a fake parameter map when a conversion error has occurred and the 'repopulateField'
* property is set to "true".
*
*
* <p/>
*
*
*
* <!-- START SNIPPET: javadoc -->
*
* The capability of auto-repopulating the stack with a fake parameter map when
* a conversion error has occurred can be done with 'repopulateField' property
* set to "true".
* The capability of auto-repopulating the stack with a fake parameter map when
* a conversion error has occurred can be done with 'repopulateField' property
* set to "true".
*
* <p/>
*
* This is typically usefull when one wants to repopulate the field with the original value
* when a conversion error occurred. Eg. with a textfield that only allows an Integer
* This is typically usefull when one wants to repopulate the field with the original value
* when a conversion error occurred. Eg. with a textfield that only allows an Integer
* (the action class have an Integer field declared), upon conversion error, the incorrectly
* entered integer (maybe a text 'one') will not appear when dispatched back. With 'repopulateField'
* porperty set to true, it will, meaning the textfield will have 'one' as its value
* porperty set to true, it will, meaning the textfield will have 'one' as its value
* upon conversion error.
*
*
* <!-- END SNIPPET: javadoc -->
*
*
* <p/>
*
*
* <pre>
* <!-- START SNIPPET: exampleJspPage -->
*
*
* &lt;!-- myJspPage.jsp --&gt;
* &lt;ww:form action="someAction" method="POST"&gt;
* ....
* &lt;ww:textfield
* &lt;ww:textfield
* label="My Integer Field"
* name="myIntegerField" /&gt;
* ....
* &lt;ww:submit /&gt;
* &lt;ww:submit /&gt;
* &lt;/ww:form&gt;
*
*
* <!-- END SNIPPET: exampleJspPage -->
* </pre>
*
*
* <pre>
* <!-- START SNIPPET: exampleXwork -->
*
*
* &lt;!-- xwork.xml --&gt;
* &lt;xwork&gt;
* &lt;include file="xwork-default.xml" /&gt;
@@ -88,31 +89,31 @@ import java.util.Map;
* &lt;/package&gt;
* ....
* &lt;/xwork&gt;
*
*
* <!-- END SNIPPET:exampleXwork -->
* </pre>
*
*
*
*
* <pre>
* <!-- START SNIPPET: exampleJava -->
*
*
* &lt;!-- MyActionSupport.java --&gt;
* public class MyActionSupport extends ActionSupport {
* private Integer myIntegerField;
*
*
* public Integer getMyIntegerField() { return this.myIntegerField; }
* public void setMyIntegerField(Integer myIntegerField) {
* this.myIntegerField = myIntegerField;
* public void setMyIntegerField(Integer myIntegerField) {
* this.myIntegerField = myIntegerField;
* }
* }
*
*
* <!-- END SNIPPET: exampleJava -->
* </pre>
*
*
*
*
* <pre>
* <!-- START SNIPPET: exampleValidation -->
*
*
* &lt;!-- MyActionSupport-someAction-validation.xml --&gt;
* &lt;validators&gt;
* ...
@@ -124,24 +125,24 @@ import java.util.Map;
* &lt;/field&gt;
* ...
* &lt;/validators&gt;
*
*
* <!-- END SNIPPET: exampleValidation -->
* </pre>
*
*
* @author tm_jee
* @version $Date$ $Id$
*/
public abstract class RepopulateConversionErrorFieldValidatorSupport extends FieldValidatorSupport {
private static final Logger LOG = LoggerFactory.getLogger(RepopulateConversionErrorFieldValidatorSupport.class);
private String repopulateFieldAsString = "false";
private boolean repopulateFieldAsBoolean = false;
public String getRepopulateField() {
public String getRepopulateField() {
return repopulateFieldAsString;
}
public void setRepopulateField(String repopulateField) {
this.repopulateFieldAsString = repopulateField == null ? repopulateField : repopulateField.trim();
this.repopulateFieldAsBoolean = "true".equalsIgnoreCase(this.repopulateFieldAsString) ? (true) : (false);
@@ -153,12 +154,12 @@ public abstract class RepopulateConversionErrorFieldValidatorSupport extends Fie
repopulateField(object);
}
}
public void repopulateField(Object object) throws ValidationException {
ActionInvocation invocation = ActionContext.getContext().getActionInvocation();
Map<String, Object> conversionErrors = ActionContext.getContext().getConversionErrors();
String fieldName = getFieldName();
String fullFieldName = getValidatorContext().getFullFieldName(fieldName);
if (conversionErrors.containsKey(fullFieldName)) {
@@ -170,19 +171,23 @@ public abstract class RepopulateConversionErrorFieldValidatorSupport extends Fie
if (value instanceof String[]) {
// take the first element, if possible
String[] tmpValue = (String[]) value;
if (tmpValue != null && (tmpValue.length > 0)) {
if ((tmpValue.length > 0)) {
doExprOverride = true;
fakeParams.put(fullFieldName, "'" + tmpValue[0] + "'");
fakeParams.put(fullFieldName, escape(tmpValue[0]));
} else {
LOG.warn("value is an empty array of String or with first element in it as null [" + value + "], will not repopulate conversion error ");
if (LOG.isWarnEnabled()) {
LOG.warn("value is an empty array of String or with first element in it as null [" + value + "], will not repopulate conversion error ");
}
}
} else if (value instanceof String) {
String tmpValue = (String) value;
doExprOverride = true;
fakeParams.put(fullFieldName, "'" + tmpValue + "'");
fakeParams.put(fullFieldName, escape(tmpValue));
} else {
// opps... it should be
LOG.warn("conversion error value is not a String or array of String but instead is [" + value + "], will not repopulate conversion error");
if (LOG.isWarnEnabled()) {
LOG.warn("conversion error value is not a String or array of String but instead is [" + value + "], will not repopulate conversion error");
}
}
if (doExprOverride) {
@@ -194,7 +199,11 @@ public abstract class RepopulateConversionErrorFieldValidatorSupport extends Fie
});
}
}
}
protected abstract void doValidate(Object object) throws ValidationException;
}
}
protected String escape(String value) {
return "\"" + StringEscapeUtils.escapeJava(value) + "\"";
}
protected abstract void doValidate(Object object) throws ValidationException;
}
xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ConversionErrorInterceptorTest.java
+53 -14
View File
@@ -35,13 +35,13 @@ public class ConversionErrorInterceptorTest extends XWorkTestCase {
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 ConversionErrorInterceptorTest extends XWorkTestCase {
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,52 @@ public class ConversionErrorInterceptorTest extends XWorkTestCase {
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 +138,7 @@ public class ConversionErrorInterceptorTest extends XWorkTestCase {
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));