From: Pierangelo Masarati <ando@openldap.org>
Date: Tue, 20 Dec 2005 14:31:16 +0000 (+0000)
Subject: fix potential overflow in syncrepl_unbind() and related helpers
X-Git-Tag: OPENLDAP_REL_ENG_2_4_BP~542
X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=81bafb9935587a8b6e1faaf4e5a3535482d49df0;p=openldap

fix potential overflow in syncrepl_unbind() and related helpers
---

diff --git a/servers/slapd/bconfig.c b/servers/slapd/bconfig.c
index c500b9971f..613358b859 100644
--- a/servers/slapd/bconfig.c
+++ b/servers/slapd/bconfig.c
@@ -2401,10 +2401,14 @@ config_security(ConfigArgs *c) {
 }
 
 char *
-anlist_unparse( AttributeName *an, char *ptr ) {
+anlist_unparse( AttributeName *an, char *ptr, ber_len_t buflen ) {
 	int comma = 0;
+	char *start = ptr;
 
 	for (; !BER_BVISNULL( &an->an_name ); an++) {
+		/* if buflen == 0, assume the buffer size has been 
+		 * already checked otherwise */
+		if ( buflen > 0 && buflen - ( ptr - start ) < comma + an->an_name.bv_len ) return NULL;
 		if ( comma ) *ptr++ = ',';
 		ptr = lutil_strcopy( ptr, an->an_name.bv_val );
 		comma = 1;
@@ -2468,7 +2472,7 @@ replica_unparse( struct slap_replica_info *ri, int i, struct berval *bv )
 		ptr = lutil_strcopy( ptr, " attrs" );
 		if ( ri->ri_exclude ) *ptr++ = '!';
 		*ptr++ = '=';
-		ptr = anlist_unparse( ri->ri_attrs, ptr );
+		ptr = anlist_unparse( ri->ri_attrs, ptr, 0 );
 	}
 }
 
diff --git a/servers/slapd/proto-slap.h b/servers/slapd/proto-slap.h
index ab6fda8196..174f406560 100644
--- a/servers/slapd/proto-slap.h
+++ b/servers/slapd/proto-slap.h
@@ -618,7 +618,7 @@ LDAP_SLAPD_F (int) bindconf_unparse LDAP_P((
 LDAP_SLAPD_F (void) bindconf_free LDAP_P(( slap_bindconf *bc ));
 LDAP_SLAPD_F (int) config_generic_wrapper LDAP_P(( Backend *be,
 	const char *fname, int lineno, int argc, char **argv ));
-LDAP_SLAPD_F (char *) anlist_unparse LDAP_P(( AttributeName *, char * ));
+LDAP_SLAPD_F (char *) anlist_unparse LDAP_P(( AttributeName *, char *, ber_len_t buflen ));
 
 #ifdef LDAP_SLAPI
 LDAP_SLAPD_V (int) slapi_plugins_used;
diff --git a/servers/slapd/syncrepl.c b/servers/slapd/syncrepl.c
index 04e1010ef2..60e15f5e25 100644
--- a/servers/slapd/syncrepl.c
+++ b/servers/slapd/syncrepl.c
@@ -3187,7 +3187,9 @@ syncrepl_unparse( syncinfo_t *si, struct berval *bv )
 	char buf[BUFSIZ*2], *ptr;
 	int i;
 
-	/* FIXME: we're not checking for buf[] overflow! */
+#define WHATSLEFT	( sizeof( buf ) - ( ptr - buf ) )
+
+	BER_BVZERO( bv );
 
 	/* temporarily inhibit bindconf from printing URI */
 	uri = si->si_bindconf.sb_uri;
@@ -3196,68 +3198,91 @@ syncrepl_unparse( syncinfo_t *si, struct berval *bv )
 	si->si_bindconf.sb_uri = uri;
 
 	ptr = buf;
-	ptr += snprintf( ptr, sizeof( buf ), IDSTR "=%03ld " PROVIDERSTR "=%s",
+	ptr += snprintf( ptr, WHATSLEFT, IDSTR "=%03ld " PROVIDERSTR "=%s",
 		si->si_rid, si->si_bindconf.sb_uri.bv_val );
+	if ( ptr - buf >= sizeof( buf ) ) return;
 	if ( !BER_BVISNULL( &bc )) {
+		if ( WHATSLEFT <= bc.bv_len ) {
+			free( bc.bv_val );
+			return;
+		}
 		ptr = lutil_strcopy( ptr, bc.bv_val );
 		free( bc.bv_val );
 	}
 	if ( !BER_BVISEMPTY( &si->si_filterstr )) {
+		if ( WHATSLEFT <= STRLENOF( " " FILTERSTR "=\"" "\"" ) + si->si_filterstr.bv_len ) return;
 		ptr = lutil_strcopy( ptr, " " FILTERSTR "=\"" );
 		ptr = lutil_strcopy( ptr, si->si_filterstr.bv_val );
 		*ptr++ = '"';
 	}
 	if ( !BER_BVISNULL( &si->si_base )) {
+		if ( WHATSLEFT <= STRLENOF( " " SEARCHBASESTR "=\"" "\"" ) + si->si_base.bv_len ) return;
 		ptr = lutil_strcopy( ptr, " " SEARCHBASESTR "=\"" );
 		ptr = lutil_strcopy( ptr, si->si_base.bv_val );
 		*ptr++ = '"';
 	}
 	if ( !BER_BVISEMPTY( &si->si_logfilterstr )) {
+		if ( WHATSLEFT <= STRLENOF( " " LOGFILTERSTR "=\"" "\"" ) + si->si_logfilterstr.bv_len ) return;
 		ptr = lutil_strcopy( ptr, " " LOGFILTERSTR "=\"" );
 		ptr = lutil_strcopy( ptr, si->si_logfilterstr.bv_val );
 		*ptr++ = '"';
 	}
 	if ( !BER_BVISNULL( &si->si_logbase )) {
+		if ( WHATSLEFT <= STRLENOF( " " LOGBASESTR "=\"" "\"" ) + si->si_logbase.bv_len ) return;
 		ptr = lutil_strcopy( ptr, " " LOGBASESTR "=\"" );
 		ptr = lutil_strcopy( ptr, si->si_logbase.bv_val );
 		*ptr++ = '"';
 	}
 	for (i=0; !BER_BVISNULL(&scopes[i].key);i++) {
 		if ( si->si_scope == scopes[i].val ) {
+			if ( WHATSLEFT <= STRLENOF( " " SCOPESTR "=" ) + scopes[i].key.bv_len ) return;
 			ptr = lutil_strcopy( ptr, " " SCOPESTR "=" );
 			ptr = lutil_strcopy( ptr, scopes[i].key.bv_val );
 			break;
 		}
 	}
 	if ( si->si_attrsonly ) {
+		if ( WHATSLEFT <= STRLENOF( " " ATTRSONLYSTR "=\"" "\"" ) ) return;
 		ptr = lutil_strcopy( ptr, " " ATTRSONLYSTR );
 	}
 	if ( si->si_anfile ) {
-		ptr = lutil_strcopy( ptr, " " ATTRSSTR "=:include:" );
+		if ( WHATSLEFT <= STRLENOF( " " ATTRSSTR "=\":include:" "\"" ) + strlen( si->si_anfile ) ) return;
+		ptr = lutil_strcopy( ptr, " " ATTRSSTR "=:include:\"" );
 		ptr = lutil_strcopy( ptr, si->si_anfile );
+		*ptr++ = '"';
 	} else if ( si->si_allattrs || si->si_allopattrs ||
-		( si->si_anlist && !BER_BVISNULL(&si->si_anlist[0].an_name) )) {
+		( si->si_anlist && !BER_BVISNULL(&si->si_anlist[0].an_name) ))
+	{
 		char *old;
+
+		if ( WHATSLEFT <= STRLENOF( " " ATTRSONLYSTR "=\"" "\"" ) ) return;
 		ptr = lutil_strcopy( ptr, " " ATTRSSTR "=\"" );
 		old = ptr;
-		ptr = anlist_unparse( si->si_anlist, ptr );
+		/* FIXME: add check for overflow */
+		ptr = anlist_unparse( si->si_anlist, ptr, WHATSLEFT );
 		if ( si->si_allattrs ) {
+			if ( WHATSLEFT <= STRLENOF( ",*\"" ) ) return;
 			if ( old != ptr ) *ptr++ = ',';
 			*ptr++ = '*';
 		}
 		if ( si->si_allopattrs ) {
+			if ( WHATSLEFT <= STRLENOF( ",+\"" ) ) return;
 			if ( old != ptr ) *ptr++ = ',';
 			*ptr++ = '+';
 		}
 		*ptr++ = '"';
 	}
 	if ( si->si_exanlist && !BER_BVISNULL(&si->si_exanlist[0].an_name) ) {
+		if ( WHATSLEFT <= STRLENOF( " " EXATTRSSTR "=" ) ) return;
 		ptr = lutil_strcopy( ptr, " " EXATTRSSTR "=" );
-		ptr = anlist_unparse( si->si_exanlist, ptr );
+		/* FIXME: add check for overflow */
+		ptr = anlist_unparse( si->si_exanlist, ptr, WHATSLEFT );
 	}
+	if ( WHATSLEFT <= STRLENOF( " " SCHEMASTR "=" ) + STRLENOF( "off" ) ) return;
 	ptr = lutil_strcopy( ptr, " " SCHEMASTR "=" );
 	ptr = lutil_strcopy( ptr, si->si_schemachecking ? "on" : "off" );
 	
+	if ( WHATSLEFT <= STRLENOF( " " TYPESTR "=" ) + STRLENOF( "refreshAndPersist" ) ) return;
 	ptr = lutil_strcopy( ptr, " " TYPESTR "=" );
 	ptr = lutil_strcopy( ptr, si->si_type == LDAP_SYNC_REFRESH_AND_PERSIST ?
 		"refreshAndPersist" : "refreshOnly" );
@@ -3273,34 +3298,40 @@ syncrepl_unparse( syncinfo_t *si, struct berval *bv )
 		hh = dd % 24;
 		dd /= 24;
 		ptr = lutil_strcopy( ptr, " " INTERVALSTR "=" );
-		ptr += sprintf( ptr, "%02d:%02d:%02d:%02d", dd, hh, mm, ss );
+		ptr += snprintf( ptr, WHATSLEFT, "%02d:%02d:%02d:%02d", dd, hh, mm, ss );
+		if ( ptr - buf >= sizeof( buf ) ) return;
 	} else if ( si->si_retryinterval ) {
 		int space=0;
+		if ( WHATSLEFT <= STRLENOF( " " RETRYSTR "=\"" "\"" ) ) return;
 		ptr = lutil_strcopy( ptr, " " RETRYSTR "=\"" );
 		for (i=0; si->si_retryinterval[i]; i++) {
 			if ( space ) *ptr++ = ' ';
 			space = 1;
-			ptr += sprintf( ptr, "%ld ", (long) si->si_retryinterval[i] );
+			ptr += snprintf( ptr, WHATSLEFT, "%ld ", (long) si->si_retryinterval[i] );
 			if ( si->si_retrynum_init[i] == RETRYNUM_FOREVER )
 				*ptr++ = '+';
 			else
-				ptr += sprintf( ptr, "%d", si->si_retrynum_init[i] );
+				ptr += snprintf( ptr, WHATSLEFT, "%d", si->si_retrynum_init[i] );
 		}
+		if ( WHATSLEFT <= STRLENOF( "\"" ) ) return;
 		*ptr++ = '"';
 	}
 
 	if ( si->si_slimit ) {
+		if ( WHATSLEFT <= STRLENOF( " " SLIMITSTR "=" ) ) return;
 		ptr = lutil_strcopy( ptr, " " SLIMITSTR "=" );
-		ptr += sprintf( ptr, "%d", si->si_slimit );
+		ptr += snprintf( ptr, WHATSLEFT, "%d", si->si_slimit );
 	}
 
 	if ( si->si_tlimit ) {
+		if ( WHATSLEFT <= STRLENOF( " " TLIMITSTR "=" ) ) return;
 		ptr = lutil_strcopy( ptr, " " TLIMITSTR "=" );
-		ptr += sprintf( ptr, "%d", si->si_tlimit );
+		ptr += snprintf( ptr, WHATSLEFT, "%d", si->si_tlimit );
 	}
 
 	if ( si->si_syncdata ) {
 		if ( enum_to_verb( datamodes, si->si_syncdata, &bc ) >= 0 ) {
+			if ( WHATSLEFT <= STRLENOF( " " SYNCDATASTR "=" ) + bc.bv_len ) return;
 			ptr = lutil_strcopy( ptr, " " SYNCDATASTR "=" );
 			ptr = lutil_strcopy( ptr, bc.bv_val );
 		}