mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Valery Kharseko
9 hours ago 9403d17e98a4dcddb7e9f2559ab67d56da361331
[#719] Fix NullPointerException decoding an ACI bind rule with a missing and/or operand (#720)
1 files modified
1 files added
153 ■■■■■ changed files
opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java 31 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleOperandTest.java 122 ●●●●● patch | view | raw | blame | history
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);
        }
opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleOperandTest.java
New file
@@ -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);
  }
}