zrhoffman / rpms / 389-ds-base

Forked from rpms/389-ds-base 3 years ago
Clone
Blob Blame History Raw
From 896091407c244ed151f2fad39a82881a6e991d26 Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@redhat.com>
Date: Tue, 29 Oct 2013 13:47:35 -0600
Subject: [PATCH] Ticket #47605 CVE-2013-4485: DoS due to improper handling of ger attr searches

https://fedorahosted.org/389/ticket/47605
Reviewed by: nhosoi (Thanks!)
Branch: 389-ds-base-1.3.1
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 7e03702932546e74f0044d11832e7e7e395cbb36)
(cherry picked from commit 12e54af6982ab5406f4bba6a02dd0724a0415501)
(cherry picked from commit 8c5e74b291d08c66e0afbf766f77f955725b9bf4)
---
 ldap/servers/slapd/search.c |   79 +++++++++----------------------------------
 1 files changed, 16 insertions(+), 63 deletions(-)

diff --git a/ldap/servers/slapd/search.c b/ldap/servers/slapd/search.c
index da1772f..59c4afb 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 @<objectclass> 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 "@<objectclass>" 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 "@<objectclass>" 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;
-		}
 
 		if (config_get_return_orig_type_switch()) {
 			/* return the original type, e.g., "sn (surname)" */
-- 
1.7.1