This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 69d6c1449b512423c34beb94456b172bd8d5d857 Author: opencode <[email protected]> AuthorDate: Wed Oct 7 15:34:15 2026 +0200 Report malformed JNDIRealm user patterns as a clear configuration error parseUserPatternString() could walk off the end of the pattern string when parentheses were unbalanced: a trailing open paren made charAt(startParenLoc + 1) throw StringIndexOutOfBoundsException, a missing close paren made charAt(endParenLoc - 1) throw with index -2, and a pattern whose only open paren was escaped made the weeding loop read charAt(-2) after indexOf had returned -1. Certain inputs such as (|x) oscillated the weeding loop forever. The obscure crash surfaced while setting the userPattern property, e.g. during server.xml parsing. Guard the paren searches, throw an IllegalArgumentException naming the offending pattern when parentheses are unbalanced, and treat a pattern in which every paren is escaped as a single pattern rather than crashing, consistent with the no-parens case. setUserPattern now parses before assigning, so a rejected pattern leaves the previous configuration intact. Add unit tests covering the malformed and fallback cases. --- java/org/apache/catalina/realm/JNDIRealm.java | 33 ++++++++++++++++--- .../apache/catalina/realm/LocalStrings.properties | 1 + test/org/apache/catalina/realm/TestJNDIRealm.java | 38 ++++++++++++++++++++++ 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/java/org/apache/catalina/realm/JNDIRealm.java b/java/org/apache/catalina/realm/JNDIRealm.java index 14b9bf2e47..200b8ca434 100644 --- a/java/org/apache/catalina/realm/JNDIRealm.java +++ b/java/org/apache/catalina/realm/JNDIRealm.java @@ -894,11 +894,13 @@ public class JNDIRealm extends RealmBase { * @param userPattern The new user pattern */ public void setUserPattern(String userPattern) { - this.userPattern = userPattern; if (userPattern == null) { + this.userPattern = null; userPatternArray = null; } else { - userPatternArray = parseUserPatternString(userPattern); + String[] parsed = parseUserPatternString(userPattern); + this.userPattern = userPattern; + userPatternArray = parsed; singleConnection = create(); } } @@ -2951,6 +2953,8 @@ public class JNDIRealm extends RealmBase { * @param userPatternString - a string LDAP search paths surrounded by parentheses * * @return a parsed string array + * + * @throws IllegalArgumentException if the pattern contains unbalanced parentheses */ protected String[] parseUserPatternString(String userPatternString) { @@ -2965,19 +2969,38 @@ public class JNDIRealm extends RealmBase { // weed out escaped open parens and parens enclosing the // whole statement (in the case of valid LDAP search // strings: (|(something)(somethingelse)) - while ((userPatternString.charAt(startParenLoc + 1) == '|') || - (startParenLoc != 0 && userPatternString.charAt(startParenLoc - 1) == '\\')) { + while (startParenLoc > -1 && startParenLoc < userPatternString.length() - 1 && + ((userPatternString.charAt(startParenLoc + 1) == '|') || + (startParenLoc != 0 && userPatternString.charAt(startParenLoc - 1) == '\\'))) { startParenLoc = userPatternString.indexOf('(', startParenLoc + 1); } + if (startParenLoc == -1) { + break; + } + if (startParenLoc == userPatternString.length() - 1) { + // an open paren at the end cannot start a pattern + throw new IllegalArgumentException(sm.getString("jndiRealm.invalidUserPattern", + userPatternString)); + } int endParenLoc = userPatternString.indexOf(')', startParenLoc + 1); // weed out escaped end-parens - while (userPatternString.charAt(endParenLoc - 1) == '\\') { + while (endParenLoc > 0 && userPatternString.charAt(endParenLoc - 1) == '\\') { endParenLoc = userPatternString.indexOf(')', endParenLoc + 1); } + if (endParenLoc == -1) { + // no matching close paren for this pattern + throw new IllegalArgumentException(sm.getString("jndiRealm.invalidUserPattern", + userPatternString)); + } String nextPathPart = userPatternString.substring(startParenLoc + 1, endParenLoc); pathList.add(nextPathPart); startParenLoc = userPatternString.indexOf('(', endParenLoc + 1); } + if (pathList.isEmpty()) { + // every paren was escaped, nothing to separate: treat the + // whole string as the pattern + return new String[] { userPatternString }; + } return pathList.toArray(new String[0]); } return null; diff --git a/java/org/apache/catalina/realm/LocalStrings.properties b/java/org/apache/catalina/realm/LocalStrings.properties index 60ea33d5f8..b85215b0e4 100644 --- a/java/org/apache/catalina/realm/LocalStrings.properties +++ b/java/org/apache/catalina/realm/LocalStrings.properties @@ -88,6 +88,7 @@ jndiRealm.invalidHostnameVerifier=[{0}] not a valid class name for a HostnameVer jndiRealm.invalidName=Search returned unparsable absolute name: [{0}] jndiRealm.invalidSslProtocol=Given protocol [{0}] is invalid. It has to be one of [{1}] jndiRealm.invalidSslSocketFactory=[{0}] not a valid class name for an SSLSocketFactory +jndiRealm.invalidUserPattern=The user pattern [{0}] is not valid: it contains unbalanced parentheses jndiRealm.multipleEntries=User name [{0}] has multiple entries jndiRealm.negotiatedTls=Negotiated tls connection using protocol [{0}] jndiRealm.open=Exception opening directory server connection diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java b/test/org/apache/catalina/realm/TestJNDIRealm.java index 0a7f3f8186..c8a870dc95 100644 --- a/test/org/apache/catalina/realm/TestJNDIRealm.java +++ b/test/org/apache/catalina/realm/TestJNDIRealm.java @@ -136,6 +136,44 @@ public class TestJNDIRealm { Assert.assertTrue(latch.await(30, TimeUnit.SECONDS)); } + @Test + public void testParseUserPatternStringValid() throws Exception { + JNDIRealm realm = new JNDIRealm(); + + Assert.assertArrayEquals(new String[] { "cn={0}" }, + realm.parseUserPatternString("cn={0}")); + Assert.assertArrayEquals(new String[] { "cn={0}", "cn={0},o=myorg" }, + realm.parseUserPatternString("(cn={0})(cn={0},o=myorg)")); + Assert.assertArrayEquals(new String[] { "cn={0}", "cn={0},o=myorg" }, + realm.parseUserPatternString("(|(cn={0})(cn={0},o=myorg))")); + Assert.assertArrayEquals(new String[] { "cn=\\(x" }, + realm.parseUserPatternString("cn=\\(x")); + } + + @Test(expected = IllegalArgumentException.class) + public void testParseUserPatternStringTrailingOpenParen() throws Exception { + JNDIRealm realm = new JNDIRealm(); + realm.setUserPattern("cn={0}("); + } + + @Test(expected = IllegalArgumentException.class) + public void testParseUserPatternStringMissingCloseParen() throws Exception { + JNDIRealm realm = new JNDIRealm(); + realm.setUserPattern("(cn={0}"); + } + + @Test(expected = IllegalArgumentException.class) + public void testParseUserPatternStringOnlyOpenParen() throws Exception { + JNDIRealm realm = new JNDIRealm(); + realm.setUserPattern("("); + } + + @Test(expected = IllegalArgumentException.class) + public void testParseUserPatternStringEscapedOpenParenOnly() throws Exception { + JNDIRealm realm = new JNDIRealm(); + realm.setUserPattern("cn=\\("); + } + private JNDIRealm buildRealm(String password) throws NamingException, NoSuchFieldException, IllegalAccessException, LifecycleException { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
