Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading