This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d22e1558431fbeb7c555d2c680c372f225e67540 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 e79a56ec8f..21c6114589 100644 --- a/java/org/apache/catalina/realm/JNDIRealm.java +++ b/java/org/apache/catalina/realm/JNDIRealm.java @@ -892,11 +892,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(); } } @@ -2970,6 +2972,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) { @@ -2984,19 +2988,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 483cfe3349..84e84b3bda 100644 --- a/java/org/apache/catalina/realm/LocalStrings.properties +++ b/java/org/apache/catalina/realm/LocalStrings.properties @@ -95,6 +95,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 ba0c19cc67..eb4bcc2de9 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]
