Skip to content

Commit

Permalink
Solves issue with vulnerable parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
lukaszlenart committed Sep 22, 2015
1 parent 925741a commit f420f28
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class DefaultExcludedPatternsChecker implements ExcludedPatternsChecker {
private static final Logger LOG = LoggerFactory.getLogger(DefaultExcludedPatternsChecker.class);

public static final String[] EXCLUDED_PATTERNS = {
"(^|.*#)(dojo|struts|session|request|application|servlet(Request|Response)|parameters|context|_memberAccess)(\\.|\\[).*",
"(^|\\%\\{)((#?)(top(\\.|\\['|\\[\")|\\[\\d\\]\\.)?)(dojo|struts|session|request|response|application|servlet(Request|Response|Context)|parameters|context|_memberAccess)(\\.|\\[).*",
"^(action|method):.*"
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,13 @@ public void testInsecureParameters() throws Exception {
pi.setParameters(action, vs, params);

// then
assertEquals(1, action.getActionMessages().size());
assertEquals(2, action.getActionMessages().size());

String msg1 = action.getActionMessage(0);
String msg2 = action.getActionMessage(1);

assertTrue(msg1.contains("Error setting expression 'top['name'](0)' with value 'true'"));
assertEquals("Error setting expression 'name' with value '(#context[\"xwork.MethodAccessor.denyMethodExecution\"]= new java.lang.Boolean(false), #_memberAccess[\"allowStaticMethodAccess\"]= new java.lang.Boolean(true), @java.lang.Runtime@getRuntime().exec('mkdir /tmp/PWNAGE'))(meh)'", msg1);
assertEquals("Error setting expression 'top['name'](0)' with value 'true'", msg2);
assertNull(action.getName());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.opensymphony.xwork2.XWorkTestCase;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class DefaultExcludedPatternsCheckerTest extends XWorkTestCase {
Expand Down Expand Up @@ -35,6 +36,10 @@ public void testHardcodedPatterns() throws Exception {
add("%{#servletResponse.test}");
add("%{#ServletResponse['test']}");
add("%{#ServletResponse.test}");
add("%{#servletContext['test']}");
add("%{#servletContext.test}");
add("%{#ServletContext['test']}");
add("%{#ServletContext.test}");
add("%{#parameters['test']}");
add("%{#parameters.test}");
add("%{#Parameters['test']}");
Expand Down Expand Up @@ -65,6 +70,36 @@ public void testHardcodedPatterns() throws Exception {
}
}

public void testDefaultExcludePatterns() throws Exception {
// given
List<String> prefixes = Arrays.asList("#[0].%s", "[0].%s", "top.%s", "%{[0].%s}", "%{#[0].%s}", "%{top.%s}", "%{#top.%s}", "%{#%s}", "%{%s}", "#%s");
List<String> inners = Arrays.asList("servletRequest", "servletResponse", "servletContext", "application", "session", "struts", "request", "response", "dojo", "parameters");
List<String> suffixes = Arrays.asList("['test']", "[\"test\"]", ".test");

DefaultExcludedPatternsChecker checker = new DefaultExcludedPatternsChecker();
checker.setAdditionalExcludePatterns(".*(^|\\.|\\[|'|\")class(\\.|\\[|'|\").*");

List<String> params = new ArrayList<String>();
for (String prefix : prefixes) {
for (String inner : inners) {
String innerUp = inner.substring(0, 1).toUpperCase() + inner.substring(1);
for (String suffix : suffixes) {
params.add(prefix.replace("%s", inner + suffix));
params.add(prefix.replace("%s", innerUp + suffix));
}
}
}

for (String param : params) {
System.out.println(param);
// when
ExcludedPatternsChecker.IsExcluded actual = checker.isExcluded(param);

// then
assertTrue("Access to " + param + " is possible!", actual.isExcluded());
}
}

public void testParamWithClassInName() throws Exception {
// given
List<String> properParams = new ArrayList<String>();
Expand Down

0 comments on commit f420f28

Please sign in to comment.