From ab7848a4a30d79c7433a1689ba1ea18897b73453 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Tue, 18 Sep 2018 16:39:26 -0400
Subject: [PATCH] Bug 1624004 - potential denial of service attack

Bug: a search request passing 8MB of NULL bytes as search attributes will
     keep one thread busy for a long time.
     The reason is that the attr array is copied/normalized to the searchattrs in
     the search operation and does this using charray_add() which iterates thru
     the array to determine the size of the array and then allocate one element more.
     so this means we iterat 8 million times an array with a then average size of
     4 million elements.

Fix: We already have traversed the array once and know the size, so we can allocate
     the needed size once and only copy the element.
     In addition we check for the kind of degenerated attributes "" as used in this
     test scenario.
     So the fix will reject invalid attr liste and improve performance for valid ones

https://bugzilla.redhat.com/show_bug.cgi?id=1624004
---
 ldap/servers/slapd/search.c | 16 ++++++++++++++--
 ldap/servers/slapd/unbind.c |  4 ++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/ldap/servers/slapd/search.c b/ldap/servers/slapd/search.c
index 731c6519e..dc26fc4d2 100644
--- a/ldap/servers/slapd/search.c
+++ b/ldap/servers/slapd/search.c
@@ -209,6 +209,7 @@ do_search(Slapi_PBlock *pb)
     if (attrs != NULL) {
         char *normaci = slapi_attr_syntax_normalize("aci");
         int replace_aci = 0;
+        int attr_count = 0;
         if (!normaci) {
             normaci = slapi_ch_strdup("aci");
         } else if (strcasecmp(normaci, "aci")) {
@@ -218,9 +219,19 @@ do_search(Slapi_PBlock *pb)
         /*
          * . store gerattrs if any
          * . add "aci" once if "*" is given
+         * . check that attrs are not degenerated
          */
         for (i = 0; attrs[i] != NULL; i++) {
             char *p = NULL;
+            attr_count++;
+
+            if ( attrs[i][0] == '\0') {
+                log_search_access(pb, base, scope, fstr, "invalid attribute request");
+                send_ldap_result(pb, LDAP_PROTOCOL_ERROR, NULL, NULL, 0, NULL);
+                slapi_ch_free_string(&normaci);
+                goto free_and_return;
+            }
+
             /* check if @<objectclass> is included */
             p = strchr(attrs[i], '@');
             if (p) {
@@ -244,6 +255,7 @@ do_search(Slapi_PBlock *pb)
             } else if (strcmp(attrs[i], LDAP_ALL_USER_ATTRS /* '*' */) == 0) {
                 if (!charray_inlist(attrs, normaci)) {
                     charray_add(&attrs, slapi_ch_strdup(normaci));
+                    attr_count++;
                 }
             } else if (replace_aci && (strcasecmp(attrs[i], "aci") == 0)) {
                 slapi_ch_free_string(&attrs[i]);
@@ -263,13 +275,13 @@ do_search(Slapi_PBlock *pb)
             }
         } else {
             /* return the chopped type, e.g., "sn" */
-            operation->o_searchattrs = NULL;
+            operation->o_searchattrs = (char **)slapi_ch_calloc(sizeof(char *), attr_count+1);
             for (i = 0; attrs[i] != NULL; i++) {
                 char *type;
                 type = slapi_attr_syntax_normalize_ext(attrs[i],
                                                        ATTR_SYNTAX_NORM_ORIG_ATTR);
                 /* attrs[i] is consumed */
-                charray_add(&operation->o_searchattrs, attrs[i]);
+                operation->o_searchattrs[i] = attrs[i];
                 attrs[i] = type;
             }
         }
diff --git a/ldap/servers/slapd/unbind.c b/ldap/servers/slapd/unbind.c
index 90f7b1546..686e27a8e 100644
--- a/ldap/servers/slapd/unbind.c
+++ b/ldap/servers/slapd/unbind.c
@@ -87,8 +87,8 @@ do_unbind(Slapi_PBlock *pb)
     /* pass the unbind to all backends */
     be_unbindall(pb_conn, operation);
 
+free_and_return:;
+
     /* close the connection to the client */
     disconnect_server(pb_conn, operation->o_connid, operation->o_opid, SLAPD_DISCONNECT_UNBIND, 0);
-
-free_and_return:;
 }
-- 
2.17.1