[389-commits] ldap/servers

[Noriko Hosoi]

 ldap/servers/plugins/acl/aclparse.c |   88 +++++++++++++++++++++++++-----------
 1 file changed, 62 insertions(+), 26 deletions(-)

New commits:
commit 6f0705102374bcff44c24f0d90e7fb4c70e646df
Author: Noriko Hosoi <nhosoi at redhat.com>
Date:   Thu May 20 14:55:36 2010 -0700

    593899 - adding specific ACI causes very large mem allocate request
    
    https://bugzilla.redhat.com/show_bug.cgi?id=593899
    
    Fix Description: There was a bug if an invalid syntax acl was given
    (e.g., the value of userdn was not double quoted), normalize_nextACERule
    mistakenly continued processing the acl and eventually tried to
    allocate a huge size of memory (since the end address was less
    than the start address, end - start became negative) and it made
    the server quit.  Added more error handling code to prevent such
    failures.

diff --git a/ldap/servers/plugins/acl/aclparse.c b/ldap/servers/plugins/acl/aclparse.c
index 80fcfa0..b128ff8 100644
--- a/ldap/servers/plugins/acl/aclparse.c
+++ b/ldap/servers/plugins/acl/aclparse.c
@@ -764,7 +764,7 @@ normalize_nextACERule:
 	 * " allow (all) groupdn = "ldap:///cn=Domain Administrators,o=$dn.o,o=ISP"
 	 */
 	s =  __aclp__getNextLASRule(aci_item, acestr, &end);
-	while ( s ) {
+	while ( s && (s < end) ) {
 		if ( (0 == strncmp(s, DS_LAS_USERDNATTR, 10)) ||
 			 (0 == strncmp(s, DS_LAS_USERATTR, 8)) ) {
 			/* 
@@ -778,14 +778,15 @@ normalize_nextACERule:
 			if (rc < 0) {
 				goto error;
 			}
-		} else if ( 0 == strncmp ( s, DS_LAS_USERDN, 6)) {
-			p = strstr ( s, "=");
+		} else if ( 0 == strncmp ( s, DS_LAS_USERDN, 6 )) {
+			p = PL_strnchr (s, '=', end - s);
 			if (NULL == p) {
 				goto error;
 			}
 			p--;
-			if ( strncmp (p, "!=", 2) == 0)
+			if ( strncmp (p, "!=", 2) == 0 ) {
 				aci_item->aci_type |= ACI_CONTAIN_NOT_USERDN;
+			}
 
 			/* XXXrbyrne
 			 * Here we need to scan for more ldap:/// within
@@ -830,7 +831,8 @@ normalize_nextACERule:
 			** we cannot cache the result. See above for more comments.
 			*/
 			/* Find out if we have a URL type of rule */
-			if ((p= strstr (s, "ldap")) != NULL) {
+			p = PL_strnstr (s, "ldap", end - s);
+			if (NULL != p) {
 				if ( aci_item->aci_elevel > ACI_ELEVEL_GROUPDNATTR_URL )
 					aci_item->aci_elevel = ACI_ELEVEL_GROUPDNATTR_URL;
 			} else if ( aci_item->aci_elevel > ACI_ELEVEL_GROUPDNATTR ) {
@@ -845,7 +847,7 @@ normalize_nextACERule:
 			}
 		} else if ( 0 == strncmp ( s, DS_LAS_GROUPDN, 7)) {
 
-			p = strstr ( s, "=");
+			p = PL_strnchr (s, '=', end - s);
 			if (NULL == p) {
 				goto error;
 			}
@@ -868,7 +870,7 @@ normalize_nextACERule:
 	
 		} else if ( 0 == strncmp ( s, DS_LAS_ROLEDN, 6)) {
 
-			p = strstr ( s, "=");
+			p = PL_strnchr (s, '=', end - s);
 			if (NULL == p) {
 				goto error;
 			}
@@ -943,21 +945,20 @@ error:
 static char *
 __aclp__getNextLASRule (aci_t *aci_item, char *original_str , char **endOfCurrRule)
 {
-	char	*newstr, *word, *next, *start, *end;
-	char	*ruleStart = NULL;
-	int		len, ruleLen = 0;
-	int in_dn_expr = 0;
-
-	*endOfCurrRule = NULL;	
-	end = start = NULL;
+	char *newstr = NULL, *word = NULL, *next = NULL, *start = NULL, *end = NULL;
+	char *ruleStart = NULL;
+	int  len, ruleLen = 0;
+	int  in_dn_expr = 0;
 
+	if (endOfCurrRule) {
+		*endOfCurrRule = NULL;
+	}
 	newstr = slapi_ch_strdup (original_str);
 	
 	if ( (strncasecmp(newstr, "allow", 5) == 0) ||
-		(strncasecmp(newstr, "deny", 4) == 0) )  {
-			word = ldap_utf8strtok_r(newstr, ")", &next);
-	}
-	else {
+		 (strncasecmp(newstr, "deny", 4) == 0) )  {
+		word = ldap_utf8strtok_r(newstr, ")", &next);
+	} else {
 		word = ldap_utf8strtok_r(newstr, " ", &next);
 	}
 
@@ -1052,7 +1053,7 @@ __aclp__getNextLASRule (aci_t *aci_item, char *original_str , char **endOfCurrRu
 									 (strncmp ( word, ">=",2) ==0)   ||
 									 (strncmp ( word, "=>",2) ==0)   ||
 									 (strncmp ( word, "=<",2) ==0))
-																	 ) ){	
+																	 ) ) {
 					aci_item->aci_ruleType |= ruleType;
 					got_rule = 1;
 				}
@@ -1088,20 +1089,55 @@ __aclp__getNextLASRule (aci_t *aci_item, char *original_str , char **endOfCurrRu
 			}
 	} /* while */
 
-
 	if ( end ) {
 		/* Found an end to the rule and it's not the last rule */
 		len = end - newstr;
-		end = original_str +len;
-		while ( (end != original_str) && *end != '\"') end--;
-		*endOfCurrRule = end;
+		end = original_str + len;
+		while ( (end != original_str) && *end != '\"' ) end--;
+		if (end == original_str) {
+			char *tmpp = NULL;
+			/* The rule has a problem!  Not double quoted?
+			   It should be like this:
+			   userdn="ldap:///cn=*,ou=testou,o=example.com"
+			   But we got this?
+			   userdn=ldap:///cn=*,ou=testou,o=example.com
+			 */
+			tmpp = original_str + len;
+			/* Just excluding the trailing spaces */
+			while ( (tmpp != original_str) && *tmpp == ' ' ) tmpp--;
+			if (tmpp != original_str) {
+				tmpp++;
+			}
+			end = tmpp;
+		}
+		if (endOfCurrRule) {
+			*endOfCurrRule = end;
+		}
 		len = start - newstr;
 		ruleStart =  original_str + len;
 	} else {
 		/* Walked off the end of the string so it's the last rule */
-		end = original_str + strlen(original_str)-1;
-		while ( (end != original_str)  && *end != '\"') end--;
-		*endOfCurrRule = end;
+		end = original_str + strlen(original_str) - 1;
+		while ( (end != original_str) && *end != '\"' ) end--;
+		if (end == original_str) {
+			char *tmpp = NULL;
+			/* The rule has a problem!  Not double quoted?
+			   It should be like this:
+			   userdn="ldap:///cn=*,ou=testou,o=example.com"
+			   But we got this?
+			   userdn=ldap:///cn=*,ou=testou,o=example.com
+			 */
+			tmpp = original_str + strlen(original_str) - 1;
+			/* Just excluding the trailing spaces */
+			while ( (tmpp != original_str) && *tmpp == ' ' ) tmpp--;
+			if (tmpp != original_str) {
+				tmpp++;
+			}
+			end = tmpp;
+		}
+		if (endOfCurrRule) {
+			*endOfCurrRule = end;
+		}
 	}
 	if ( start ) {
 		/* Got a rule, fixup the pointer */