diff --git a/src/main/java/org/owasp/esapi/waf/rules/MustMatchRule.java b/src/main/java/org/owasp/esapi/waf/rules/MustMatchRule.java index 9b0c42ff5..37cc62525 100644 --- a/src/main/java/org/owasp/esapi/waf/rules/MustMatchRule.java +++ b/src/main/java/org/owasp/esapi/waf/rules/MustMatchRule.java @@ -103,7 +103,7 @@ public Action check(HttpServletRequest req, if ( target.contains("*") || target.contains("?") ) { - target = target.replaceAll("*", ".*"); + target = target.replaceAll("\\*", ".*"); Pattern p = Pattern.compile(target); Enumeration e = request.getParameterNames(); @@ -120,6 +120,8 @@ public Action check(HttpServletRequest req, } } + return new DoNothingAction(); + } else { String s = request.getParameter(target); @@ -129,6 +131,8 @@ public Action check(HttpServletRequest req, return new DefaultAction(); } + return new DoNothingAction(); + } } @@ -160,7 +164,7 @@ public Action check(HttpServletRequest req, if ( target.contains("*") || target.contains("?") ) { - target = target.replaceAll("*", ".*"); + target = target.replaceAll("\\*", ".*"); Pattern p = Pattern.compile(target); Enumeration e = request.getHeaderNames(); diff --git a/src/test/java/org/owasp/esapi/waf/rules/MustMatchRuleTest.java b/src/test/java/org/owasp/esapi/waf/rules/MustMatchRuleTest.java new file mode 100644 index 000000000..17c16cd02 --- /dev/null +++ b/src/test/java/org/owasp/esapi/waf/rules/MustMatchRuleTest.java @@ -0,0 +1,82 @@ +package org.owasp.esapi.waf.rules; + +import static org.junit.Assert.assertTrue; + +import java.util.regex.Pattern; + +import org.junit.Test; +import org.owasp.esapi.http.MockHttpServletRequest; +import org.owasp.esapi.waf.actions.Action; +import org.owasp.esapi.waf.actions.DefaultAction; +import org.owasp.esapi.waf.actions.DoNothingAction; +import org.owasp.esapi.waf.configuration.AppGuardianConfiguration; +import org.owasp.esapi.waf.internal.InterceptingHTTPServletRequest; + +/** + * Checks wildcard names in <must-match> rules. See GitHub issue #458. + */ +public class MustMatchRuleTest { + + private static final Pattern ADMIN_PATH = Pattern.compile("^/admin/.*"); + + private Action check(MustMatchRule rule, MockHttpServletRequest mock) throws Exception { + mock.setRequestURI("/admin/config"); + InterceptingHTTPServletRequest request = new InterceptingHTTPServletRequest(mock); + return rule.check(request, null, null); + } + + private MustMatchRule rule(String variable, String value) { + return new MustMatchRule("458", ADMIN_PATH, variable, + AppGuardianConfiguration.OPERATOR_CONTAINS, value); + } + + @Test + public void wildcardHeaderPasses() throws Exception { + MockHttpServletRequest mock = new MockHttpServletRequest(); + mock.addHeader("x-roles", "admin"); + Action action = check(rule("request.headers.x-*", "admin"), mock); + assertTrue(action instanceof DoNothingAction); + } + + @Test + public void wildcardHeaderFails() throws Exception { + MockHttpServletRequest mock = new MockHttpServletRequest(); + mock.addHeader("x-roles", "guest"); + Action action = check(rule("request.headers.x-*", "admin"), mock); + assertTrue(action instanceof DefaultAction); + } + + @Test + public void wildcardParameterPasses() throws Exception { + MockHttpServletRequest mock = new MockHttpServletRequest(); + mock.addParameter("role1", "admin"); + mock.addParameter("role2", "admin"); + Action action = check(rule("request.parameters.role*", "admin"), mock); + assertTrue(action instanceof DoNothingAction); + } + + @Test + public void wildcardParameterFails() throws Exception { + MockHttpServletRequest mock = new MockHttpServletRequest(); + mock.addParameter("role1", "admin"); + mock.addParameter("role2", "guest"); + Action action = check(rule("request.parameters.role*", "admin"), mock); + assertTrue(action instanceof DefaultAction); + } + + @Test + public void plainParameterPasses() throws Exception { + MockHttpServletRequest mock = new MockHttpServletRequest(); + mock.addParameter("role", "admin"); + Action action = check(rule("request.parameters.role", "admin"), mock); + assertTrue(action instanceof DoNothingAction); + } + + @Test + public void plainParameterFails() throws Exception { + MockHttpServletRequest mock = new MockHttpServletRequest(); + mock.addParameter("role", "guest"); + Action action = check(rule("request.parameters.role", "admin"), mock); + assertTrue(action instanceof DefaultAction); + } +}