From 9403d17e98a4dcddb7e9f2559ab67d56da361331 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Sat, 11 Jul 2026 07:40:10 +0000
Subject: [PATCH] [#719] Fix NullPointerException decoding an ACI bind rule with a missing and/or operand (#720)
---
opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleOperandTest.java | 122 ++++++++++++++++++++++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java | 31 +++++++++-
2 files changed, 150 insertions(+), 3 deletions(-)
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 288a28f..96bbef5 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 @@
/**
* 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 @@
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);
@@ -203,6 +207,17 @@
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 @@
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 0000000..cd6ef77
--- /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);
+ }
+}
--
Gitblit v1.10.0