diff --git a/core/src/main/java/com/opensymphony/xwork2/inject/ContainerImpl.java b/core/src/main/java/com/opensymphony/xwork2/inject/ContainerImpl.java
index 9505c64ef..1fa6b94b9 100644
--- a/core/src/main/java/com/opensymphony/xwork2/inject/ContainerImpl.java
+++ b/core/src/main/java/com/opensymphony/xwork2/inject/ContainerImpl.java
@@ -164,7 +164,8 @@ class ContainerImpl implements Container {
public FieldInjector(ContainerImpl container, Field field, String name)
throws MissingDependencyException {
this.field = field;
- if (!field.isAccessible()) {
+ if ((!Modifier.isPublic(field.getModifiers()) || !Modifier.isPublic(field.getDeclaringClass().getModifiers()))
+ && !field.isAccessible()) {
SecurityManager sm = System.getSecurityManager();
try {
if (sm != null) {
@@ -256,7 +257,8 @@ class ContainerImpl implements Container {
public MethodInjector(ContainerImpl container, Method method, String name) throws MissingDependencyException {
this.method = method;
- if (!method.isAccessible()) {
+ if ((!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers()))
+ && !method.isAccessible()) {
SecurityManager sm = System.getSecurityManager();
try {
if (sm != null) {
@@ -306,7 +308,8 @@ class ContainerImpl implements Container {
this.implementation = implementation;
constructor = findConstructorIn(implementation);
- if (!constructor.isAccessible()) {
+ if ((!Modifier.isPublic(constructor.getModifiers()) || !Modifier.isPublic(constructor.getDeclaringClass().getModifiers()))
+ && !constructor.isAccessible()) {
SecurityManager sm = System.getSecurityManager();
try {
if (sm != null) {
diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
index 071ed71f9..8bd8b5631 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -184,6 +184,19 @@ public class OgnlUtil {
this.disallowProxyMemberAccess = Boolean.parseBoolean(disallowProxyMemberAccess);
}
+ /**
+ * @param maxLength Injects the Struts OGNL maximum expression length.
+ */
+ @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH, required = false)
+ protected void applyExpressionMaxLength(String maxLength) {
+ if (maxLength == null || maxLength.isEmpty()) {
+ // user is going to disable this functionality
+ Ognl.applyExpressionMaxLength(null);
+ } else {
+ Ognl.applyExpressionMaxLength(Integer.parseInt(maxLength));
+ }
+ }
+
public boolean isDisallowProxyMemberAccess() {
return disallowProxyMemberAccess;
}
@@ -755,6 +768,9 @@ public class OgnlUtil {
setValue(name, context, o, value);
} catch (OgnlException e) {
Throwable reason = e.getReason();
+ if (reason instanceof SecurityException) {
+ LOG.warn("Could not evaluate this expression due to security constraints: [{}]", name, e);
+ }
String msg = "Caught OgnlException while setting property '" + name + "' on type '" + o.getClass().getName() + "'.";
Throwable exception = (reason == null) ? e : reason;
diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java
index 6bfdb31c9..bb7b4cb14 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java
@@ -204,6 +204,9 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS
}
protected void handleOgnlException(String expr, Object value, boolean throwExceptionOnFailure, OgnlException e) {
+ if (e.getReason() instanceof SecurityException) {
+ LOG.warn("Could not evaluate this expression due to security constraints: [{}]", expr, e);
+ }
boolean shouldLog = shouldLogMissingPropertyWarning(e);
String msg = null;
if (throwExceptionOnFailure || shouldLog) {
@@ -326,7 +329,12 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS
}
protected Object handleOgnlException(String expr, boolean throwExceptionOnFailure, OgnlException e) {
- Object ret = findInContext(expr);
+ Object ret = null;
+ if (e.getReason() instanceof SecurityException) {
+ LOG.warn("Could not evaluate this expression due to security constraints: [{}]", expr, e);
+ } else {
+ ret = findInContext(expr);
+ }
if (ret == null) {
if (shouldLogMissingPropertyWarning(e)) {
LOG.warn("Could not find property [{}]!", expr, e);
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index f7863050c..371540bca 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -261,6 +261,9 @@ public final class StrutsConstants {
/** Enables evaluation of OGNL expressions */
public static final String STRUTS_ENABLE_OGNL_EVAL_EXPRESSION = "struts.ognl.enableOGNLEvalExpression";
+ /** The maximum length of an expression (OGNL) */
+ public static final String STRUTS_OGNL_EXPRESSION_MAX_LENGTH = "struts.ognl.expressionMaxLength";
+
/** Disables {@link org.apache.struts2.dispatcher.StrutsRequestWrapper} request attribute value stack lookup (JSTL accessibility) */
public static final String STRUTS_DISABLE_REQUEST_ATTRIBUTE_VALUE_STACK_LOOKUP = "struts.disableRequestAttributeValueStackLookup";
diff --git a/core/src/main/java/org/apache/struts2/views/jsp/ui/OgnlTool.java b/core/src/main/java/org/apache/struts2/views/jsp/ui/OgnlTool.java
index 952575503..d792d6d9d 100644
--- a/core/src/main/java/org/apache/struts2/views/jsp/ui/OgnlTool.java
+++ b/core/src/main/java/org/apache/struts2/views/jsp/ui/OgnlTool.java
@@ -23,12 +23,16 @@ import ognl.OgnlException;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.ognl.OgnlUtil;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
/**
* FIXME: remove?
*/
public class OgnlTool {
+ private static final Logger LOG = LogManager.getLogger(OgnlTool.class);
+
private OgnlUtil ognlUtil;
public OgnlTool() {
@@ -43,6 +47,9 @@ public class OgnlTool {
try {
return Ognl.getValue(ognlUtil.compile(expr), context);
} catch (OgnlException e) {
+ if (e.getReason() instanceof SecurityException) {
+ LOG.warn("Could not evaluate this expression due to security constraints: [{}]", expr, e);
+ }
return null;
}
}
diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties
index 9cd2c6ba0..23f159ef4 100644
--- a/core/src/main/resources/org/apache/struts2/default.properties
+++ b/core/src/main/resources/org/apache/struts2/default.properties
@@ -219,4 +219,8 @@ struts.ognl.enableExpressionCache=true
### Indicates if Dispatcher should handle unexpected exceptions by calling sendError()
### or simply rethrow it as a ServletException to allow future processing by other frameworks like Spring Security
struts.handle.exception=true
+
+### applies maximum length allowed on OGNL expressions for security enhancement
+struts.ognl.expressionMaxLength=200
+
### END SNIPPET: complete_file
diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml
index e85bc6aa0..f8f8bd0f8 100644
--- a/core/src/main/resources/struts-default.xml
+++ b/core/src/main/resources/struts-default.xml
@@ -55,13 +55,18 @@
diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
index 72a84e9dd..c601a9733 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
@@ -30,6 +30,7 @@ import com.opensymphony.xwork2.util.*;
import com.opensymphony.xwork2.util.Foo;
import com.opensymphony.xwork2.util.location.LocatableProperties;
import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
+import ognl.OgnlException;
import ognl.PropertyAccessor;
import java.io.*;
@@ -40,11 +41,13 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.Logger;
import org.apache.logging.log4j.core.appender.AbstractAppender;
import org.apache.struts2.StrutsConstants;
+import org.apache.struts2.config.DefaultPropertiesProvider;
/**
@@ -347,6 +350,38 @@ public class OgnlValueStackTest extends XWorkTestCase {
}
}
+ public void testFailOnTooLongExpressionWithDefaultProperties() {
+ loadConfigurationProviders(new DefaultPropertiesProvider());
+ Integer repeat = Integer.parseInt(
+ container.getInstance(String.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH));
+
+ OgnlValueStack vs = createValueStack();
+ try {
+ vs.findValue(StringUtils.repeat('.', repeat + 1), true);
+ fail("Failed to throw exception on too long expression");
+ } catch (Exception ex) {
+ assertTrue(ex.getCause() instanceof OgnlException);
+ assertTrue(((OgnlException) ex.getCause()).getReason() instanceof SecurityException);
+ }
+ }
+
+ public void testNotFailOnTooLongValueWithDefaultProperties() {
+ loadConfigurationProviders(new DefaultPropertiesProvider());
+ Integer repeat = Integer.parseInt(
+ container.getInstance(String.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_MAX_LENGTH));
+
+ OgnlValueStack vs = createValueStack();
+
+ Dog dog = new Dog();
+ vs.push(dog);
+
+ String value = StringUtils.repeat('.', repeat + 1);
+
+ vs.setValue("name", value);
+
+ assertEquals(value, dog.getName());
+ }
+
public void testFailsOnMethodThatThrowsException() {
SimpleAction action = new SimpleAction();
OgnlValueStack stack = createValueStack();
diff --git a/core/src/test/java/org/apache/struts2/result/StreamResultTest.java b/core/src/test/java/org/apache/struts2/result/StreamResultTest.java
index 5a147fd32..1b46d0bd9 100644
--- a/core/src/test/java/org/apache/struts2/result/StreamResultTest.java
+++ b/core/src/test/java/org/apache/struts2/result/StreamResultTest.java
@@ -246,12 +246,19 @@ public class StreamResultTest extends StrutsInternalTestCase {
public class MyImageAction implements Action {
- public InputStream getStreamForImage() throws Exception {
+ FileInputStream streamForImage;
+ long contentLength;
+
+ public MyImageAction() throws Exception {
// just use src/test/log4j2.xml as test file
URL url = ClassLoaderUtil.getResource("log4j2.xml", StreamResultTest.class);
File file = new File(new URI(url.toString()));
- FileInputStream fis = new FileInputStream(file);
- return fis;
+ streamForImage = new FileInputStream(file);
+ contentLength = file.length();
+ }
+
+ public InputStream getStreamForImage() throws Exception {
+ return streamForImage;
}
public String execute() throws Exception {
@@ -259,9 +266,7 @@ public class StreamResultTest extends StrutsInternalTestCase {
}
public long getContentLength() throws Exception {
- URL url = ClassLoaderUtil.getResource("log4j2.xml", StreamResultTest.class);
- File file = new File(new URI(url.toString()));
- return file.length();
+ return contentLength;
}
public String getStreamForImageAsString() {
diff --git a/pom.xml b/pom.xml
index 596bc43a2..f80fba98c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -96,9 +96,8 @@
UTF-8
-
+ 3.1.26
4.3.25.RELEASE
- 3.1.23
7.1
3.0.8
1.0.7