don't trust strchr/strrchr with bervals

configure

@@ -40342,6 +40342,105 @@ _ACEOF
 fi
+echo "$as_me:$LINENO: checking for memrchr" >&5
+echo $ECHO_N "checking for memrchr... $ECHO_C" >&6
+if test "${ac_cv_func_memrchr+set}" = set; then
+  echo $ECHO_N "(cached) $ECHO_C" >&6
+else
+  cat >conftest.$ac_ext <<_ACEOF
+/* confdefs.h.  */
+_ACEOF
+cat confdefs.h >>conftest.$ac_ext
+cat >>conftest.$ac_ext <<_ACEOF
+/* end confdefs.h.  */
+/* Define memrchr to an innocuous variant, in case <limits.h> declares memrchr.
+   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
+#define memrchr innocuous_memrchr
+
+/* System header to define __stub macros and hopefully few prototypes,
+    which can conflict with char memrchr (); below.
+    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
+    <limits.h> exists even on freestanding compilers.  */
+
+#ifdef __STDC__
+# include <limits.h>
+#else
+# include <assert.h>
+#endif
+
+#undef memrchr
+
+/* Override any gcc2 internal prototype to avoid an error.  */
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+/* We use char because int might match the return type of a gcc2
+   builtin and then its argument prototype would still apply.  */
+char memrchr ();
+/* The GNU C library defines this for functions which it implements
+    to always fail with ENOSYS.  Some functions are actually named
+    something starting with __ and the normal name is an alias.  */
+#if defined (__stub_memrchr) || defined (__stub___memrchr)
+choke me
+#else
+char (*f) () = memrchr;
+#endif
+#ifdef __cplusplus
+}
+#endif
+
+int
+main ()
+{
+return f != memrchr;
+  ;
+  return 0;
+}
+_ACEOF
+rm -f conftest.$ac_objext conftest$ac_exeext
+if { (eval echo "$as_me:$LINENO: \"$ac_link\"") >&5
+  (eval $ac_link) 2>conftest.er1
+  ac_status=$?
+  grep -v '^ *+' conftest.er1 >conftest.err
+  rm -f conftest.er1
+  cat conftest.err >&5
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); } &&
+	 { ac_try='test -z "$ac_c_werror_flag"
+			 || test ! -s conftest.err'
+  { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); }; } &&
+	 { ac_try='test -s conftest$ac_exeext'
+  { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); }; }; then
+  ac_cv_func_memrchr=yes
+else
+  echo "$as_me: failed program was:" >&5
+sed 's/^/| /' conftest.$ac_ext >&5
+
+ac_cv_func_memrchr=no
+fi
+rm -f conftest.err conftest.$ac_objext \
+      conftest$ac_exeext conftest.$ac_ext
+fi
+echo "$as_me:$LINENO: result: $ac_cv_func_memrchr" >&5
+echo "${ECHO_T}$ac_cv_func_memrchr" >&6
+if test $ac_cv_func_memrchr = yes; then
+
+cat >>confdefs.h <<\_ACEOF
+#define HAVE_MEMRCHR 1
+_ACEOF
+
+fi
+
+
 for ac_func in strftime
 do

configure.in

@@ -2490,6 +2490,8 @@ if test $ac_cv_func_memcmp_working = no ; then
 		[define if memcmp is not 8-bit clean or is otherwise broken])
 fi
 
+AC_CHECK_FUNC(memrchr, AC_DEFINE(HAVE_MEMRCHR,1,[if you have memrchr()]))
+
 AC_FUNC_STRFTIME
 
 OL_FUNC_INET_ATON

include/ac/string.h

@@ -94,6 +94,13 @@ int (strncasecmp)();
 #define memcmp lutil_memcmp
 #endif
 
+#ifndef HAVE_MEMRCHR
+	/* Actually, I think this is a GNU extension only */
+	void * lutil_memrchr(const void *b, int c, size_t len);
+#undef memrchr
+#define memrchr lutil_memrchr
+#endif /* ! HAVE_MEMRCHR */
+
 #define STRLENOF(s)	(sizeof(s)-1)
 
 #if defined( HAVE_NONPOSIX_STRERROR_R )

include/lber_pvt.h

@@ -169,6 +169,35 @@ ber_bvarray_add_x LDAP_P(( BerVarray *p, BerValue *bv, void *ctx ));
 #define ber_bvchr(bv,c) \
 	memchr( (bv)->bv_val, (c), (bv)->bv_len )
 
+#define ber_bvrchr(bv,c) \
+	memrchr( (bv)->bv_val, (c), (bv)->bv_len )
+
+#define ber_bvchr_right(dst,bv,c) \
+	do { \
+		(dst)->bv_val = memchr( (bv)->bv_val, (c), (bv)->bv_len ); \
+		(dst)->bv_len = (dst)->bv_val ? (bv)->bv_len - ((dst)->bv_val - (bv)->bv_val) : 0; \
+	} while (0)
+
+#define ber_bvchr_left(dst,bv,c) \
+	do { \
+		(dst)->bv_val = memchr( (bv)->bv_val, (c), (bv)->bv_len ); \
+		(dst)->bv_len = (dst)->bv_val ? ((dst)->bv_val - (bv)->bv_val) : (bv)->bv_len; \
+		(dst)->bv_val = (bv)->bv_val; \
+	} while (0)
+
+#define ber_bvrchr_right(dst,bv,c) \
+	do { \
+		(dst)->bv_val = memrchr( (bv)->bv_val, (c), (bv)->bv_len ); \
+		(dst)->bv_len = (dst)->bv_val ? (bv)->bv_len - ((dst)->bv_val - (bv)->bv_val) : 0; \
+	} while (0)
+
+#define ber_bvrchr_left(dst,bv,c) \
+	do { \
+		(dst)->bv_val = memrchr( (bv)->bv_val, (c), (bv)->bv_len ); \
+		(dst)->bv_len = (dst)->bv_val ? ((dst)->bv_val - (bv)->bv_val) : (bv)->bv_len; \
+		(dst)->bv_val = (bv)->bv_val; \
+	} while (0)
+
 #define BER_BVC(s)		{ STRLENOF(s), (s) }
 #define BER_BVNULL		{ 0L, NULL }
 #define BER_BVZERO(bv) \

include/portable.hin

@@ -397,6 +397,9 @@
 /* Define to 1 if you have the <memory.h> header file. */
 #undef HAVE_MEMORY_H
 
+/* if you have memrchr() */
+#undef HAVE_MEMRCHR
+
 /* Define to 1 if you have the `mkstemp' function. */
 #undef HAVE_MKSTEMP
 

libraries/liblutil/utils.c

@@ -310,3 +310,23 @@ int mkstemp( char * template )
 #endif
 }
 #endif
+
+/* 
+ * Memory Reverse Search
+ */
+void *
+lutil_memrchr(const void *b, int c, size_t n) 
+{
+	if (n != 0) {
+		const unsigned char *s;
+
+		for ( s = b + n; s-- > b; ) {
+			if ( *s == c ) {
+				return s;
+			}
+		}
+	}
+
+	return NULL;
+} 
+

servers/slapd/aci.c

@@ -1394,16 +1394,14 @@ OpenLDAPaciValidate(
 		struct berval	ocbv = BER_BVNULL,
 				atbv = BER_BVNULL;
 
-		ocbv.bv_val = strchr( type.bv_val, '/' );
-		if ( ocbv.bv_val != NULL
-			&& ( ocbv.bv_val - type.bv_val ) < type.bv_len )
-		{
+		ocbv.bv_val = ber_bvchr( &type, '/' );
+		if ( ocbv.bv_val != NULL ) {
 			ocbv.bv_val++;
+			ocbv.bv_len = type.bv_len
+					- ( ocbv.bv_val - type.bv_val );
 
-			atbv.bv_val = strchr( ocbv.bv_val, '/' );
-			if ( atbv.bv_val != NULL
-				&& ( atbv.bv_val - ocbv.bv_val ) < ocbv.bv_len )
-			{
+			atbv.bv_val = ber_bvchr( &ocbv, '/' );
+			if ( atbv.bv_val != NULL ) {
 				AttributeDescription	*ad = NULL;
 				const char		*text = NULL;
 				int			rc;
@@ -1417,10 +1415,6 @@ OpenLDAPaciValidate(
 				if ( rc != LDAP_SUCCESS ) {
 					return LDAP_INVALID_SYNTAX;
 				}
-				
-			} else {
-				ocbv.bv_len = type.bv_len
-					- ( ocbv.bv_val - type.bv_val );
 			}
 
 			if ( oc_bvfind( &ocbv ) == NULL ) {
@@ -1549,10 +1543,8 @@ OpenLDAPaciPrettyNormal(
 			struct berval	ocbv = BER_BVNULL,
 					atbv = BER_BVNULL;
 
-			ocbv.bv_val = strchr( type.bv_val, '/' );
-			if ( ocbv.bv_val != NULL
-				&& ( ocbv.bv_val - type.bv_val ) < type.bv_len )
-			{
+			ocbv.bv_val = ber_bvchr( &type, '/' );
+			if ( ocbv.bv_val != NULL ) {
 				ObjectClass		*oc = NULL;
 				AttributeDescription	*ad = NULL;
 				const char		*text = NULL;
@@ -1564,10 +1556,8 @@ OpenLDAPaciPrettyNormal(
 				ocbv.bv_val++;
 				ocbv.bv_len = type.bv_len - ( ocbv.bv_val - type.bv_val );
 
-				atbv.bv_val = strchr( ocbv.bv_val, '/' );
-				if ( atbv.bv_val != NULL
-					&& ( atbv.bv_val - ocbv.bv_val ) < ocbv.bv_len )
-				{
+				atbv.bv_val = ber_bvchr( &ocbv, '/' );
+				if ( atbv.bv_val != NULL ) {
 					atbv.bv_val++;
 					atbv.bv_len = type.bv_len
 						- ( atbv.bv_val - type.bv_val );
@@ -1580,10 +1570,6 @@ OpenLDAPaciPrettyNormal(
 					}
 
 					bv.bv_len += STRLENOF( "/" ) + ad->ad_cname.bv_len;
-					
-				} else {
-					ocbv.bv_len = type.bv_len
-						- ( ocbv.bv_val - type.bv_val );
 				}
 
 				oc = oc_bvfind( &ocbv );

servers/slapd/ad.c

@@ -177,7 +177,7 @@ int slap_bv2ad(
 	memset( &desc, 0, sizeof( desc ) );
 	desc.ad_cname = *bv;
 	name = bv->bv_val;
-	options = strchr( name, ';' );
+	options = ber_bvchr( bv, ';' );
 	if ( options != NULL && (unsigned) ( options - name ) < bv->bv_len ) {
 		/* don't go past the end of the berval! */
 		desc.ad_cname.bv_len = options - name;

servers/slapd/add.c

@@ -664,7 +664,7 @@ int slap_add_opattrs(
 		} else {
 			csn = op->o_csn;
 		}
-		ptr = strchr( csn.bv_val, '#' );
+		ptr = ber_bvchr( &csn, '#' );
 		if ( ptr ) {
 			timestamp.bv_len = ptr - csn.bv_val;
 			if ( timestamp.bv_len >= sizeof(timebuf) )

servers/slapd/back-bdb/cache.c

@@ -899,7 +899,8 @@ bdb_cache_add(
 
 #ifdef BDB_HIER
 	if ( nrdn->bv_len != e->e_nname.bv_len ) {
-		char *ptr = strchr( rdn.bv_val, ',' );
+		char *ptr = ber_bvchr( &rdn, ',' );
+		assert( ptr != NULL );
 		rdn.bv_len = ptr - rdn.bv_val;
 	}
 	ber_dupbv( &ei.bei_rdn, &rdn );
@@ -1012,7 +1013,8 @@ bdb_cache_modrdn(
 
 	rdn = e->e_name;
 	if ( nrdn->bv_len != e->e_nname.bv_len ) {
-		char *ptr = strchr(rdn.bv_val, ',');
+		char *ptr = ber_bvchr(&rdn, ',');
+		assert( ptr != NULL );
 		rdn.bv_len = ptr - rdn.bv_val;
 	}
 	ber_dupbv( &ei->bei_rdn, &rdn );

servers/slapd/back-passwd/search.c

@@ -324,10 +324,10 @@ pw2entry( Backend *be, struct passwd *pw, Entry *e )
 		ber_str2bv( pw->pw_gecos, 0, 0, &val );
 		attr_merge_normalize_one( e, ad_desc, &val, NULL );
 
-		s = strchr( val.bv_val, ',' );
+		s = ber_bvchr( &val, ',' );
 		if ( s ) *s = '\0';
 
-		s = strchr( val.bv_val, '&' );
+		s = ber_bvchr( &val, '&' );
 		if ( s ) {
 			char buf[1024];
 

servers/slapd/bconfig.c

@@ -1324,7 +1324,7 @@ config_generic(ConfigArgs *c) {
 				/* quote all args but the first */
 				line = ldap_charray2str( c->argv, "\" \"" );
 				ber_str2bv( line, 0, 0, &bv );
-				s = strchr( bv.bv_val, '"' );
+				s = ber_bvchr( &bv, '"' );
 				assert( s != NULL );
 				/* move the trailing quote of argv[0] to the end */
 				AC_MEMCPY( s, s + 1, bv.bv_len - ( s - bv.bv_val ) );
@@ -3162,7 +3162,7 @@ check_name_index( CfEntryInfo *parent, ConfigType ce_type, Entry *e,
 
 	/* See if the rdn has an index already */
 	dnRdn( &e->e_name, &rdn );
-	ptr1 = strchr( e->e_name.bv_val, '{' );
+	ptr1 = ber_bvchr( &e->e_name, '{' );
 	if ( ptr1 && ptr1 - e->e_name.bv_val < rdn.bv_len ) {
 		ptr2 = strchr( ptr1, '}' );
 		if (!ptr2 || ptr2 - e->e_name.bv_val > rdn.bv_len)

servers/slapd/dn.c

@@ -1129,7 +1129,7 @@ dnParent(
 {
 	char	*p;
 
-	p = strchr( dn->bv_val, ',' );
+	p = ber_bvchr( dn, ',' );
 
 	/* one-level dn */
 	if ( p == NULL ) {
@@ -1161,7 +1161,7 @@ dnRdn(
 	char	*p;
 
 	*rdn = *dn;
-	p = strchr( dn->bv_val, ',' );
+	p = ber_bvchr( dn, ',' );
 
 	/* one-level dn */
 	if ( p == NULL ) {
@@ -1228,7 +1228,7 @@ dn_rdnlen(
 		return 0;
 	}
 
-	p = strchr( dn_in->bv_val, ',' );
+	p = ber_bvchr( dn_in, ',' );
 
 	return p ? p - dn_in->bv_val : dn_in->bv_len;
 }
@@ -1252,7 +1252,7 @@ rdn_validate( struct berval *rdn )
 	{
 		return LDAP_INVALID_SYNTAX;
 	}
-	return strchr( rdn->bv_val, ',' ) == NULL
+	return ber_bvchr( rdn, ',' ) == NULL
 		? LDAP_SUCCESS : LDAP_INVALID_SYNTAX;
 
 #else

servers/slapd/ldapsync.c

@@ -97,20 +97,26 @@ slap_parse_sync_cookie(
 	int valid = 0;
 	char *rid_ptr;
 	char *cval;
+	char *next;
 
 	if ( cookie == NULL )
 		return -1;
 
+	if ( cookie->octet_str.bv_len <= STRLENOF( "rid=" ) )
+		return -1;
+
 	cookie->rid = -1;
-	if (( rid_ptr = strstr( cookie->octet_str.bv_val, "rid=" )) != NULL ) {
-		if ( (cval = strchr( rid_ptr, ',' )) != NULL ) {
-			*cval = '\0';
-		}
-		cookie->rid = atoi( rid_ptr + sizeof("rid=") - 1 );
-		if ( cval != NULL ) {
-			*cval = ',';
-		}
-	} else {
+	/* FIXME: may read past end of cookie->octet_str.bv_val */
+	rid_ptr = strstr( cookie->octet_str.bv_val, "rid=" );
+	if ( rid_ptr == NULL 
+		|| rid_ptr > &cookie->octet_str.bv_val[ cookie->octet_str.bv_len - STRLENOF( "rid=" ) ] )
+	{
+		return -1;
+
+	}
+
+	cookie->rid = strtoul( &rid_ptr[ STRLENOF( "rid=" ) ], &next, 10 );
+	if ( next == &rid_ptr[ STRLENOF( "rid=" ) ] || ( next[ 0 ] != ',' && next[ 0 ] != '\0' ) ) {
 		return -1;
 	}
 
@@ -123,16 +129,20 @@ slap_parse_sync_cookie(
 		if ( ad == NULL )
 			break;
 
+		if ( csn_ptr >= &cookie->octet_str.bv_val[ cookie->octet_str.bv_len - STRLENOF( "csn=" ) ] ) {
+			return -1;
+		}
+
 		csn_str = csn_ptr + STRLENOF("csn=");
 		cval = strchr( csn_str, ',' );
-		if ( cval )
+		if ( cval && cval < &cookie->octet_str.bv_val[ cookie->octet_str.bv_len ] )
 			csn_str_len = cval - csn_str;
 		else
 			csn_str_len = 0;
 
 		/* FIXME use csnValidate when it gets implemented */
 		csn_ptr = strchr( csn_str, '#' );
-		if ( !csn_ptr ) break;
+		if ( !csn_ptr || csn_str >= &cookie->octet_str.bv_val[ cookie->octet_str.bv_len ] ) break;
 
 		stamp.bv_val = csn_str;
 		stamp.bv_len = csn_ptr - csn_str;

servers/slapd/modify.c

@@ -829,7 +829,7 @@ void slap_mods_opattrs(
 		} else {
 			csn = op->o_csn;
 		}
-		ptr = strchr( csn.bv_val, '#' );
+		ptr = ber_bvchr( &csn, '#' );
 		if ( ptr && ptr < &csn.bv_val[csn.bv_len] ) {
 			timestamp.bv_len = ptr - csn.bv_val;
 			if ( timestamp.bv_len >= sizeof( timebuf ))

servers/slapd/overlays/translucent.c

@@ -63,18 +63,12 @@ void glue_parent(Operation *op) {
 	Operation nop = *op;
 	slap_overinst *on = (slap_overinst *) op->o_bd->bd_info;
 	struct berval dn = { 0, NULL };
-	char *odn = op->o_req_ndn.bv_val;
 	Attribute *a;
 	Entry *e;
-	int idn, ldn;
-
-	/* tis more work to use strchr() for a berval... */
-	for(idn = 0; odn[idn] && odn[idn] != ','; idn++);
-	if(!idn || !odn[idn]) return;   /* because you never know */
-	idn++;
-	ldn = dn.bv_len = op->o_req_ndn.bv_len - idn;
-	dn.bv_val = ch_malloc(ldn + 1);
-	strcpy(dn.bv_val, odn + idn);
+	struct berval	pdn;
+
+	dnParent( &op->o_req_ndn, &pdn );
+	ber_dupbv( &dn, &pdn );
 
 	Debug(LDAP_DEBUG_TRACE, "=> glue_parent: fabricating glue for <%s>\n", dn.bv_val, 0, 0);
 

servers/slapd/overlays/valsort.c

@@ -314,7 +314,7 @@ valsort_response( Operation *op, SlapReply *rs )
 			gotnvals = (a->a_vals != a->a_nvals );
 
 			for (i=0; i<n; i++) {
-				char *ptr = strchr( a->a_nvals[i].bv_val, '{' );
+				char *ptr = ber_bvchr( &a->a_nvals[i], '{' );
 				char *end = NULL;
 				if ( !ptr ) {
 					Debug(LDAP_DEBUG_TRACE, "weights missing from attr %s "
@@ -339,7 +339,8 @@ valsort_response( Operation *op, SlapReply *rs )
 
 				if ( a->a_vals != a->a_nvals ) {
 					ptr = a->a_vals[i].bv_val;
-					end = strchr( ptr, '}' ) + 1;
+					end = ber_bvchr( &a->a_vals[i], '}' ) + 1;
+					assert( end != NULL );
 					for (;*end;)
 						*ptr++ = *end++;
 					*ptr = '\0';
@@ -407,7 +408,7 @@ valsort_add( Operation *op, SlapReply *rs )
 		if ( !a )
 			continue;
 		for (i=0; !BER_BVISNULL( &a->a_vals[i] ); i++) {
-			ptr = strchr(a->a_vals[i].bv_val, '{' );
+			ptr = ber_bvchr(&a->a_vals[i], '{' );
 			if ( !ptr ) {
 				Debug(LDAP_DEBUG_TRACE, "weight missing from attribute %s\n",
 					vi->vi_ad->ad_cname.bv_val, 0, 0);
@@ -451,7 +452,7 @@ valsort_modify( Operation *op, SlapReply *rs )
 		if ( !ml )
 			continue;
 		for (i=0; !BER_BVISNULL( &ml->sml_values[i] ); i++) {
-			ptr = strchr(ml->sml_values[i].bv_val, '{' );
+			ptr = ber_bvchr(&ml->sml_values[i], '{' );
 			if ( !ptr ) {
 				Debug(LDAP_DEBUG_TRACE, "weight missing from attribute %s\n",
 					vi->vi_ad->ad_cname.bv_val, 0, 0);

servers/slapd/saslauthz.c

@@ -148,7 +148,7 @@ int slap_parse_user( struct berval *id, struct berval *user,
 	 *		u[.mech[/realm]]:user
 	 */
 	
-	user->bv_val = strchr( id->bv_val, ':' );
+	user->bv_val = ber_bvchr( id, ':' );
 	if ( BER_BVISNULL( user ) ) {
 		return LDAP_PROTOCOL_ERROR;
 	}
@@ -156,20 +156,19 @@ int slap_parse_user( struct berval *id, struct berval *user,
 	user->bv_val++;
 	user->bv_len = id->bv_len - ( user->bv_val - id->bv_val );
 
-	mech->bv_val = strchr( id->bv_val, '.' );
+	mech->bv_val = ber_bvchr( id, '.' );
 	if ( !BER_BVISNULL( mech ) ) {
 		mech->bv_val[ 0 ] = '\0';
 		mech->bv_val++;
+		mech->bv_len = user->bv_val - mech->bv_val - 1;
 
-		realm->bv_val = strchr( mech->bv_val, '/' );
+		realm->bv_val = ber_bvchr( mech, '/' );
 
 		if ( !BER_BVISNULL( realm ) ) {
 			realm->bv_val[ 0 ] = '\0';
 			realm->bv_val++;
 			mech->bv_len = realm->bv_val - mech->bv_val - 1;
 			realm->bv_len = user->bv_val - realm->bv_val - 1;
-		} else {
-			mech->bv_len = user->bv_val - mech->bv_val - 1;
 		}
 
 	} else {
@@ -341,7 +340,8 @@ is_dn:		bv.bv_len = in->bv_len - ( bv.bv_val - in->bv_val );
 				member_at = BER_BVNULL;
 
 		bv.bv_val = in->bv_val + STRLENOF( "group" );
-		group_dn.bv_val = strchr( bv.bv_val, ':' );
+		bv.bv_len = in->bv_len - STRLENOF( "group" );
+		group_dn.bv_val = ber_bvchr( &bv, ':' );
 		if ( group_dn.bv_val == NULL ) {
 			/* last chance: assume it's a(n exact) DN ... */
 			bv.bv_val = in->bv_val;
@@ -355,8 +355,9 @@ is_dn:		bv.bv_len = in->bv_len - ( bv.bv_val - in->bv_val );
 		 */
 		if ( bv.bv_val[ 0 ] == '/' ) {
 			group_oc.bv_val = &bv.bv_val[ 1 ];
+			group_oc.bv_len = group_dn.bv_val - group_oc.bv_val;
 
-			member_at.bv_val = strchr( group_oc.bv_val, '/' );
+			member_at.bv_val = ber_bvchr( &group_oc, '/' );
 			if ( member_at.bv_val ) {
 				AttributeDescription	*ad = NULL;
 				const char		*text = NULL;
@@ -368,13 +369,10 @@ is_dn:		bv.bv_len = in->bv_len - ( bv.bv_val - in->bv_val );
 				if ( rc != LDAP_SUCCESS ) {
 					return rc;
 				}
+			}
 
-			} else {
-				group_oc.bv_len = group_dn.bv_val - group_oc.bv_val;
-
-				if ( oc_bvfind( &group_oc ) == NULL ) {
-					return LDAP_INVALID_SYNTAX;
-				}
+			if ( oc_bvfind( &group_oc ) == NULL ) {
+				return LDAP_INVALID_SYNTAX;
 			}
 		}
 
@@ -668,7 +666,8 @@ is_dn:		bv.bv_len = val->bv_len - ( bv.bv_val - val->bv_val );
 		char		*ptr;
 
 		bv.bv_val = val->bv_val + STRLENOF( "group" );
-		group_dn.bv_val = strchr( bv.bv_val, ':' );
+		bv.bv_len = val->bv_len - STRLENOF( "group" );
+		group_dn.bv_val = ber_bvchr( &bv, ':' );
 		if ( group_dn.bv_val == NULL ) {
 			/* last chance: assume it's a(n exact) DN ... */
 			bv.bv_val = val->bv_val;
@@ -681,9 +680,12 @@ is_dn:		bv.bv_len = val->bv_len - ( bv.bv_val - val->bv_val );
 		 * are present in schema...
 		 */
 		if ( bv.bv_val[ 0 ] == '/' ) {
+			ObjectClass		*oc = NULL;
+
 			group_oc.bv_val = &bv.bv_val[ 1 ];
+			group_oc.bv_len = group_dn.bv_val - group_oc.bv_val;
 
-			member_at.bv_val = strchr( group_oc.bv_val, '/' );
+			member_at.bv_val = ber_bvchr( &group_oc, '/' );
 			if ( member_at.bv_val ) {
 				AttributeDescription	*ad = NULL;
 				const char		*text = NULL;
@@ -698,18 +700,14 @@ is_dn:		bv.bv_len = val->bv_len - ( bv.bv_val - val->bv_val );
 
 				member_at = ad->ad_cname;
 
-			} else {
-				ObjectClass		*oc = NULL;
-
-				group_oc.bv_len = group_dn.bv_val - group_oc.bv_val;