From 82ada987ffecd941fe9010564623669b7d330649 Mon Sep 17 00:00:00 2001 From: benwa Date: Fri, 24 Apr 2015 16:46:38 +0200 Subject: [PATCH] JWC-121 Regular expression used in MailboxQuery should be escaped to avoid attacks --- api/pom.xml | 5 + .../apache/james/mailbox/model/MailboxQuery.java | 26 +++- .../james/mailbox/model/MailboxQueryTest.java | 149 +++++++++++++++++++++ pom.xml | 7 +- 4 files changed, 183 insertions(+), 4 deletions(-) create mode 100644 api/src/test/java/org/apache/james/mailbox/model/MailboxQueryTest.java diff --git a/api/pom.xml b/api/pom.xml index 057d75d..699b2fd 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -60,5 +60,10 @@ jmock-junit4 test + + org.assertj + assertj-core + test + diff --git a/api/src/main/java/org/apache/james/mailbox/model/MailboxQuery.java b/api/src/main/java/org/apache/james/mailbox/model/MailboxQuery.java index 0df382d..bde8ae8 100644 --- a/api/src/main/java/org/apache/james/mailbox/model/MailboxQuery.java +++ b/api/src/main/java/org/apache/james/mailbox/model/MailboxQuery.java @@ -19,6 +19,7 @@ package org.apache.james.mailbox.model; +import java.util.StringTokenizer; import java.util.regex.Pattern; @@ -67,9 +68,7 @@ public final class MailboxQuery { this.expression = expression; } this.pathDelimiter = pathDelimiter; - - // Compile some pattern which is used later - pattern = Pattern.compile(this.expression.replaceAll("\\" + pathDelimiter ,"\\\\" + pathDelimiter).replaceAll("\\*", "\\.\\*").replaceAll("\\%", "[^\\" + pathDelimiter + "]*")); + pattern = constructEscapedRegex(); } /** @@ -189,4 +188,25 @@ public final class MailboxQuery { return "MailboxExpression [ " + "base = " + this.base + TAB + "expression = " + this.expression + TAB + "freeWildcard = " + this.getFreeWildcard() + TAB + "localWildcard = " + this.getLocalWildcard() + TAB + " ]"; } + + private Pattern constructEscapedRegex() { + StringBuilder stringBuilder = new StringBuilder(); + StringTokenizer tokenizer = new StringTokenizer(expression, "*%", true); + while (tokenizer.hasMoreTokens()) { + stringBuilder.append(getRegexPartAssociatedWithToken(tokenizer)); + } + return Pattern.compile(stringBuilder.toString()); + } + + private String getRegexPartAssociatedWithToken(StringTokenizer tokenizer) { + String token = tokenizer.nextToken(); + if (token.equals("*")) { + return ".*"; + } else if (token.equals("%")) { + return "[^" + pathDelimiter + "]*"; + } else { + return Pattern.quote(token); + } + } + } diff --git a/api/src/test/java/org/apache/james/mailbox/model/MailboxQueryTest.java b/api/src/test/java/org/apache/james/mailbox/model/MailboxQueryTest.java new file mode 100644 index 0000000..692437c --- /dev/null +++ b/api/src/test/java/org/apache/james/mailbox/model/MailboxQueryTest.java @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +package org.apache.james.mailbox.model; + +import org.junit.Before; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class MailboxQueryTest { + + MailboxPath path; + + @Before + public void setUp() { + path = new MailboxPath("namespace", "user", "name"); + } + + @Test + public void simpleMailboxQueryShouldMatchItsValue() { + assertThat(new MailboxQuery(path, "folder", ':').isExpressionMatch("folder")).isTrue(); + } + + @Test + public void simpleMailboxQueryShouldNotMatchChildFolder() { + assertThat(new MailboxQuery(path, "folder", ':').isExpressionMatch("folder:123")).isFalse(); + } + + @Test + public void simpleMailboxQueryShouldNotMatchFolderWithAnExpendedName() { + assertThat(new MailboxQuery(path, "folder", ':').isExpressionMatch("folder123")).isFalse(); + } + + @Test + public void freeWildcardQueryShouldMatchItsValue() { + assertThat(new MailboxQuery(path, "folder*", ':').isExpressionMatch("folder")).isTrue(); + } + + @Test + public void freeWildcardQueryShouldMatchChildFolder() { + assertThat(new MailboxQuery(path, "folder*", ':').isExpressionMatch("folder:123")).isTrue(); + } + + @Test + public void freeWildcardQueryShouldMatchFolderWithAnExpendedName() { + assertThat(new MailboxQuery(path, "folder*", ':').isExpressionMatch("folder123")).isTrue(); + } + + @Test + public void localWildcardWithOtherCharactersQueryShouldNotMatchItsValue() { + assertThat(new MailboxQuery(path, "folder%3", ':').isExpressionMatch("folder")).isFalse(); + } + + @Test + public void localWildcardWithOtherCharactersQueryShouldNotMatchChildFolder() { + assertThat(new MailboxQuery(path, "folder%3", ':').isExpressionMatch("folder:123")).isFalse(); + } + + @Test + public void localWildcardWithOtherCharactersQueryShouldMatchFolderWithAnExpendedName() { + assertThat(new MailboxQuery(path, "folder%3", ':').isExpressionMatch("folder123")).isTrue(); + } + + @Test + public void localWildcardWithOtherCharactersQueryShouldMatchFolderWithRegexSpecialCharacter() { + assertThat(new MailboxQuery(path, "folder^$!)(%3", ':').isExpressionMatch("folder^$!)(123")).isTrue(); + } + + @Test + public void emptyMailboxQueryShouldMatchItsValue() { + assertThat(new MailboxQuery(path, "", ':').isExpressionMatch("")).isTrue(); + } + + @Test + public void emptyMailboxQueryShouldNotMatchChildFolder() { + assertThat(new MailboxQuery(path, "", ':').isExpressionMatch(":123")).isFalse(); + } + + @Test + public void emptyMailboxQueryShouldNotMatchFolderWithAnOtherName() { + assertThat(new MailboxQuery(path, "", ':').isExpressionMatch("folder")).isFalse(); + } + + @Test + public void freeWildcardAloneMailboxQueryShouldMatchAnyValue() { + assertThat(new MailboxQuery(path, "*", ':').isExpressionMatch("folder")).isTrue(); + } + + @Test + public void freeWildcardAloneMailboxQueryShouldMatchChild() { + assertThat(new MailboxQuery(path, "*", ':').isExpressionMatch("folder:123")).isTrue(); + } + + @Test + public void localWildcardAloneMailboxQueryShouldMatchAnyValue() { + assertThat(new MailboxQuery(path, "%", ':').isExpressionMatch("folder")).isTrue(); + } + + @Test + public void localWildcardAloneMailboxQueryShouldNotMatchChild() { + assertThat(new MailboxQuery(path, "%", ':').isExpressionMatch("folder:123")).isFalse(); + } + + @Test + public void regexInjectionShouldNotBePossibleUsingEndOfQuote() { + assertThat(new MailboxQuery(path, "\\Efo.", ':').isExpressionMatch("\\Efol")).isFalse(); + assertThat(new MailboxQuery(path, "\\Efo.", ':').isExpressionMatch("\\Efo.")).isTrue(); + } + + @Test + public void regexInjectionShouldNotBePossibleUsingBeginOfQuote() { + assertThat(new MailboxQuery(path, "\\Qfo.", ':').isExpressionMatch("\\Qfol")).isFalse(); + assertThat(new MailboxQuery(path, "\\Qfo.", ':').isExpressionMatch("\\Qfo.")).isTrue(); + } + + @Test + public void nullMailboxQueryShouldNotMathAnything() { + assertThat(new MailboxQuery(path, null, ':').isExpressionMatch("folder")).isFalse(); + } + + @Test + public void freeWildcardAreNotEscaped() { + assertThat(new MailboxQuery(path, "folder\\*", ':').isExpressionMatch("folder\\123")).isTrue(); + } + + @Test + public void localWildcardAreNotEscaped() { + assertThat(new MailboxQuery(path, "folder\\%", ':').isExpressionMatch("folder\\123")).isTrue(); + } + +} diff --git a/pom.xml b/pom.xml index aad4632..b1d5fb5 100644 --- a/pom.xml +++ b/pom.xml @@ -121,6 +121,7 @@ 13.0 2.0.1 2.0.2.1 + 2.0.0 @@ -408,7 +409,11 @@ commons-io ${commons-io.version} - + + org.assertj + assertj-core + ${assertj.version} + -- 2.3.6