Merge pull request #765 from apache/WW-5354-block-params

WW-5354 Ensure ActionSupport fields are not parameter injectable
This commit is contained in:
Kusal Kithul-Godage
2023-10-13 11:40:57 +11:00
committed by GitHub
2 changed files with 20 additions and 19 deletions
@@ -36,8 +36,9 @@ public class DefaultExcludedPatternsChecker implements ExcludedPatternsChecker {
private static final Logger LOG = LogManager.getLogger(DefaultExcludedPatternsChecker.class);
public static final String[] EXCLUDED_PATTERNS = {
"(^|\\%\\{)((#?)(top(\\.|\\['|\\[\")|\\[\\d\\]\\.)?)(dojo|struts|session|request|response|application|servlet(Request|Response|Context)|parameters|context|_memberAccess)(\\.|\\[).*",
".*(^|\\.|\\[|\\'|\"|get)class(\\(\\.|\\[|\\'|\").*"
"(^|\\%\\{)((#?)(top(\\.|\\['|\\[\")|\\[\\d\\]\\.)?)(dojo|struts|session|request|response|application|servlet(Request|Response|Context)|parameters|context|_memberAccess)(\\.|\\[).*",
".*(^|\\.|\\[|\\'|\"|get)class(\\(\\.|\\[|\\'|\").*",
"actionErrors|actionMessages|fieldErrors"
};
private Set<Pattern> excludedPatterns;
@@ -48,21 +49,7 @@ public class DefaultExcludedPatternsChecker implements ExcludedPatternsChecker {
@Inject(value = StrutsConstants.STRUTS_OVERRIDE_EXCLUDED_PATTERNS, required = false)
protected void setOverrideExcludePatterns(String excludePatterns) {
if (excludedPatterns != null && excludedPatterns.size() > 0) {
LOG.warn("Overriding excluded patterns [{}] with [{}], be aware that this affects all instances and safety of your application!",
excludedPatterns, excludePatterns);
} else {
// Limit unwanted log entries (when excludedPatterns null/empty - usually 1st call)
LOG.debug("Overriding excluded patterns with [{}]", excludePatterns);
}
excludedPatterns = new HashSet<>();
try {
for (String pattern : TextParseUtil.commaDelimitedStringToSet(excludePatterns)) {
excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
}
} finally {
excludedPatterns = Collections.unmodifiableSet(excludedPatterns);
}
setExcludedPatterns(excludePatterns);
}
@Inject(value = StrutsConstants.STRUTS_ADDITIONAL_EXCLUDED_PATTERNS, required = false)
@@ -98,7 +85,7 @@ public class DefaultExcludedPatternsChecker implements ExcludedPatternsChecker {
@Override
public void setExcludedPatterns(Set<String> patterns) {
if (excludedPatterns != null && excludedPatterns.size() > 0) {
if (excludedPatterns != null && !excludedPatterns.isEmpty()) {
LOG.warn("Replacing excluded patterns [{}] with [{}], be aware that this affects all instances and safety of your application!",
excludedPatterns, patterns);
} else {
@@ -21,6 +21,7 @@ package com.opensymphony.xwork2.interceptor;
import com.opensymphony.xwork2.Action;
import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.ActionProxy;
import com.opensymphony.xwork2.ActionSupport;
import com.opensymphony.xwork2.ModelDrivenAction;
import com.opensymphony.xwork2.SimpleAction;
import com.opensymphony.xwork2.TestBean;
@@ -300,7 +301,7 @@ public class ParametersInterceptorTest extends XWorkTestCase {
}
};
final Map<String, Boolean> excluded = new HashMap<String, Boolean>();
final Map<String, Boolean> excluded = new HashMap<>();
ParametersInterceptor pi = new ParametersInterceptor() {
@Override
@@ -364,6 +365,19 @@ public class ParametersInterceptorTest extends XWorkTestCase {
assertEquals("This is blah", ((SimpleAction) proxy.getAction()).getBlah());
}
public void testActionSupportParametersBlocked() throws Exception {
Map<String, Object> params = new HashMap<>();
params.put("actionErrors", "fakeError");
params.put("actionMessages", "fakeMessage");
ActionContext extraContext = ActionContext.of().withParameters(HttpParameters.create(params).build());
ActionProxy proxy = actionProxyFactory.createActionProxy("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME, null, extraContext.getContextMap());
proxy.execute();
assertEquals(0, ((ActionSupport) proxy.getAction()).getActionMessages().size());
assertEquals(0, ((ActionSupport) proxy.getAction()).getActionErrors().size());
}
public void testParametersWithSpacesInTheName() throws Exception {
Map<String, Object> params = new HashMap<>();
params.put("theProtectedMap['p0 p1']", "test1");