Fix memory leak in get_sampwd_entries(), reindent for clarity.

 - call pdb_reset_sam() after each getent call.

Fix bug in get_group_alias_entries(), were if num_entries was zero this caused
talloc() to return NULL, failing a test below with NT_STATUS_NO_MEMORY.

Fix pdb_reset_sam() to correctly initalise the sam structure.

Move default value code into a single place, likewise for sam freeing code.
 - should make things easier if we decide to malloc other strings, or get more
 non-zero default values.

Finally, add a function in init a sam struct from a getpwnam() return.

Andrew Bartlett
(This used to be commit a41fb44f5e90cf8734d57217e836e14f4a80bd47)

2 files changed, 153 insertions, 74 deletions

diff --git a/source3/passdb/passdb.c b/source3/passdb/passdb.c
index a3ec590010..f02b1d2531 100644
--- a/source3/passdb/passdb.c
+++ b/source3/passdb/passdb.c
@@ -65,6 +65,36 @@ static void pdb_init_dispinfo(struct sam_disp_info *user)
 	ZERO_STRUCTP(user);
 }
 
+
+/************************************************************
+ Fill the SAM_ACCOUNT with default values.
+ ***********************************************************/
+
+static BOOL pdb_fill_default_sam(SAM_ACCOUNT *user)
+{
+	if (user == NULL) {
+		DEBUG(0,("pdb_fill_default_sam: SAM_ACCOUNT was NULL\n"));
+		return False;
+	}
+	
+	ZERO_STRUCTP(user);
+	(user)->logon_time            = (time_t)0;
+	(user)->logoff_time           = (time_t)-1;
+	(user)->kickoff_time          = (time_t)-1;
+	(user)->pass_last_set_time    = (time_t)-1;
+	(user)->pass_can_change_time  = (time_t)-1;
+	(user)->pass_must_change_time = (time_t)-1;
+
+	(user)->unknown_3 = 0x00ffffff; 	/* don't know */
+	(user)->logon_divs = 168; 	/* hours per week */
+	(user)->hours_len = 21; 		/* 21 times 8 bits = 168 */
+	memset((user)->hours, 0xff, (user)->hours_len); /* available at all hours */
+	(user)->unknown_5 = 0x00000000; /* don't know */
+	(user)->unknown_6 = 0x000004ec; /* don't know */
+	return True;
+}	
+
+
 /*************************************************************
  Alloc memory and initialises a struct sam_passwd.
  ************************************************************/
@@ -85,49 +115,69 @@ BOOL pdb_init_sam(SAM_ACCOUNT **user)
 		DEBUG(0,("pdb_init_sam: error while allocating memory\n"));
 		return False;
 	}
-	
-	ZERO_STRUCTP(*user);
-
-	(*user)->logon_time            = (time_t)0;
-	(*user)->logoff_time           = (time_t)-1;
-	(*user)->kickoff_time          = (time_t)-1;
-	(*user)->pass_last_set_time    = (time_t)-1;
-	(*user)->pass_can_change_time  = (time_t)-1;
-	(*user)->pass_must_change_time = (time_t)-1;
-
-	(*user)->unknown_3 = 0x00ffffff; 	/* don't know */
-	(*user)->logon_divs = 168; 	/* hours per week */
-	(*user)->hours_len = 21; 		/* 21 times 8 bits = 168 */
-	memset((*user)->hours, 0xff, (*user)->hours_len); /* available at all hours */
-	(*user)->unknown_5 = 0x00000000; /* don't know */
-	(*user)->unknown_6 = 0x000004ec; /* don't know */
-	
+
+	pdb_fill_default_sam(*user);
+
+	return True;
+}
+
+
+/*************************************************************
+ Initialises a struct sam_passwd with sane values.
+ ************************************************************/
+
+BOOL pdb_init_sam_pw(SAM_ACCOUNT **new_sam_acct, struct passwd *pwd)
+{
+	if (!pwd) {
+		new_sam_acct = NULL;
+		return False;
+	}
+
+	if (!pdb_init_sam(new_sam_acct)) {
+		new_sam_acct = NULL;
+		return False;
+	}
+
+	pdb_set_username(*new_sam_acct, pwd->pw_name);
+	pdb_set_fullname(*new_sam_acct, pwd->pw_gecos);
+	pdb_set_uid(*new_sam_acct, pwd->pw_uid);
+	pdb_set_gid(*new_sam_acct, pwd->pw_gid);
+	pdb_set_pass_last_set_time(*new_sam_acct, time(NULL));
+	pdb_set_profile_path(*new_sam_acct, lp_logon_path());
+	pdb_set_homedir(*new_sam_acct, lp_logon_home());
+	pdb_set_dir_drive(*new_sam_acct, lp_logon_drive());
+	pdb_set_logon_script(*new_sam_acct, lp_logon_script());
 	return True;
 }
 
+
 /************************************************************
- Free the SAM_ACCOUNT and the NT/LM hashes.
+ Free the NT/LM hashes only.
  ***********************************************************/
 
-BOOL pdb_free_sam(SAM_ACCOUNT *user)
+static BOOL pdb_free_sam_contents(SAM_ACCOUNT *user)
 {
 	if (user == NULL) {
-		DEBUG(0,("pdb_free_sam: SAM_ACCOUNT was NULL\n"));
+		DEBUG(0,("pdb_free_sam_contents: SAM_ACCOUNT was NULL\n"));
 #if 0
 		smb_panic("NULL pointer passed to pdb_free_sam\n");
 #endif
 		return False;
 	}
 
+	/* As we start mallocing more strings this is where  
+	   we should free them. */
+
 	SAFE_FREE(user->nt_pw);
 	SAFE_FREE(user->lm_pw);
-	SAFE_FREE(user);
 	
 	return True;	
 }
 
+
 /************************************************************
- Reset the SAM_ACCOUNT and the NT/LM hashes.
+ Reset the SAM_ACCOUNT and free the NT/LM hashes.
+  - note: they are not zero'ed out however.
  ***********************************************************/
 
 BOOL pdb_reset_sam(SAM_ACCOUNT *user)
@@ -137,13 +187,42 @@ BOOL pdb_reset_sam(SAM_ACCOUNT *user)
 		return False;
 	}
 	
-	SAFE_FREE(user->nt_pw);
-	SAFE_FREE(user->lm_pw);
-	ZERO_STRUCTP(user);
+	if (!pdb_free_sam_contents(user)) {
+		return False;
+	}
+
+	if (!pdb_fill_default_sam(user)) {
+		return False;
+	}
 
 	return True;
 }
 
+
+/************************************************************
+ Free the SAM_ACCOUNT and the NT/LM hashes.
+ ***********************************************************/
+
+BOOL pdb_free_sam(SAM_ACCOUNT *user)
+{
+	if (user == NULL) {
+		DEBUG(0,("pdb_free_sam: SAM_ACCOUNT was NULL\n"));
+#if 0
+		smb_panic("NULL pointer passed to pdb_free_sam\n");
+#endif
+		return False;
+	}
+
+	if (!pdb_free_sam_contents(user)) {
+		return False;
+	}
+
+	SAFE_FREE(user);
+	
+	return True;	
+}
+
+
 /*************************************************************************
  Routine to return the next entry in the sam passwd list.
  *************************************************************************/
@@ -800,16 +879,9 @@ account without a valid local system user.\n", user_name);
 		/* create the SAM_ACCOUNT struct and call pdb_add_sam_account.
 		   Because the new_sam_pwd only exists in the scope of this function
 		   we will not allocate memory for members */
-		pdb_init_sam(&new_sam_acct);
-		pdb_set_username(new_sam_acct, user_name);
-		pdb_set_fullname(new_sam_acct, pwd->pw_gecos);
-		pdb_set_uid(new_sam_acct, pwd->pw_uid);
-		pdb_set_gid(new_sam_acct, pwd->pw_gid);
-		pdb_set_pass_last_set_time(new_sam_acct, time(NULL));
-		pdb_set_profile_path(new_sam_acct, lp_logon_path());
-		pdb_set_homedir(new_sam_acct, lp_logon_home());
-		pdb_set_dir_drive(new_sam_acct, lp_logon_drive());
-		pdb_set_logon_script(new_sam_acct, lp_logon_script());
+		if (!pdb_init_sam_pw(&new_sam_acct, pwd)) {
+			return False;
+		}
 
 		/* set account flags */
 		pdb_set_acct_ctrl(new_sam_acct,((local_flags & LOCAL_TRUST_ACCOUNT) ? ACB_WSTRUST : ACB_NORMAL) );
diff --git a/source3/rpc_server/srv_samr_nt.c b/source3/rpc_server/srv_samr_nt.c
index 4290e24395..f7e6317edf 100644
--- a/source3/rpc_server/srv_samr_nt.c
+++ b/source3/rpc_server/srv_samr_nt.c
@@ -106,44 +106,50 @@ static NTSTATUS get_sampwd_entries(SAM_USER_INFO_21 *pw_buf, int start_idx,
 		pdb_free_sam(pwd);
 		return NT_STATUS_ACCESS_DENIED;
 	}
-
+	
 	while (((not_finished = pdb_getsampwent(pwd)) != False) 
-		&& (*num_entries) < max_num_entries) 
+	       && (*num_entries) < max_num_entries) 
 	{
 	        int user_name_len;
-
+		
 	        if (start_idx > 0) {
-	            /* skip the requested number of entries.
-	               not very efficient, but hey...  */
-		start_idx--;
-		continue;
-        }
-
-	user_name_len = strlen(pdb_get_username(pwd))+1;
-	init_unistr2(&pw_buf[(*num_entries)].uni_user_name, pdb_get_username(pwd), user_name_len);
-	init_uni_hdr(&pw_buf[(*num_entries)].hdr_user_name, user_name_len);
-	pw_buf[(*num_entries)].user_rid = pwd->user_rid;
-	memset((char *)pw_buf[(*num_entries)].nt_pwd, '\0', 16);
 
-        /* Now check if the NT compatible password is available. */
-        if (pdb_get_nt_passwd(pwd))
-		memcpy( pw_buf[(*num_entries)].nt_pwd , pdb_get_nt_passwd(pwd), 16);
-
-        pw_buf[(*num_entries)].acb_info = pdb_get_acct_ctrl(pwd);
-
-        DEBUG(5, ("entry idx: %d user %s, rid 0x%x, acb %x",
-                  (*num_entries), pdb_get_username(pwd), pdb_get_user_rid(pwd), pdb_get_acct_ctrl(pwd) ));
+			pdb_reset_sam(pwd);
 
-        if (acb_mask == 0 || (pwd->acct_ctrl & acb_mask)) {
-		DEBUG(5,(" acb_mask %x accepts\n", acb_mask));
-		(*num_entries)++;
-        }
-        else
-		DEBUG(5,(" acb_mask %x rejects\n", acb_mask));
+			/* skip the requested number of entries.
+			   not very efficient, but hey...  */
+			start_idx--;
+			continue;
+		}
+		
+		user_name_len = strlen(pdb_get_username(pwd))+1;
+		init_unistr2(&pw_buf[(*num_entries)].uni_user_name, pdb_get_username(pwd), user_name_len);
+		init_uni_hdr(&pw_buf[(*num_entries)].hdr_user_name, user_name_len);
+		pw_buf[(*num_entries)].user_rid = pwd->user_rid;
+		memset((char *)pw_buf[(*num_entries)].nt_pwd, '\0', 16);
+		
+		/* Now check if the NT compatible password is available. */
+		if (pdb_get_nt_passwd(pwd))
+			memcpy( pw_buf[(*num_entries)].nt_pwd , pdb_get_nt_passwd(pwd), 16);
+		
+		pw_buf[(*num_entries)].acb_info = pdb_get_acct_ctrl(pwd);
+		
+		DEBUG(5, ("entry idx: %d user %s, rid 0x%x, acb %x",
+			  (*num_entries), pdb_get_username(pwd), pdb_get_user_rid(pwd), pdb_get_acct_ctrl(pwd) ));
+		
+		if (acb_mask == 0 || (pwd->acct_ctrl & acb_mask)) {
+			DEBUG(5,(" acb_mask %x accepts\n", acb_mask));
+			(*num_entries)++;
+		} else {
+			DEBUG(5,(" acb_mask %x rejects\n", acb_mask));
+		}
 
 		(*total_entries)++;
-	}
+		
+		pdb_reset_sam(pwd);
 
+	}
+	
 	pdb_endsampwent();
 	pdb_free_sam(pwd);
 
@@ -807,17 +813,18 @@ static NTSTATUS get_group_alias_entries(TALLOC_CTX *ctx, DOMAIN_GRP **d_grp, DOM
 	if (sid_equal(sid, &global_sid_Builtin) && !lp_hide_local_users()) {
 		
 		enum_group_mapping(SID_NAME_WKN_GRP, &map, (int *)&num_entries, ENUM_ALL_MAPPED);
-	
-		*d_grp=(DOMAIN_GRP *)talloc_zero(ctx, num_entries*sizeof(DOMAIN_GRP));
-		if (*d_grp==NULL)
-			return NT_STATUS_NO_MEMORY;
 		
-		for(i=0; i<num_entries && i<max_entries; i++) {
-			fstrcpy((*d_grp)[i].name, map[i+start_idx].nt_name);
-			sid_split_rid(&map[i].sid, &(*d_grp)[i].rid);
-
+		if (num_entries != 0) {
+			*d_grp=(DOMAIN_GRP *)talloc_zero(ctx, num_entries*sizeof(DOMAIN_GRP));
+			if (*d_grp==NULL)
+				return NT_STATUS_NO_MEMORY;
+			
+			for(i=0; i<num_entries && i<max_entries; i++) {
+				fstrcpy((*d_grp)[i].name, map[i+start_idx].nt_name);
+				sid_split_rid(&map[i].sid, &(*d_grp)[i].rid);
+				
+			}
 		}
-		
 		SAFE_FREE(map);
 		
 	} else if (sid_equal(sid, &global_sam_sid) && !lp_hide_local_users()) {
@@ -1343,7 +1350,7 @@ NTSTATUS _samr_chgpasswd_user(pipes_struct *p, SAMR_Q_CHGPASSWD_USER *q_u, SAMR_
     r_u->status = NT_STATUS_OK;
 
     rpcstr_pull(user_name, q_u->uni_user_name.buffer, sizeof(user_name), q_u->uni_user_name.uni_str_len*2, 0);
-    rpcstr_pull(wks, q_u->uni_dest_host.buffer, sizeof(wks), q_u->uni_dest_host.uni_str_len,0);
+    rpcstr_pull(wks, q_u->uni_dest_host.buffer, sizeof(wks), q_u->uni_dest_host.uni_str_len*2,0);
 
     DEBUG(5,("samr_chgpasswd_user: user: %s wks: %s\n", user_name, wks));