diff --git a/opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java b/opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java index 288a28f590..96bbef501c 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java @@ -142,7 +142,9 @@ private BindRule(BindRule left, BindRule right, EnumBooleanTypes booleanType) { /** * Decode an ACI bind rule string representation. * @param input The string representation of the bind rule. - * @return A BindRule class representing the bind rule. + * @return A BindRule class representing the bind rule, or {@code null} if + * {@code input} is null or blank. This null is the recursion base case for + * an empty group; callers must reject it rather than dereference it. * @throws AciException If the string is an invalid bind rule. */ public static BindRule decode (String input) throws AciException { @@ -153,8 +155,10 @@ public static BindRule decode (String input) throws AciException { String bindruleStr = input.trim(); if (bindruleStr.isEmpty()) { - // A blank bind rule (e.g. "( )") must be rejected as a - // syntax error rather than throwing StringIndexOutOfBoundsException. + // A blank bind rule (e.g. "( )") decodes to null rather than + // throwing StringIndexOutOfBoundsException on charAt(0). This null is + // the recursion base case for an empty group; the caller rejects it as + // a syntax error (see the bindrule_1 == null check below). return null; } char firstChar = bindruleStr.charAt(0); @@ -202,6 +206,17 @@ else if (!inQuotes && currentChar == ')') if (numOpen > numClose) { throw new AciException(WARN_ACI_SYNTAX_BIND_RULE_MISSING_CLOSE_PAREN.get(input)); } + /* + * An empty group such as "()" or "( )" decodes to a null bind rule. + * Such a group is not a valid bind rule on its own, and using it as the + * left operand of an "and"/"or" (e.g. "()or userdn=...") would build a + * complex rule with a null left side that is later dereferenced during + * evaluation. Reject both cases here, before the remaining-chars test, + * so the message names the full offending input rather than a fragment. + */ + if (bindrule_1 == null) { + throw new AciException(WARN_ACI_SYNTAX_INVALID_BIND_RULE_SYNTAX.get(input)); + } /* * If there are remaining chars => there MUST be an operand (AND / OR) * otherwise there is a syntax error @@ -318,6 +333,16 @@ private static BindRule createBindRule(BindRule bindrule, boolean negate=determineNegation(ruleExpr); remainingBindrule=ruleExpr.toString(); BindRule bindrule_2 = BindRule.decode(remainingBindrule); + /* + * The right operand of an "and"/"or" bind rule must exist. It is null + * when the boolean operator is not followed by a bind rule, e.g. the + * "or" in "userdn=\"ldap:///self\" or" has nothing to its right + * (issue #719). + */ + if (bindrule_2 == null) { + throw new AciException( + WARN_ACI_SYNTAX_INVALID_BIND_RULE_SYNTAX.get(remainingBindruleStr)); + } bindrule_2.setNegate(negate); return new BindRule(bindrule, bindrule_2, operand); } diff --git a/opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleOperandTest.java b/opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleOperandTest.java new file mode 100644 index 0000000000..cd6ef77570 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleOperandTest.java @@ -0,0 +1,122 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions Copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.server.authorization.dseecompat; + +import static org.assertj.core.api.Assertions.*; + +import org.forgerock.opendj.ldap.ByteString; +import org.forgerock.opendj.ldap.DN; +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.opends.server.types.DirectoryException; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +/** + * Verifies that {@link BindRule#decode(String)} rejects a boolean ("and"/"or") + * bind rule that is missing one of its operands, instead of throwing a + * {@link NullPointerException}. An empty group such as "()" decodes to a null + * bind rule; before this fix, using such a group as the left side, or leaving + * the right side of the boolean empty (e.g. "(()or)"), dereferenced that null + * while parsing and failed with an NPE rather than a diagnosable + * {@link AciException} (issue #719). + */ +@SuppressWarnings("javadoc") +public class BindRuleOperandTest extends DirectoryServerTestCase +{ + @BeforeClass + public void setUp() throws Exception + { + TestCaseUtils.startFakeServer(); + } + + @AfterClass + public void tearDown() throws DirectoryException + { + TestCaseUtils.shutdownFakeServer(); + } + + @DataProvider(name = "bindRulesWithMissingOperand") + public Object[][] bindRulesWithMissingOperand() + { + return new Object[][] { + // the exact bind rule from issue #719: "or" with no right operand and an + // empty left group + { "(()or)" }, + // empty right operand after a valid left operand + { "(userdn=\"ldap:///self\" or)" }, + { "(userdn=\"ldap:///self\" and)" }, + // empty left operand before a valid right operand + { "()or userdn=\"ldap:///self\"" }, + // an empty group is not a bind rule on its own + { "()" }, + // a nested empty group is still empty + { "(())" }, + // a whitespace-only group is empty once trimmed + { "( )" }, + // empty right *group*: reaches the decode-level guard from createBindRule + { "(userdn=\"ldap:///self\" or ())" }, + // "not" applied to an empty group + { "not ()" }, + }; + } + + @Test(dataProvider = "bindRulesWithMissingOperand") + public void rejectsMissingOperand(String bindRule) + { + assertThatThrownBy(() -> BindRule.decode(bindRule)).isInstanceOf(AciException.class); + } + + @DataProvider(name = "validBindRules") + public Object[][] validBindRules() + { + return new Object[][] { + // a valid rule inside a nested group must still decode: guards must not + // start over-rejecting non-empty groups + { "((userdn=\"ldap:///self\"))" }, + // a valid complex rule with both operands present + { "(userdn=\"ldap:///self\" or userdn=\"ldap:///anyone\")" }, + }; + } + + @Test(dataProvider = "validBindRules") + public void acceptsValidBindRule(String bindRule) throws Exception + { + assertThat(BindRule.decode(bindRule)).isNotNull(); + } + + @DataProvider(name = "acisWithMissingOperand") + public Object[][] acisWithMissingOperand() + { + return new Object[][] { + // the full ACI from issue #719 + { "(version 3.0; acl \"ac\"; allow (search)(()or) (userdn=\"ldap:///self\"); )" }, + // missing *left* operand: previously accepted and stored with left == null, + // then NPEd during evaluation. Must now be rejected at decode time. + { "(version 3.0; acl \"ac\"; allow (search) ()or userdn=\"ldap:///self\"; )" }, + }; + } + + /** Reproduces missing-operand ACIs from issue #719 through {@link Aci#decode}. */ + @Test(dataProvider = "acisWithMissingOperand") + public void decodeOfAciWithMissingOperandThrowsAciException(String aci) + { + assertThatThrownBy(() -> Aci.decode(ByteString.valueOfUtf8(aci), DN.rootDN())) + .isInstanceOf(AciException.class); + } +}