Fix up a number of intertwined issues:

The big one is a global change to allow us to NULLify the free'ed pointer to a
former passdb object.  This was done to allow idra's SAFE_FREE() macro to do
its magic, and to satisfy the input test in pdb_init_sam() for a NULL pointer
to start with.

This NULL pointer test was what was breaking the adding of accounts up until
now, and this code has been reworked to avoid duplicating work - I hope this
will avoid a similar mess-up in future.

Finally, I fixed a few nasty bugs where the pdb_ fuctions's return codes were
being ignored.  Some of these functions malloc() and are permitted to fail.
Also, this caught a nasty bug where pdb_set_lanman_password(sam, NULL) acheived
precisely didilly-squat, just returning False.  Now that we check the returns
this bug was spotted.  This could allow different LM and NT passwords.

 - the pdbedit code needs to start checking these too, but I havn't had a
chance to fix it.

I have also fixed up where some of the password changing code was using the
pdb_set functions to store *internal* data.  I assume this is from a previous
lot of mass conversion work...

Most likally (and going on past experience) I have missed somthing, probably in
the LanMan password change code which I havn't yet been able to test, but this
lot is in much better shape than it was before.

If all this is too much to swallow (particularly for 2.2.2) then just adding a
sam_pass = NULL to the particular line of passdb.c should do the trick for the
ovbious bug.

Andrew Bartlett
(This used to be commit 762c8758a7869809d89b4da9c2a5249678942930)

1 files changed, 71 insertions, 56 deletions

diff --git a/source3/rpc_server/srv_samr_nt.c b/source3/rpc_server/srv_samr_nt.c
index f7e6317edf..9748d0e950 100644
--- a/source3/rpc_server/srv_samr_nt.c
+++ b/source3/rpc_server/srv_samr_nt.c
@@ -103,7 +103,7 @@ static NTSTATUS get_sampwd_entries(SAM_USER_INFO_21 *pw_buf, int start_idx,
 
 	if (!pdb_setsampwent(False)) {
 		DEBUG(0, ("get_sampwd_entries: Unable to open passdb.\n"));
-		pdb_free_sam(pwd);
+		pdb_free_sam(&pwd);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 	
@@ -151,7 +151,7 @@ static NTSTATUS get_sampwd_entries(SAM_USER_INFO_21 *pw_buf, int start_idx,
 	}
 	
 	pdb_endsampwent();
-	pdb_free_sam(pwd);
+	pdb_free_sam(&pwd);
 
 	if (not_finished)
 		return STATUS_MORE_ENTRIES;
@@ -231,7 +231,7 @@ static NTSTATUS jf_get_sampwd_entries(SAM_USER_INFO_21 *pw_buf, int start_idx,
 
 	*total_entries = *num_entries;
 	
-	pdb_free_sam(pwd);
+	pdb_free_sam(&pwd);
 
 	if (not_finished)
 		return STATUS_MORE_ENTRIES;
@@ -1508,12 +1508,12 @@ NTSTATUS _api_samr_open_user(pipes_struct *p, SAMR_Q_OPEN_USER *q_u, SAMR_R_OPEN
 
 	/* check that the RID exists in our domain. */
 	if (ret == False) {
-		pdb_free_sam(sampass);
+		pdb_free_sam(&sampass);
         	return NT_STATUS_NO_SUCH_USER;
 	}
 
 	samr_clear_sam_passwd(sampass);
-	pdb_free_sam(sampass);
+	pdb_free_sam(&sampass);
 
 	/* Get the domain SID stored in the domain policy */
 	if(!get_lsa_policy_samr_sid(p, &domain_pol, &sid))
@@ -1559,7 +1559,7 @@ static BOOL get_user_info_10(SAM_USER_INFO_10 *id10, uint32 user_rid)
 
 	if (ret==False) {
 		DEBUG(4,("User 0x%x not found\n", user_rid));
-		pdb_free_sam(smbpass);
+		pdb_free_sam(&smbpass);
 		return False;
 	}
 
@@ -1569,7 +1569,7 @@ static BOOL get_user_info_10(SAM_USER_INFO_10 *id10, uint32 user_rid)
 	init_sam_user_info10(id10, pdb_get_acct_ctrl(smbpass) );
 
 	samr_clear_sam_passwd(smbpass);
-	pdb_free_sam(smbpass);
+	pdb_free_sam(&smbpass);
 
 	return True;
 }
@@ -1600,21 +1600,21 @@ static NTSTATUS get_user_info_12(pipes_struct *p, SAM_USER_INFO_12 * id12, uint3
 
 	if (ret == False) {
 		DEBUG(4, ("User 0x%x not found\n", user_rid));
-		pdb_free_sam(smbpass);
+		pdb_free_sam(&smbpass);
 		return (geteuid() == (uid_t)0) ? NT_STATUS_NO_SUCH_USER : NT_STATUS_ACCESS_DENIED;
 	}
 
 	DEBUG(3,("User:[%s] 0x%x\n", pdb_get_username(smbpass), pdb_get_acct_ctrl(smbpass) ));
 
 	if ( pdb_get_acct_ctrl(smbpass) & ACB_DISABLED) {
-		pdb_free_sam(smbpass);
+		pdb_free_sam(&smbpass);
 		return NT_STATUS_ACCOUNT_DISABLED;
 	}
 
 	ZERO_STRUCTP(id12);
 	init_sam_user_info12(id12, pdb_get_lanman_passwd(smbpass), pdb_get_nt_passwd(smbpass));
 	
-	pdb_free_sam(smbpass);
+	pdb_free_sam(&smbpass);
 
 	return NT_STATUS_OK;
 }
@@ -1641,7 +1641,7 @@ static BOOL get_user_info_20(SAM_USER_INFO_20 *id20, uint32 user_rid)
 
 	if (ret == False) {
 		DEBUG(4,("User 0x%x not found\n", user_rid));
-		pdb_free_sam(sampass);
+		pdb_free_sam(&sampass);
 		return False;
 	}
 
@@ -1652,7 +1652,7 @@ static BOOL get_user_info_20(SAM_USER_INFO_20 *id20, uint32 user_rid)
 	ZERO_STRUCTP(id20);
 	init_sam_user_info20A(id20, sampass);
 	
-	pdb_free_sam(sampass);
+	pdb_free_sam(&sampass);
 
 	return True;
 }
@@ -1679,7 +1679,7 @@ static BOOL get_user_info_21(SAM_USER_INFO_21 *id21, uint32 user_rid)
 
 	if (ret == False) {
 		DEBUG(4,("User 0x%x not found\n", user_rid));
-		pdb_free_sam(sampass);
+		pdb_free_sam(&sampass);
 		return False;
 	}
 
@@ -1690,7 +1690,7 @@ static BOOL get_user_info_21(SAM_USER_INFO_21 *id21, uint32 user_rid)
 	ZERO_STRUCTP(id21);
 	init_sam_user_info21A(id21, sampass);
 	
-	pdb_free_sam(sampass);
+	pdb_free_sam(&sampass);
 
 	return True;
 }
@@ -1946,7 +1946,7 @@ NTSTATUS _api_samr_create_user(pipes_struct *p, SAMR_Q_CREATE_USER *q_u, SAMR_R_
 	unbecome_root();
 	if (ret == True) {
 		/* this account exists: say so */
-		pdb_free_sam(sam_pass);
+		pdb_free_sam(&sam_pass);
 		return NT_STATUS_USER_EXISTS;
 	}
 
@@ -1982,7 +1982,7 @@ NTSTATUS _api_samr_create_user(pipes_struct *p, SAMR_Q_CREATE_USER *q_u, SAMR_R_
 		pstrcpy(add_script, lp_adduser_script());
 	} else {
 		DEBUG(0, ("_api_samr_create_user: mismatch between trust flags and $ termination\n"));
-		pdb_free_sam(sam_pass);
+		pdb_free_sam(&sam_pass);
 		return NT_STATUS_UNSUCCESSFUL;
 	}
 
@@ -1997,7 +1997,7 @@ NTSTATUS _api_samr_create_user(pipes_struct *p, SAMR_Q_CREATE_USER *q_u, SAMR_R_
 	if (!local_password_change(account, local_flags, NULL, err_str,
 	    sizeof(err_str), msg_str, sizeof(msg_str))) {
 		DEBUG(0, ("%s\n", err_str));
-		pdb_free_sam(sam_pass);
+		pdb_free_sam(&sam_pass);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
@@ -2006,25 +2006,25 @@ NTSTATUS _api_samr_create_user(pipes_struct *p, SAMR_Q_CREATE_USER *q_u, SAMR_R_
  	unbecome_root();
  	if (ret == False) {
 		/* account doesn't exist: say so */
-		pdb_free_sam(sam_pass);
+		pdb_free_sam(&sam_pass);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
 	/* Get the domain SID stored in the domain policy */
 	if(!get_lsa_policy_samr_sid(p, &dom_pol, &sid)) {
-		pdb_free_sam(sam_pass);
+		pdb_free_sam(&sam_pass);
 		return NT_STATUS_INVALID_HANDLE;
 	}
 
 	/* append the user's RID to it */
 	if(!sid_append_rid(&sid, pdb_get_user_rid(sam_pass) )) {
-		pdb_free_sam(sam_pass);
+		pdb_free_sam(&sam_pass);
 		return NT_STATUS_NO_SUCH_USER;
 	}
 
 	/* associate the user's SID with the new handle. */
 	if ((info = (struct samr_info *)malloc(sizeof(struct samr_info))) == NULL) {
-		pdb_free_sam(sam_pass);
+		pdb_free_sam(&sam_pass);
 		return NT_STATUS_NO_MEMORY;
 	}
 
@@ -2033,14 +2033,14 @@ NTSTATUS _api_samr_create_user(pipes_struct *p, SAMR_Q_CREATE_USER *q_u, SAMR_R_
 
 	/* get a (unique) handle.  open a policy on it. */
 	if (!create_policy_hnd(p, user_pol, free_samr_info, (void *)info)) {
-		pdb_free_sam(sam_pass);
+		pdb_free_sam(&sam_pass);
 		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
 	}
 
 	r_u->user_rid=sam_pass->user_rid;
 	r_u->unknown_0 = 0x000703ff;
 
-	pdb_free_sam(sam_pass);
+	pdb_free_sam(&sam_pass);
 
 	return NT_STATUS_OK;
 }
@@ -2236,24 +2236,27 @@ static BOOL set_user_info_10(const SAM_USER_INFO_10 *id10, uint32 rid)
 	ret = pdb_getsampwrid(pwd, rid);
 	
 	if(ret==False) {
-		pdb_free_sam(pwd);
+		pdb_free_sam(&pwd);
 		return False;
 	}
 
 	if (id10 == NULL) {
 		DEBUG(5, ("set_user_info_10: NULL id10\n"));
-		pdb_free_sam(pwd);
+		pdb_free_sam(&pwd);
 		return False;
 	}
 
-	pdb_set_acct_ctrl(pwd, id10->acb_info);
+	if (!pdb_set_acct_ctrl(pwd, id10->acb_info)) {
+		pdb_free_sam(&pwd);
+		return False;
+	}
 
 	if(!pdb_update_sam_account(pwd, True)) {
-		pdb_free_sam(pwd);
+		pdb_free_sam(&pwd);
 		return False;
 	}
 
-	pdb_free_sam(pwd);
+	pdb_free_sam(&pwd);
 
 	return True;
 }
@@ -2269,25 +2272,31 @@ static BOOL set_user_info_12(SAM_USER_INFO_12 *id12, uint32 rid)
 	pdb_init_sam(&pwd);
 
 	if(!pdb_getsampwrid(pwd, rid)) {
-		pdb_free_sam(pwd);
+		pdb_free_sam(&pwd);
 		return False;
 	}
 
 	if (id12 == NULL) {
 		DEBUG(2, ("set_user_info_12: id12 is NULL\n"));
-		pdb_free_sam(pwd);
+		pdb_free_sam(&pwd);
 		return False;
 	}
  
-	pdb_set_lanman_passwd (pwd, id12->lm_pwd);
-	pdb_set_nt_passwd     (pwd, id12->nt_pwd);
+	if (!pdb_set_lanman_passwd (pwd, id12->lm_pwd)) {
+		pdb_free_sam(&pwd);
+		return False;
+	}
+	if (!pdb_set_nt_passwd     (pwd, id12->nt_pwd)) {
+		pdb_free_sam(&pwd);
+		return False;
+	}
  
 	if(!pdb_update_sam_account(pwd, True)) {
-		pdb_free_sam(pwd);
+		pdb_free_sam(&pwd);
 		return False;
  	}
 
-	pdb_free_sam(pwd);
+	pdb_free_sam(&pwd);
 	return True;
 }
 
@@ -2309,8 +2318,8 @@ static BOOL set_user_info_21(SAM_USER_INFO_21 *id21, uint32 rid)
 	pdb_init_sam(&new_pwd);
  
 	if (!pdb_getsampwrid(pwd, rid)) {
-		pdb_free_sam(pwd);
-		pdb_free_sam(new_pwd);
+		pdb_free_sam(&pwd);
+		pdb_free_sam(&new_pwd);
 		return False;
 	}
  
@@ -2327,13 +2336,13 @@ static BOOL set_user_info_21(SAM_USER_INFO_21 *id21, uint32 rid)
  
 	/* write the change out */
 	if(!pdb_update_sam_account(new_pwd, True)) {
-		pdb_free_sam(pwd);
-		pdb_free_sam(new_pwd);
+		pdb_free_sam(&pwd);
+		pdb_free_sam(&new_pwd);
 		return False;
  	}
 
-	pdb_free_sam(pwd);
-	pdb_free_sam(new_pwd);
+	pdb_free_sam(&pwd);
+	pdb_free_sam(&new_pwd);
 
 	return True;
 }
@@ -2359,24 +2368,27 @@ static BOOL set_user_info_23(SAM_USER_INFO_23 *id23, uint32 rid)
 	pdb_init_sam(&new_pwd);
  
 	if (!pdb_getsampwrid(pwd, rid)) {
-		pdb_free_sam(pwd);
-		pdb_free_sam(new_pwd);
+		pdb_free_sam(&pwd);
+		pdb_free_sam(&new_pwd);
 		return False;
  	}
 
 	acct_ctrl = pdb_get_acct_ctrl(pwd);
 
 	copy_sam_passwd(new_pwd, pwd);
-	pdb_free_sam(pwd);
+	pdb_free_sam(&pwd);
 	
 	copy_id23_to_sam_passwd(new_pwd, id23);
  
 	if (!decode_pw_buffer((char*)id23->pass, plaintext_buf, 256, &len)) {
-		pdb_free_sam(new_pwd);
+		pdb_free_sam(&new_pwd);
 		return False;
  	}
   
-	pdb_set_plaintext_passwd (new_pwd, plaintext_buf);
+	if (!pdb_set_plaintext_passwd (new_pwd, plaintext_buf)) {
+		pdb_free_sam(&new_pwd);
+		return False;
+	}
  
 	/* if it's a trust account, don't update /etc/passwd */
 	if ( ( (acct_ctrl &  ACB_DOMTRUST) == ACB_DOMTRUST ) ||
@@ -2387,7 +2399,7 @@ static BOOL set_user_info_23(SAM_USER_INFO_23 *id23, uint32 rid)
 		/* update the UNIX password */
 		if (lp_unix_password_sync() )
 			if(!chgpasswd(pdb_get_username(new_pwd), "", plaintext_buf, True)) {
-				pdb_free_sam(new_pwd);
+				pdb_free_sam(&new_pwd);
 				return False;
 			}
 	}
@@ -2395,11 +2407,11 @@ static BOOL set_user_info_23(SAM_USER_INFO_23 *id23, uint32 rid)
 	ZERO_STRUCT(plaintext_buf);
  
 	if(!pdb_update_sam_account(new_pwd, True)) {
-		pdb_free_sam(new_pwd);
+		pdb_free_sam(&new_pwd);
 		return False;
 	}
  
-	pdb_free_sam(new_pwd);
+	pdb_free_sam(&new_pwd);
 
 	return True;
 }
@@ -2418,7 +2430,7 @@ static BOOL set_user_info_pw(char *pass, uint32 rid)
  	pdb_init_sam(&pwd);
  
 	if (!pdb_getsampwrid(pwd, rid)) {
-		pdb_free_sam(pwd);
+		pdb_free_sam(&pwd);
 		return False;
  	}
 	
@@ -2427,11 +2439,14 @@ static BOOL set_user_info_pw(char *pass, uint32 rid)
 	ZERO_STRUCT(plaintext_buf);
  
 	if (!decode_pw_buffer(pass, plaintext_buf, 256, &len)) {
-		pdb_free_sam(pwd);
+		pdb_free_sam(&pwd);
 		return False;
  	}
 
-	pdb_set_plaintext_passwd (pwd, plaintext_buf);
+	if (!pdb_set_plaintext_passwd (pwd, plaintext_buf)) {
+		pdb_free_sam(&pwd);
+		return False;
+	}
  
 	/* if it's a trust account, don't update /etc/passwd */
 	if ( ( (acct_ctrl &  ACB_DOMTRUST) == ACB_DOMTRUST ) ||
@@ -2442,7 +2457,7 @@ static BOOL set_user_info_pw(char *pass, uint32 rid)
 		/* update the UNIX password */
 		if (lp_unix_password_sync())
 			if(!chgpasswd(pdb_get_username(pwd), "", plaintext_buf, True)) {
-				pdb_free_sam(pwd);
+				pdb_free_sam(&pwd);
 				return False;
 			}
 	}
@@ -2453,11 +2468,11 @@ static BOOL set_user_info_pw(char *pass, uint32 rid)
  
 	/* update the SAMBA password */
 	if(!pdb_update_sam_account(pwd, True)) {
-		pdb_free_sam(pwd);
+		pdb_free_sam(&pwd);
 		return False;
  	}
 
-	pdb_free_sam(pwd);
+	pdb_free_sam(&pwd);
 
 	return True;
 }
@@ -2516,14 +2531,14 @@ NTSTATUS _samr_set_userinfo(pipes_struct *p, SAMR_Q_SET_USERINFO *q_u, SAMR_R_SE
 	unbecome_root();
 	if(ret == False) {
 		DEBUG(0,("_samr_set_userinfo: Unable to get smbpasswd entry for uid %u\n", (unsigned int)user.uid ));
-		pdb_free_sam(sam_pass);
+		pdb_free_sam(&sam_pass);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 		
 	memset(sess_key, '\0', 16);
 	mdfour(sess_key, pdb_get_nt_passwd(sam_pass), 16);
 
-	pdb_free_sam(sam_pass);
+	pdb_free_sam(&sam_pass);
 
 	/* ok!  user info levels (lots: see MSDEV help), off we go... */
 	switch (switch_value) {