r20090: Fix a class of bugs found by James Peach. Ensure

we never mix malloc and talloc'ed contexts in the
add_XX_to_array() and add_XX_to_array_unique()
calls. Ensure that these calls always return
False on out of memory, True otherwise and always
check them. Ensure that the relevent parts of
the conn struct and the nt_user_tokens are
TALLOC_DESTROYED not SAFE_FREE'd.
James - this should fix your crash bug in both
branches.
Jeremy.
(This used to be commit 0ffca7559e07500bd09a64b775e230d448ce5c24)

5 files changed, 44 insertions, 46 deletions

diff --git a/source3/lib/privileges.c b/source3/lib/privileges.c
index 32535394c7..c0f7857c95 100644
--- a/source3/lib/privileges.c
+++ b/source3/lib/privileges.c
@@ -517,7 +517,9 @@ static int priv_traverse_fn(TDB_CONTEXT *t, TDB_DATA key, TDB_DATA data, void *s
 		return 0;
 	}
 
-	add_sid_to_array( NULL, &sid, &priv->sids.list, &priv->sids.count );
+	if (!add_sid_to_array( NULL, &sid, &priv->sids.list, &priv->sids.count )) {
+		return 0;
+	}
 	
 	return 0;
 }
diff --git a/source3/lib/system_smbd.c b/source3/lib/system_smbd.c
index fc506c901d..509b2bbcb1 100644
--- a/source3/lib/system_smbd.c
+++ b/source3/lib/system_smbd.c
@@ -181,11 +181,18 @@ BOOL getgroups_unix_user(TALLOC_CTX *mem_ctx, const char *user,
 	groups = NULL;
 
 	/* Add in primary group first */
-	add_gid_to_array_unique(mem_ctx, primary_gid, &groups, &ngrp);
+	if (!add_gid_to_array_unique(mem_ctx, primary_gid, &groups, &ngrp)) {
+		SAFE_FREE(temp_groups);
+		return False;
+	}
 
-	for (i=0; i<max_grp; i++)
-		add_gid_to_array_unique(mem_ctx, temp_groups[i],
-					&groups, &ngrp);
+	for (i=0; i<max_grp; i++) {
+		if (!add_gid_to_array_unique(mem_ctx, temp_groups[i],
+					&groups, &ngrp)) {
+			SAFE_FREE(temp_groups);
+			return False;
+		}
+	}
 
 	*p_ngroups = ngrp;
 	*ret_groups = groups;
diff --git a/source3/lib/util.c b/source3/lib/util.c
index 19c6cab5b2..d1801527e9 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -307,7 +307,7 @@ const char *tmpdir(void)
  Add a gid to an array of gids if it's not already there.
 ****************************************************************************/
 
-void add_gid_to_array_unique(TALLOC_CTX *mem_ctx, gid_t gid,
+BOOL add_gid_to_array_unique(TALLOC_CTX *mem_ctx, gid_t gid,
 			     gid_t **gids, size_t *num_gids)
 {
 	int i;
@@ -316,26 +316,24 @@ void add_gid_to_array_unique(TALLOC_CTX *mem_ctx, gid_t gid,
 		/*
 		 * A former call to this routine has failed to allocate memory
 		 */
-		return;
+		return False;
 	}
 
 	for (i=0; i<*num_gids; i++) {
-		if ((*gids)[i] == gid)
-			return;
-	}
-
-	if (mem_ctx != NULL) {
-		*gids = TALLOC_REALLOC_ARRAY(mem_ctx, *gids, gid_t, *num_gids+1);
-	} else {
-		*gids = SMB_REALLOC_ARRAY(*gids, gid_t, *num_gids+1);
+		if ((*gids)[i] == gid) {
+			return True;
+		}
 	}
 
+	*gids = TALLOC_REALLOC_ARRAY(mem_ctx, *gids, gid_t, *num_gids+1);
 	if (*gids == NULL) {
-		return;
+		*num_gids = 0;
+		return False;
 	}
 
 	(*gids)[*num_gids] = gid;
 	*num_gids += 1;
+	return True;
 }
 
 /****************************************************************************
@@ -1077,12 +1075,7 @@ void add_to_large_array(TALLOC_CTX *mem_ctx, size_t element_size,
 			goto error;
 		}
 
-		if (mem_ctx != NULL) {
-			*array = TALLOC(mem_ctx, element_size * (*array_size));
-		} else {
-			*array = SMB_MALLOC(element_size * (*array_size));
-		}
-
+		*array = TALLOC(mem_ctx, element_size * (*array_size));
 		if (*array == NULL) {
 			goto error;
 		}
@@ -1095,13 +1088,8 @@ void add_to_large_array(TALLOC_CTX *mem_ctx, size_t element_size,
 			goto error;
 		}
 
-		if (mem_ctx != NULL) {
-			*array = TALLOC_REALLOC(mem_ctx, *array,
-						element_size * (*array_size));
-		} else {
-			*array = SMB_REALLOC(*array,
-					     element_size * (*array_size));
-		}
+		*array = TALLOC_REALLOC(mem_ctx, *array,
+					element_size * (*array_size));
 
 		if (*array == NULL) {
 			goto error;
diff --git a/source3/lib/util_sid.c b/source3/lib/util_sid.c
index b6952fca81..032be9aa93 100644
--- a/source3/lib/util_sid.c
+++ b/source3/lib/util_sid.c
@@ -580,24 +580,20 @@ DOM_SID *sid_dup_talloc(TALLOC_CTX *ctx, const DOM_SID *src)
  Add SID to an array SIDs
 ********************************************************************/
 
-void add_sid_to_array(TALLOC_CTX *mem_ctx, const DOM_SID *sid, 
+BOOL add_sid_to_array(TALLOC_CTX *mem_ctx, const DOM_SID *sid, 
 		      DOM_SID **sids, size_t *num)
 {
-	if (mem_ctx != NULL) {
-		*sids = TALLOC_REALLOC_ARRAY(mem_ctx, *sids, DOM_SID,
+	*sids = TALLOC_REALLOC_ARRAY(mem_ctx, *sids, DOM_SID,
 					     (*num)+1);
-	} else {
-		*sids = SMB_REALLOC_ARRAY(*sids, DOM_SID, (*num)+1);
-	}
-
 	if (*sids == NULL) {
-		return;
+		*num = 0;
+		return False;
 	}
 
 	sid_copy(&((*sids)[*num]), sid);
 	*num += 1;
 
-	return;
+	return True;
 }
 
 
@@ -605,17 +601,17 @@ void add_sid_to_array(TALLOC_CTX *mem_ctx, const DOM_SID *sid,
  Add SID to an array SIDs ensuring that it is not already there
 ********************************************************************/
 
-void add_sid_to_array_unique(TALLOC_CTX *mem_ctx, const DOM_SID *sid,
+BOOL add_sid_to_array_unique(TALLOC_CTX *mem_ctx, const DOM_SID *sid,
 			     DOM_SID **sids, size_t *num_sids)
 {
 	size_t i;
 
 	for (i=0; i<(*num_sids); i++) {
 		if (sid_compare(sid, &(*sids)[i]) == 0)
-			return;
+			return True;
 	}
 
-	add_sid_to_array(mem_ctx, sid, sids, num_sids);
+	return add_sid_to_array(mem_ctx, sid, sids, num_sids);
 }
 
 /********************************************************************
@@ -647,23 +643,26 @@ void del_sid_from_array(const DOM_SID *sid, DOM_SID **sids, size_t *num)
 	return;
 }
 
-void add_rid_to_array_unique(TALLOC_CTX *mem_ctx,
+BOOL add_rid_to_array_unique(TALLOC_CTX *mem_ctx,
 				    uint32 rid, uint32 **pp_rids, size_t *p_num)
 {
 	size_t i;
 
 	for (i=0; i<*p_num; i++) {
 		if ((*pp_rids)[i] == rid)
-			return;
+			return True;
 	}
 	
 	*pp_rids = TALLOC_REALLOC_ARRAY(mem_ctx, *pp_rids, uint32, *p_num+1);
 
-	if (*pp_rids == NULL)
-		return;
+	if (*pp_rids == NULL) {
+		*p_num = 0;
+		return False;
+	}
 
 	(*pp_rids)[*p_num] = rid;
 	*p_num += 1;
+	return True;
 }
 
 BOOL is_null_sid(const DOM_SID *sid)
diff --git a/source3/lib/util_str.c b/source3/lib/util_str.c
index fc13b75cc5..cd52faa52d 100644
--- a/source3/lib/util_str.c
+++ b/source3/lib/util_str.c
@@ -2428,8 +2428,10 @@ BOOL add_string_to_array(TALLOC_CTX *mem_ctx,
 
 	*strings = TALLOC_REALLOC_ARRAY(mem_ctx, *strings, const char *, (*num)+1);
 
-	if ((*strings == NULL) || (dup_str == NULL))
+	if ((*strings == NULL) || (dup_str == NULL)) {
+		*num = 0;
 		return False;
+	}
 
 	(*strings)[*num] = dup_str;
 	*num += 1;