From dab98483e6f1cc78ac9b3d56441bc70aeef8816f Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Nov 11 2013 14:47:03 +0000 Subject: Bug 1024552 DoS due to improper handling of ger attr searches https://bugzilla.redhat.com/show_bug.cgi?id=1024552 Reviewed by: nhosoi (Thanks!) Branch: master Fix Description: The traversal of the attr list looking for GER objectclasses was modifying the same attribute twice, removing the "@" from it. The second time, since there was no "@" in the string, the strchr would return NULL, and the code would not check for it. The code was simplified and rewritten to use charray_merge_nodup to build the gerattrs list with unique objectclass values, which I believe was the intention of the original code. I also added some error checking to look for invalid attributes like "@name" "name@" and "name@name@name". Platforms tested: RHEL6 x86_64 Flag Day: no Doc impact: no (cherry picked from commit 3a1ce9e326d9788be233f7edd9d7bad20efb9690) (cherry picked from commit 47f1769dbd1618d0385fb3e5441219f9c280486b) --- diff --git a/ldap/servers/slapd/search.c b/ldap/servers/slapd/search.c index 1a824b2..9f165a1 100644 --- a/ldap/servers/slapd/search.c +++ b/ldap/servers/slapd/search.c @@ -246,8 +246,6 @@ do_search( Slapi_PBlock *pb ) } if ( attrs != NULL ) { - int gerattrsiz = 1; - int gerattridx = 0; int aciin = 0; /* * . store gerattrs if any @@ -257,66 +255,25 @@ do_search( Slapi_PBlock *pb ) { char *p = NULL; /* check if @ is included */ - p = strchr(attrs[i], '@'); - if ( p && '\0' != *(p+1) ) /* don't store "*@", e.g. */ + p = strchr(attrs[i], '@'); + if ( p ) { - int j = 0; - if (gerattridx + 1 >= gerattrsiz) + char *dummyary[2]; /* need a char ** for charray_merge_nodup */ + if ((*(p + 1) == '\0') || (p == attrs[i]) || (strchr(p+1, '@'))) /* e.g. "foo@" or "@objectclassname" or "foo@bar@baz" */ { - char **tmpgerattrs; - gerattrsiz *= 2; - tmpgerattrs = - (char **)slapi_ch_calloc(1, gerattrsiz*sizeof(char *)); - if (NULL != gerattrs) - { - memcpy(tmpgerattrs, gerattrs, gerattrsiz*sizeof(char *)); - slapi_ch_free((void **)&gerattrs); - } - gerattrs = tmpgerattrs; - } - for ( j = 0; gerattrs; j++ ) - { - char *attri = NULL; - if ( NULL == gerattrs[j] ) - { - if (0 == j) - { - /* first time */ - gerattrs[gerattridx++] = attrs[i]; - /* get rid of "@" part from the attr - list, which is needed only in gerattr list */ - *p = '\0'; - attri = slapi_ch_strdup(attrs[i]); - attrs[i] = attri; - *p = '@'; - } - else - { - break; /* done */ - } - } - else if ( 0 == strcasecmp( attrs[i], gerattrs[j] )) - { - /* skip if attrs[i] is already in gerattrs */ - continue; - } - else - { - char *q = strchr(gerattrs[j], '@'); /* q never be 0 */ - if ( 0 != strcasecmp( p+1, q+1 )) - { - /* you don't want to display the same template - entry multiple times */ - gerattrs[gerattridx++] = attrs[i]; - } - /* get rid of "@" part from the attr - list, which is needed only in gerattr list */ - *p = '\0'; - attri = slapi_ch_strdup(attrs[i]); - attrs[i] = attri; - *p = '@'; - } + slapi_log_error( SLAPI_LOG_ARGS, "do_search", + "invalid attribute [%s] in list - must be of the form " + "attributename@objectclassname where attributename is the " + "name of an attribute or \"*\" or \"+\" and objectclassname " + "is the name of an objectclass\n", attrs[i] ); + continue; } + dummyary[0] = p; /* p = @objectclassname */ + dummyary[1] = NULL; + /* copy string to gerattrs with leading @ - disallow dups */ + charray_merge_nodup(&gerattrs, dummyary, 1); + /* null terminate the attribute name at the @ after it has been copied */ + *p = '\0'; } else if ( !aciin && strcasecmp(attrs[i], LDAP_ALL_USER_ATTRS) == 0 ) { @@ -324,10 +281,6 @@ do_search( Slapi_PBlock *pb ) aciin = 1; } } - if (NULL != gerattrs) - { - gerattrs[gerattridx] = NULL; - } /* Set attrs to SLAPI_SEARCH_ATTRS once to get rid of the forbidden attrs */ slapi_pblock_set( pb, SLAPI_SEARCH_ATTRS, attrs );