Bug(s) fixed: 179135

Bug Description: memory leaks using ber_scanf when handling bad BER packets
Reviewed by: All (Thanks!)
Files: https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=123783
Branch: HEAD
Fix Description:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=179135#c0
I basically did a search through our code for all calls to ber_scanf,
ber_get_stringa, and ber_get_stringal and made sure we properly free any
arguments that may have been allocated.  There was a bug in the ldapsdk
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=179135 that causes
us to free uninitialized memory when trying to clean up the result of
ber_get_stringal (or ber_scanf with 'V').  I had to initialize some
variables to NULL so that we could properly clean them up, and added
some additional clean ups that were missing.  Also, in repl_extop.c, we
were calling free on an array that we should have been calling
ch_array_free on.  Yet another lesson in the evils of slapi_ch_free and
disabling compiler type checks in general.
Platforms tested: Fedora Core 4
Flag Day: no
Doc impact: no

12 files changed, 51 insertions, 28 deletions

diff --git a/ldap/servers/plugins/replication/repl5_total.c b/ldap/servers/plugins/replication/repl5_total.c
index 65d719e4..a5cab31d 100644
--- a/ldap/servers/plugins/replication/repl5_total.c
+++ b/ldap/servers/plugins/replication/repl5_total.c
@@ -585,7 +585,7 @@ my_ber_scanf_attr (BerElement *ber, Slapi_Attr **attr, PRBool *deleted)
     char *lasti;
     unsigned long len;
 	unsigned long tag;
-    char *str;
+    char *str = NULL;
     int rc;
     Slapi_Value *value;
 
@@ -685,6 +685,9 @@ loser:
     if (value)
         slapi_value_free (&value);
 
+    slapi_ch_free_string(&attrtype);
+    slapi_ch_free_string(&str);
+
     return -1;    
 }
 
diff --git a/ldap/servers/plugins/replication/repl_controls.c b/ldap/servers/plugins/replication/repl_controls.c
index 51e9900c..2cf0f928 100644
--- a/ldap/servers/plugins/replication/repl_controls.c
+++ b/ldap/servers/plugins/replication/repl_controls.c
@@ -349,15 +349,15 @@ add_repl_control_mods( Slapi_PBlock *pb, Slapi_Mods *smods )
         		      emtag != LBER_ERROR && emtag != LBER_END_OF_SEQORSET;
         		      emtag = ber_next_element( ember, &emlen, emlast ))
 			    {
-        		    struct berval **embvals;
-        		    if ( ber_scanf( ember, "{i{a[V]}}", &op, &type, &embvals ) == LBER_ERROR )
+        		    struct berval **embvals = NULL;
+        		    type = NULL;
+        		    if ( ber_scanf( ember, "{i{a[V]}}", &op, &type, &embvals ) != LBER_ERROR )
 					{
-            			continue;
+        				slapi_mods_add_modbvps( smods, op, type, embvals);
 					/* GGOODREPL I suspect this will cause two sets of lastmods attr values
 						to end up in the entry. We need to remove the old ones.
 					*/
         		    }
-                    slapi_mods_add_modbvps( smods, op, type, embvals);
         		    free( type );
         		    ber_bvecfree( embvals );
         		}
diff --git a/ldap/servers/plugins/replication/repl_extop.c b/ldap/servers/plugins/replication/repl_extop.c
index 22b829f7..132657ab 100644
--- a/ldap/servers/plugins/replication/repl_extop.c
+++ b/ldap/servers/plugins/replication/repl_extop.c
@@ -384,7 +384,8 @@ free_and_return:
 		/* slapi_ch_free accepts NULL pointer */
 		slapi_ch_free ((void**)protocol_oid);
 		slapi_ch_free ((void**)repl_root);
-		slapi_ch_free ((void **)extra_referrals);
+		slapi_ch_array_free (*extra_referrals);
+        *extra_referrals = NULL;
 		slapi_ch_free ((void**)csnstr);
 
 		if (*supplier_ruv)
diff --git a/ldap/servers/slapd/add.c b/ldap/servers/slapd/add.c
index bdae324a..d8bfe328 100644
--- a/ldap/servers/slapd/add.c
+++ b/ldap/servers/slapd/add.c
@@ -102,8 +102,9 @@ do_add( Slapi_PBlock *pb )
 	 */
 	/* get the name */
 	{
-    	char *dn;
+    	char *dn = NULL;
     	if ( ber_scanf( ber, "{a", &dn ) == LBER_ERROR ) {
+            slapi_ch_free_string(&dn);
     		LDAPDebug( LDAP_DEBUG_ANY,
     		    "ber_scanf failed (op=Add; params=DN)\n", 0, 0, 0 );
 			op_shared_log_error_access (pb, "ADD", "???", "decoding error");
@@ -121,11 +122,13 @@ do_add( Slapi_PBlock *pb )
 	      tag != LBER_DEFAULT && tag != LBER_END_OF_SEQORSET;
 	      tag = ber_next_element( ber, &len, last ) ) {
 		char *type = NULL, *normtype = NULL;
-		struct berval	**vals;
+		struct berval	**vals = NULL;
 		if ( ber_scanf( ber, "{a{V}}", &type, &vals ) == LBER_ERROR ) {
 			op_shared_log_error_access (pb, "ADD", slapi_sdn_get_dn (slapi_entry_get_sdn_const(e)), "decoding error");
 			send_ldap_result( pb, LDAP_PROTOCOL_ERROR, NULL,
 			    "decoding error", 0, NULL );
+            slapi_ch_free_string(&type);
+            ber_bvecfree( vals );
 			goto free_and_return;
 		}
 
@@ -134,7 +137,7 @@ do_add( Slapi_PBlock *pb )
 			op_shared_log_error_access (pb, "ADD", slapi_sdn_get_dn (slapi_entry_get_sdn_const(e)), "null value");
 			send_ldap_result( pb, LDAP_PROTOCOL_ERROR, NULL, NULL,
 			    0, NULL );
-			free( type );			
+            slapi_ch_free_string(&type);
 			goto free_and_return;
 		}
 
@@ -144,7 +147,7 @@ do_add( Slapi_PBlock *pb )
 			PR_snprintf (ebuf, BUFSIZ, "invalid type '%s'", type);
 			op_shared_log_error_access (pb, "ADD", slapi_sdn_get_dn (slapi_entry_get_sdn_const(e)), ebuf);
 			send_ldap_result( pb, rc, NULL, ebuf, 0, NULL );
-			free( type );
+            slapi_ch_free_string(&type);
 			slapi_ch_free( (void**)&normtype );
 			ber_bvecfree( vals );
 			goto free_and_return;
diff --git a/ldap/servers/slapd/ava.c b/ldap/servers/slapd/ava.c
index a1974db8..7ea35f0a 100644
--- a/ldap/servers/slapd/ava.c
+++ b/ldap/servers/slapd/ava.c
@@ -53,10 +53,12 @@ get_ava(
     struct ava	*ava
 )
 {
-	char	*type;
+	char	*type = NULL;
 
 	if ( ber_scanf( ber, "{ao}", &type, &ava->ava_value )
 	    == LBER_ERROR ) {
+        slapi_ch_free_string(&type);
+        ava_done(ava);
 		LDAPDebug( LDAP_DEBUG_ANY, "  get_ava ber_scanf\n", 0, 0, 0 );
 		return( LDAP_PROTOCOL_ERROR );
 	}
diff --git a/ldap/servers/slapd/back-ldbm/sort.c b/ldap/servers/slapd/back-ldbm/sort.c
index 07de9e14..13e72d3a 100644
--- a/ldap/servers/slapd/back-ldbm/sort.c
+++ b/ldap/servers/slapd/back-ldbm/sort.c
@@ -384,6 +384,7 @@ int parse_sort_spec(struct berval *sort_spec_ber, sort_spec **ps)
 
 		return_value = ber_scanf(ber,"a",&rtype);
 		if (LBER_ERROR == return_value) {
+			slapi_ch_free_string(&rtype);
 			rc = LDAP_PROTOCOL_ERROR;
                         goto err;
 		}
diff --git a/ldap/servers/slapd/bind.c b/ldap/servers/slapd/bind.c
index df8f816a..c20d5082 100644
--- a/ldap/servers/slapd/bind.c
+++ b/ldap/servers/slapd/bind.c
@@ -111,7 +111,7 @@ do_bind( Slapi_PBlock *pb )
     long ber_version = -1;
     int		auth_response_requested = 0;
     int		pw_response_requested = 0;
-    char		*dn, *saslmech = NULL;
+    char		*dn = NULL, *saslmech = NULL;
     struct berval	cred = {0};
     Slapi_Backend		*be = NULL;
     unsigned long rc;
@@ -154,6 +154,7 @@ do_bind( Slapi_PBlock *pb )
         log_bind_access (pb, "???", method, version, saslmech, "decoding error");
         send_ldap_result( pb, LDAP_PROTOCOL_ERROR, NULL,
                           "decoding error", 0, NULL );
+        slapi_ch_free_string(&dn);
         return;
     }
 
diff --git a/ldap/servers/slapd/compare.c b/ldap/servers/slapd/compare.c
index e14dc490..8bd2f549 100644
--- a/ldap/servers/slapd/compare.c
+++ b/ldap/servers/slapd/compare.c
@@ -60,13 +60,13 @@ void
 do_compare( Slapi_PBlock *pb )
 {
 	BerElement	*ber = pb->pb_op->o_ber;
-	char		*dn;
-	struct ava	ava;
+	char		*dn = NULL;
+	struct ava	ava = {0};
 	Slapi_Backend		*be = NULL;
 	int		err;
 	char		ebuf[ BUFSIZ ];
 	Slapi_DN sdn;
-	Slapi_Entry *referral;
+	Slapi_Entry *referral = NULL;
 	char errorbuf[BUFSIZ];
 
 	LDAPDebug( LDAP_DEBUG_TRACE, "do_compare\n", 0, 0, 0 );
@@ -74,6 +74,9 @@ do_compare( Slapi_PBlock *pb )
 	/* count the compare request */
 	PR_AtomicIncrement(g_get_global_snmp_vars()->ops_tbl.dsCompareOps);
 
+    /* have to init this here so we can "done" it below if we short circuit */
+    slapi_sdn_init(&sdn);
+
 	/*
 	 * Parse the compare request.  It looks like this:
 	 *
@@ -86,7 +89,6 @@ do_compare( Slapi_PBlock *pb )
 	 *	}
 	 */
 
-
 	if ( ber_scanf( ber, "{a{ao}}", &dn, &ava.ava_type,
 	    &ava.ava_value ) == LBER_ERROR ) {
 		LDAPDebug( LDAP_DEBUG_ANY,
@@ -94,7 +96,7 @@ do_compare( Slapi_PBlock *pb )
 		    0, 0, 0 );
 		send_ldap_result( pb, LDAP_PROTOCOL_ERROR, NULL, NULL, 0,
 		    NULL );
-		return;
+		goto free_and_return;
 	}
 	/*
 	 * in LDAPv3 there can be optional control extensions on
@@ -106,6 +108,7 @@ do_compare( Slapi_PBlock *pb )
 		goto free_and_return;
 	}
 	slapi_sdn_init_dn_passin(&sdn,dn);
+    dn = NULL; /* do not free - sdn owns it now */
 
 	/* target spec is used to decide which plugins are applicable for the operation */
 	operation_set_target_spec (pb->pb_op, &sdn);
@@ -181,5 +184,6 @@ free_and_return:;
 	if (be)
 		slapi_be_Unlock(be);
 	slapi_sdn_done(&sdn);
+    slapi_ch_free_string(&dn);
 	ava_done( &ava );
 }
diff --git a/ldap/servers/slapd/delete.c b/ldap/servers/slapd/delete.c
index 44db6463..0e590957 100644
--- a/ldap/servers/slapd/delete.c
+++ b/ldap/servers/slapd/delete.c
@@ -66,7 +66,7 @@ do_delete( Slapi_PBlock *pb )
 {
 	Slapi_Operation *operation;
 	BerElement	*ber;
-	char	    *dn;
+	char	    *dn = NULL;
 	int			err;
 
 	LDAPDebug( LDAP_DEBUG_TRACE, "do_delete\n", 0, 0, 0 );
@@ -89,7 +89,7 @@ do_delete( Slapi_PBlock *pb )
 		op_shared_log_error_access (pb, "DEL", "???", "decoding error");
 		send_ldap_result( pb, LDAP_PROTOCOL_ERROR, NULL, NULL, 0,
 		    NULL );
-		return;
+		goto free_and_return;
 	}
 
 	/*
diff --git a/ldap/servers/slapd/filter.c b/ldap/servers/slapd/filter.c
index cd2c774a..eb2a4056 100644
--- a/ldap/servers/slapd/filter.c
+++ b/ldap/servers/slapd/filter.c
@@ -175,7 +175,7 @@ get_filter_internal( Connection *conn, BerElement *ber,
     unsigned long	len;
     int		err;
     struct slapi_filter	*f;
-    char		*ftmp, *type;
+    char		*ftmp, *type = NULL;
 
 	LDAPDebug( LDAP_DEBUG_FILTER, "=> get_filter_internal\n", 0, 0, 0 );
 
@@ -293,6 +293,7 @@ get_filter_internal( Connection *conn, BerElement *ber,
 	case LDAP_FILTER_PRESENT:
 		LDAPDebug( LDAP_DEBUG_FILTER, "PRESENT\n", 0, 0, 0 );
 		if ( ber_scanf( ber, "a", &type ) == LBER_ERROR ) {
+            slapi_ch_free_string(&type);
 			err = LDAP_PROTOCOL_ERROR;
 		} else {
 			err = LDAP_SUCCESS;
@@ -440,12 +441,13 @@ get_substring_filter(
 )
 {
 	unsigned long	tag, len, rc;
-	char		*val, *last, *type;
+	char		*val, *last, *type = NULL;
 	char		ebuf[BUFSIZ];
 
 	LDAPDebug( LDAP_DEBUG_FILTER, "=> get_substring_filter\n", 0, 0, 0 );
 
 	if ( ber_scanf( ber, "{a", &type ) == LBER_ERROR ) {
+        slapi_ch_free_string(&type);
 		return( LDAP_PROTOCOL_ERROR );
 	}
 	f->f_sub_type = slapi_attr_syntax_normalize( type );
@@ -460,8 +462,10 @@ get_substring_filter(
 	    tag != LBER_ERROR && tag != LBER_END_OF_SEQORSET;
 	    tag = ber_next_element( ber, &len, last ) )
 	{
+        val = NULL;
 		rc = ber_scanf( ber, "a", &val );
 		if ( rc == LBER_ERROR ) {
+            slapi_ch_free_string(&val);
 			return( LDAP_PROTOCOL_ERROR );
 		}
 		if ( val == NULL || *val == '\0' ) {
@@ -573,8 +577,9 @@ get_extensible_filter( BerElement *ber, mr_filter_t* mrf )
 				}
 			}
 			{
-			    char* type;
+			    char* type = NULL;
 			    if (ber_scanf( ber, "a", &type ) == LBER_ERROR) {
+				slapi_ch_free_string (&type);
 				rc = LDAP_PROTOCOL_ERROR;
 			    } else {
 				mrf->mrf_type = slapi_attr_syntax_normalize(type);
diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c
index 6d4d5ac9..9f572236 100644
--- a/ldap/servers/slapd/modify.c
+++ b/ldap/servers/slapd/modify.c
@@ -114,7 +114,7 @@ do_modify( Slapi_PBlock *pb )
 {
 	Slapi_Operation *operation;
 	BerElement			*ber;
-	char				*last, *type;
+	char				*last, *type = NULL;
 	unsigned long		tag, len;
 	LDAPMod				*mod;
 	LDAPMod			    **mods;
@@ -124,7 +124,7 @@ do_modify( Slapi_PBlock *pb )
 	int					ignored_some_mods = 0;
 	int                 has_password_mod = 0; /* number of password mods */
 	char				*old_pw = NULL;	/* remember the old password */
-	char				*dn;
+	char				*dn = NULL;
 
 	LDAPDebug( LDAP_DEBUG_TRACE, "do_modify\n", 0, 0, 0 );
 
@@ -161,6 +161,7 @@ do_modify( Slapi_PBlock *pb )
 			op_shared_log_error_access (pb, "MOD", "???", "decoding error");
     		send_ldap_result( pb, LDAP_PROTOCOL_ERROR, NULL, NULL, 0,
     		    NULL );
+    		slapi_ch_free_string(&dn);
     		return;
     	}
 	}
@@ -186,7 +187,9 @@ do_modify( Slapi_PBlock *pb )
 			op_shared_log_error_access (pb, "MOD", dn, "decoding error");
 			send_ldap_result( pb, LDAP_PROTOCOL_ERROR, NULL,
 							  "decoding error", 0, NULL );
+			ber_bvecfree(mod->mod_bvalues);
 			slapi_ch_free((void **)&mod);
+			slapi_ch_free_string(&type);
 			goto free_and_return;
 		}
 		mod->mod_op = long_mod_op;
diff --git a/ldap/servers/slapd/modrdn.c b/ldap/servers/slapd/modrdn.c
index 1f53a641..64ccccf4 100644
--- a/ldap/servers/slapd/modrdn.c
+++ b/ldap/servers/slapd/modrdn.c
@@ -66,10 +66,10 @@ do_modrdn( Slapi_PBlock *pb )
 {
 	Slapi_Operation *operation;
 	BerElement	*ber;
-	char		*dn, *newsuperior = NULL;
+	char		*dn = NULL, *newsuperior = NULL;
 	char        *newrdn = NULL;
-	int			err, deloldrdn;
-	unsigned long	len;
+	int			err = 0, deloldrdn = 0;
+	unsigned long	len = 0;
 
 	LDAPDebug( LDAP_DEBUG_TRACE, "do_modrdn\n", 0, 0, 0 );
 
@@ -99,7 +99,7 @@ do_modrdn( Slapi_PBlock *pb )
 		send_ldap_result( pb, LDAP_PROTOCOL_ERROR, NULL,
 		    "unable to decode DN, newRDN, or deleteOldRDN parameters",
 		    0, NULL );
-		return;
+        goto free_and_return;
 	}
 
 	if ( ber_peek_tag( ber, &len ) == LDAP_TAG_NEWSUPERIOR ) {