From 705c5c811c56c1e6ccc7c672cb34860527ffb33f Mon Sep 17 00:00:00 2001
From: neil_a_wilson <neil_a_wilson@localhost>
Date: Thu, 05 Jul 2007 06:44:14 +0000
Subject: [PATCH] Update the LDAPFilter code so that the process for decoding filters from strings will perform more strict checking to ensure that the attribute description in a simple filter contains only valid characters.  This will catch filters that are invalid but were not properly rejected, like "((uid=user.0))", "(&&(uid=user.0))", or "!uid=user.0".

---
 opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPFilter.java                             |  113 +++++++++++++++++++++++++++++++++++--
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/ldap/TestLDAPFilter.java |    3 +
 opendj-sdk/opends/src/server/org/opends/server/messages/ProtocolMessages.java                             |   30 +++++++--
 3 files changed, 131 insertions(+), 15 deletions(-)

diff --git a/opendj-sdk/opends/src/server/org/opends/server/messages/ProtocolMessages.java b/opendj-sdk/opends/src/server/org/opends/server/messages/ProtocolMessages.java
index a305e3c..f96dac6 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/messages/ProtocolMessages.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/messages/ProtocolMessages.java
@@ -4609,14 +4609,13 @@
 
   /**
    * The message ID for the message that will be used if an LDAP search filter
-   * is enclosed in apostrophes ("single-quotes").
-   * (FIXME -- This error is a workaround for
-   * https://opends.dev.java.net/issues/show_bug.cgi?id=1024. A correct fix
-   * is to validate the characters used in the attribute type.
+   * is enclosed in apostrophes ("single-quotes").  See issue #1024.
    */
   public static final int MSGID_LDAP_FILTER_ENCLOSED_IN_APOSTROPHES =
        CATEGORY_MASK_PROTOCOL | SEVERITY_MASK_MILD_ERROR | 427;
 
+
+
   /**
    * The message ID for the message that will be used as the description of the
    * configuration attribute specifying whether to enable the LDAPS
@@ -4625,6 +4624,19 @@
   public static final int MSGID_JMX_CONNHANDLER_DESCRIPTION_ENABLE =
        CATEGORY_MASK_PROTOCOL | SEVERITY_MASK_INFORMATIONAL | 428;
 
+
+
+  /**
+   * The message ID for the message that will be used if an LDAP search filter
+   * includes an invalid character in an attribute type.  This takes three
+   * arguments, which are the attribute type, the illegal character, and the
+   * position at which it occurred.
+   */
+  public static final int MSGID_LDAP_FILTER_INVALID_CHAR_IN_ATTR_TYPE =
+       CATEGORY_MASK_PROTOCOL | SEVERITY_MASK_MILD_ERROR | 429;
+
+
+
   /**
    * Associates a set of generic messages with the message IDs defined in this
    * class.
@@ -5410,6 +5422,12 @@
                     "The provided search filter \"%s\" could not be decoded " +
                     "because the NOT filter between positions %d and %d " +
                     "did not contain exactly one filter component");
+    registerMessage(MSGID_LDAP_FILTER_ENCLOSED_IN_APOSTROPHES,
+                    "An LDAP filter enclosed in apostrophes is invalid:  %s");
+    registerMessage(MSGID_LDAP_FILTER_INVALID_CHAR_IN_ATTR_TYPE,
+                    "The provided search filter contains an invalid " +
+                    "attribute type '%s' with invalid character '%s' at " +
+                    "position %d");
 
 
     registerMessage(MSGID_LDAP_CLIENT_SEND_RESPONSE_NO_RESULT_CODE,
@@ -6605,10 +6623,6 @@
     registerMessage(MSGID_CANNOT_DECODE_GETEFFECTIVERIGHTS_AUTHZID_DN,
                     "Unable to decode authzid DN string \"%s\" as a valid " +
                     "distinguished name:  %s");
-
-
-    registerMessage(MSGID_LDAP_FILTER_ENCLOSED_IN_APOSTROPHES,
-                    "An LDAP filter enclosed in apostrophes is invalid:  %s");
   }
 }
 
diff --git a/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPFilter.java b/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPFilter.java
index 4b8ba36..4f0325c 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPFilter.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPFilter.java
@@ -367,10 +367,7 @@
     }
 
     // If the filter is enclosed in a pair of apostrophes ("single-quotes") it
-    // is invalid.
-    // (FIXME -- This error is a workaround for
-    //  https://opends.dev.java.net/issues/show_bug.cgi?id=1024. A correct fix
-    // is to validate the characters used in the attribute type.)
+    // is invalid (issue #1024).
     if (1 < filterString.length()
          && filterString.startsWith("'") && filterString.endsWith("'"))
     {
@@ -432,8 +429,8 @@
 
     if (equalPos <= startPos)
     {
-        int    msgID   = MSGID_LDAP_FILTER_NO_EQUAL_SIGN;
-        String message = getMessage(msgID, filterString, startPos, endPos);
+      int    msgID   = MSGID_LDAP_FILTER_NO_EQUAL_SIGN;
+      String message = getMessage(msgID, filterString, startPos, endPos);
       throw new LDAPException(LDAPResultCode.PROTOCOL_ERROR, msgID, message);
     }
 
@@ -467,8 +464,110 @@
 
 
     // The part of the filter string before the equal sign should be the
-    // attribute type.
+    // attribute type.  Make sure that the characters it contains are acceptable
+    // for attribute types, including those allowed by attribute name
+    // exceptions (ASCII letters and digits, the dash, and the underscore).  We
+    // also need to allow attribute options, which includes the semicolon and
+    // the equal sign.
     String attrType = filterString.substring(startPos, attrEndPos);
+    for (int i=0; i < attrType.length(); i++)
+    {
+      switch (attrType.charAt(i))
+      {
+        case '-':
+        case '0':
+        case '1':
+        case '2':
+        case '3':
+        case '4':
+        case '5':
+        case '6':
+        case '7':
+        case '8':
+        case '9':
+        case ';':
+        case '=':
+        case 'A':
+        case 'B':
+        case 'C':
+        case 'D':
+        case 'E':
+        case 'F':
+        case 'G':
+        case 'H':
+        case 'I':
+        case 'J':
+        case 'K':
+        case 'L':
+        case 'M':
+        case 'N':
+        case 'O':
+        case 'P':
+        case 'Q':
+        case 'R':
+        case 'S':
+        case 'T':
+        case 'U':
+        case 'V':
+        case 'W':
+        case 'X':
+        case 'Y':
+        case 'Z':
+        case '_':
+        case 'a':
+        case 'b':
+        case 'c':
+        case 'd':
+        case 'e':
+        case 'f':
+        case 'g':
+        case 'h':
+        case 'i':
+        case 'j':
+        case 'k':
+        case 'l':
+        case 'm':
+        case 'n':
+        case 'o':
+        case 'p':
+        case 'q':
+        case 'r':
+        case 's':
+        case 't':
+        case 'u':
+        case 'v':
+        case 'w':
+        case 'x':
+        case 'y':
+        case 'z':
+          // These are all OK.
+          break;
+
+        case '.':
+        case '/':
+        case ':':
+        case '<':
+        case '>':
+        case '?':
+        case '@':
+        case '[':
+        case '\\':
+        case ']':
+        case '^':
+        case '`':
+          // These are not allowed, but they are explicitly called out because
+          // they are included in the range of values between '-' and 'z', and
+          // making sure all possible characters are included can help make the
+          // switch statement more efficient.  We'll fall through to the default
+          // clause to reject them.
+        default:
+          int    msgID   = MSGID_LDAP_FILTER_INVALID_CHAR_IN_ATTR_TYPE;
+          String message = getMessage(msgID, attrType,
+                                      String.valueOf(attrType.charAt(i)), i);
+          throw new LDAPException(LDAPResultCode.PROTOCOL_ERROR, msgID,
+                                  message);
+      }
+    }
 
 
     // Get the attribute value.
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/ldap/TestLDAPFilter.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/ldap/TestLDAPFilter.java
index c7be60c..448c76c 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/ldap/TestLDAPFilter.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/ldap/TestLDAPFilter.java
@@ -70,6 +70,9 @@
       { "(cn=billy bob", null },
       { "(|(!(title=sweep*)(l=Paris*)))", null },
       { "(|(!))", null },
+      { "((uid=user.0))", null },
+      { "(&&(uid=user.0))", null },
+      { "!uid=user.0", null },
 
     };
   }

--
Gitblit v1.10.0