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)

5 files changed, 80 insertions, 61 deletions

diff --git a/source3/smbd/auth.c b/source3/smbd/auth.c
index 5b6b2d4c42..6aa2714b0b 100644
--- a/source3/smbd/auth.c
+++ b/source3/smbd/auth.c
@@ -189,6 +189,10 @@ NTSTATUS pass_check_smb_with_chal(char *smb_user, char *unix_user,
 			user_info.lm_resp.buffer = (uint8 *)local_lm_response;
 			user_info.lm_resp.len = 24;
 
+
+			/* WATCH OUT. This doesn't work if the incoming password is incorrectly cased. 
+			   We might want to add a check here and only do an LM in that case */
+
 			/* This encrypts the lm_pwd feild, which actualy contains the password
 			   rather than the nt_pwd field becouse that contains nothing */
 			SMBNTencrypt((uchar *)lm_pwd, user_info.chal, local_nt_response);
diff --git a/source3/smbd/auth_smbpasswd.c b/source3/smbd/auth_smbpasswd.c
index 567414d1a2..8159ad988f 100644
--- a/source3/smbd/auth_smbpasswd.c
+++ b/source3/smbd/auth_smbpasswd.c
@@ -306,7 +306,7 @@ NTSTATUS check_smbpasswd_security(const auth_usersupplied_info *user_info, auth_
 	if (ret == False)
 	{
 		DEBUG(1,("Couldn't find user '%s' in passdb file.\n", user_info->unix_username.str));
-		pdb_free_sam(sampass);
+		pdb_free_sam(&sampass);
 		return NT_STATUS_NO_SUCH_USER;
 	}
 
@@ -316,7 +316,7 @@ NTSTATUS check_smbpasswd_security(const auth_usersupplied_info *user_info, auth_
 		nt_status = sam_account_ok(sampass, user_info);
 	}
 
-	pdb_free_sam(sampass);
+	pdb_free_sam(&sampass);
 	return nt_status;
 }
 
diff --git a/source3/smbd/auth_unix.c b/source3/smbd/auth_unix.c
index 5582682d98..0d73988d8a 100644
--- a/source3/smbd/auth_unix.c
+++ b/source3/smbd/auth_unix.c
@@ -31,7 +31,7 @@ this ugly hack needs to die, but not quite yet...
 static BOOL update_smbpassword_file(char *user, char *password)
 {
 	SAM_ACCOUNT 	*sampass = NULL;
-	BOOL 		ret;
+	BOOL            ret;
 	
 	pdb_init_sam(&sampass);
 	
@@ -41,7 +41,7 @@ static BOOL update_smbpassword_file(char *user, char *password)
 
 	if(ret == False) {
 		DEBUG(0,("pdb_getsampwnam returned NULL\n"));
-		pdb_free_sam(sampass);
+		pdb_free_sam(&sampass);
 		return False;
 	}
 
@@ -49,16 +49,32 @@ static BOOL update_smbpassword_file(char *user, char *password)
 	 * Remove the account disabled flag - we are updating the
 	 * users password from a login.
 	 */
-	pdb_set_acct_ctrl(sampass, pdb_get_acct_ctrl(sampass) & ~ACB_DISABLED);
+	if (!pdb_set_acct_ctrl(sampass, pdb_get_acct_ctrl(sampass) & ~ACB_DISABLED)) {
+		pdb_free_sam(&sampass);
+		return False;
+	}
+
+	if (!pdb_set_plaintext_passwd (sampass, password)) {
+		pdb_free_sam(&sampass);
+		return False;
+	}
 
-	/* Here, the flag is one, because we want to ignore the
+	/* Now write it into the file. */
+	become_root();
+
+	/* Here, the override flag is True, because we want to ignore the
            XXXXXXX'd out password */
-	ret = change_oem_password( sampass, password, True);
-	if (ret == False) {
-		DEBUG(3,("change_oem_password returned False\n"));
+	ret = pdb_update_sam_account (sampass, True);
+
+	unbecome_root();
+
+	if (ret) {
+		DEBUG(3,("pdb_update_sam_account returned %d\n",ret));
 	}
 
-	pdb_free_sam(sampass);
+	memset(password, '\0', strlen(password));
+
+	pdb_free_sam(&sampass);
 	return ret;
 }
 
diff --git a/source3/smbd/chgpasswd.c b/source3/smbd/chgpasswd.c
index 9dbd57129c..de49083960 100644
--- a/source3/smbd/chgpasswd.c
+++ b/source3/smbd/chgpasswd.c
@@ -557,7 +557,6 @@ BOOL chgpasswd(char *name, char *oldpass, char *newpass, BOOL as_root)
 BOOL check_lanman_password(char *user, uchar * pass1,
 			   uchar * pass2, SAM_ACCOUNT **hnd)
 {
-	static uchar null_pw[16];
 	uchar unenc_new_pw[16];
 	uchar unenc_old_pw[16];
 	SAM_ACCOUNT *sampass = NULL;
@@ -571,7 +570,7 @@ BOOL check_lanman_password(char *user, uchar * pass1,
 
 	if (ret == False) {
 		DEBUG(0,("check_lanman_password: getsampwnam returned NULL\n"));
-		pdb_free_sam(sampass);
+		pdb_free_sam(&sampass);
 		return False;
 	}
 	
@@ -580,20 +579,20 @@ BOOL check_lanman_password(char *user, uchar * pass1,
 
 	if (acct_ctrl & ACB_DISABLED) {
 		DEBUG(0,("check_lanman_password: account %s disabled.\n", user));
-		pdb_free_sam(sampass);
+		pdb_free_sam(&sampass);
 		return False;
 	}
 
-	if ((lanman_pw == NULL) && (acct_ctrl & ACB_PWNOTREQ)) {
-		uchar no_pw[14];
-		memset(no_pw, '\0', 14);
-		E_P16(no_pw, null_pw);
-		pdb_set_lanman_passwd (sampass, null_pw);
-	}
-	else if (lanman_pw == NULL) {
-		DEBUG(0, ("check_lanman_password: no lanman password !\n"));
-		pdb_free_sam(sampass);
-		return False;
+	if (lanman_pw == NULL) {
+		if (acct_ctrl & ACB_PWNOTREQ) {
+			/* this saves the pointer for the caller */
+			*hnd = sampass;
+			return True;
+		} else {
+			DEBUG(0, ("check_lanman_password: no lanman password !\n"));
+			pdb_free_sam(&sampass);
+			return False;
+		}
 	}
 
 	/* Get the new lanman hash. */
@@ -605,13 +604,12 @@ BOOL check_lanman_password(char *user, uchar * pass1,
 	/* Check that the two old passwords match. */
 	if (memcmp(lanman_pw, unenc_old_pw, 16)) {
 		DEBUG(0,("check_lanman_password: old password doesn't match.\n"));
-		pdb_free_sam(sampass);
+		pdb_free_sam(&sampass);
 		return False;
 	}
 
 	/* this saves the pointer for the caller */
 	*hnd = sampass;
-	
 	return True;
 }
 
@@ -644,22 +642,30 @@ BOOL change_lanman_password(SAM_ACCOUNT *sampass, uchar * pass1,
 		return False;
 	}
 
-	if ((pwd == NULL) && (acct_ctrl & ACB_PWNOTREQ)) {
-		uchar no_pw[14];
-		memset(no_pw, '\0', 14);
-		E_P16(no_pw, null_pw);
-		pdb_set_lanman_passwd(sampass, null_pw);
+	if (pwd == NULL) { 
+		if (acct_ctrl & ACB_PWNOTREQ) {
+			uchar no_pw[14];
+			memset(no_pw, '\0', 14);
+			E_P16(no_pw, null_pw);
+
+			/* Get the new lanman hash. */
+			D_P16(null_pw, pass2, unenc_new_pw);
+		} else {
+			DEBUG(0,("change_lanman_password: no lanman password !\n"));
+			return False;
+		}
+	} else {
+		/* Get the new lanman hash. */
+		D_P16(pwd, pass2, unenc_new_pw);
 	}
-	else if (pwd == NULL) {
-		DEBUG(0,("change_lanman_password: no lanman password !\n"));
+
+	if (!pdb_set_lanman_passwd(sampass, unenc_new_pw)) {
 		return False;
 	}
 
-	/* Get the new lanman hash. */
-	D_P16(pwd, pass2, unenc_new_pw);
-
-	pdb_set_lanman_passwd(sampass, unenc_new_pw);
-	pdb_set_nt_passwd    (sampass, NULL);	/* We lose the NT hash. Sorry. */
+	if (!pdb_set_nt_passwd    (sampass, NULL)) {
+		return False;	/* We lose the NT hash. Sorry. */
+	}
 
 	/* Now flush the sam_passwd struct to persistent storage */
 	become_root();
@@ -690,15 +696,15 @@ BOOL pass_oem_change(char *user,
 	 * available. JRA.
 	 */
 
-	if (ret && lp_unix_password_sync())
+	if ((ret) && lp_unix_password_sync())
 		ret = chgpasswd(user, "", new_passwd, True);
 
 	if (ret)
-		ret = change_oem_password(sampass, new_passwd, False);
+		ret = change_oem_password(sampass, new_passwd);
 
 	memset(new_passwd, 0, sizeof(new_passwd));
 
-	pdb_free_sam(sampass);
+	pdb_free_sam(&sampass);
 
 	return ret;
 }
@@ -762,23 +768,19 @@ static BOOL check_oem_password(char *user,
 
 	/* check for null passwords */
 	if (lanman_pw == NULL) {
-		if (acct_ctrl & ACB_PWNOTREQ)
-			pdb_set_lanman_passwd(sampass, null_pw);
-		else {
+		if (!(acct_ctrl & ACB_PWNOTREQ)) {
 			DEBUG(0,("check_oem_password: no lanman password !\n"));
 			return False;
 		}
 	}
-
+	
 	if (pdb_get_nt_passwd(sampass) == NULL && nt_pass_set) {
-		if (acct_ctrl & ACB_PWNOTREQ)
-			pdb_set_nt_passwd(sampass, null_pw);
-		else {
+		if (!(acct_ctrl & ACB_PWNOTREQ)) {
 			DEBUG(0,("check_oem_password: no ntlm password !\n"));
 			return False;
 		}
 	}
-
+	
 	/* 
 	 * Call the hash function to get the new password.
 	 */
@@ -862,24 +864,21 @@ static BOOL check_oem_password(char *user,
 /***********************************************************
  Code to change the oem password. Changes both the lanman
  and NT hashes.
- override = False, normal
- override = True, override XXXXXXXXXX'd password
 ************************************************************/
 
-BOOL change_oem_password(SAM_ACCOUNT *hnd, char *new_passwd,
-			 BOOL override)
+BOOL change_oem_password(SAM_ACCOUNT *hnd, char *new_passwd)
 {
-	int ret;
+	BOOL ret;
 
-	pdb_set_plaintext_passwd (hnd, new_passwd);
+	if (!pdb_set_plaintext_passwd (hnd, new_passwd)) {
+		return False;
+	}
 
 	/* Now write it into the file. */
 	become_root();
-	ret = pdb_update_sam_account (hnd, override);
+	ret = pdb_update_sam_account (hnd, False);
 	unbecome_root();
 
-	memset(new_passwd, '\0', strlen(new_passwd));
-
 	return ret;
 }
 
diff --git a/source3/smbd/lanman.c b/source3/smbd/lanman.c
index e9213e75a2..b7acfa5f32 100644
--- a/source3/smbd/lanman.c
+++ b/source3/smbd/lanman.c
@@ -1884,7 +1884,7 @@ static BOOL api_SetUserPassword(connection_struct *conn,uint16 vuid, char *param
     fstrcpy(saved_pass2, pass2);
 
     if (check_plaintext_password(user,pass1,strlen(pass1),&sampass) &&
-        change_oem_password(sampass,pass2,False))
+        change_oem_password(sampass,pass2))
     {
       SSVAL(*rparam,0,NERR_Success);
 
@@ -1897,7 +1897,7 @@ static BOOL api_SetUserPassword(connection_struct *conn,uint16 vuid, char *param
       if(lp_unix_password_sync() && !chgpasswd(user,pass1,saved_pass2,False))
         SSVAL(*rparam,0,NERR_badpass);
     }
- 	pdb_free_sam(sampass);
+ 	pdb_free_sam(&sampass);
  }
   
 
@@ -1931,12 +1931,12 @@ static BOOL api_SetUserPassword(connection_struct *conn,uint16 vuid, char *param
   {
     SAM_ACCOUNT *hnd = NULL;
 
-    if(check_lanman_password(user,(unsigned char *)pass1,(unsigned char *)pass2, &hnd) && 
+    if (check_lanman_password(user,(unsigned char *)pass1,(unsigned char *)pass2, &hnd) && 
        change_lanman_password(hnd,(unsigned char *)pass1,(unsigned char *)pass2))
     {
       SSVAL(*rparam,0,NERR_Success);
     }
-	pdb_free_sam(hnd);
+	pdb_free_sam(&hnd);
   }