s3-samr: fix various potential memleaks in samr_SetUserInfo.

Was there any reason why we did all these individual TALLOC_FREEs ?

Guenther

1 files changed, 2 insertions, 43 deletions

diff --git a/source3/rpc_server/srv_samr_nt.c b/source3/rpc_server/srv_samr_nt.c
index 4b913bace1..3b8c3a1547 100644
--- a/source3/rpc_server/srv_samr_nt.c
+++ b/source3/rpc_server/srv_samr_nt.c
@@ -3774,13 +3774,11 @@ static NTSTATUS set_user_info_7(TALLOC_CTX *mem_ctx,
 
 	if (id7 == NULL) {
 		DEBUG(5, ("set_user_info_7: NULL id7\n"));
-		TALLOC_FREE(pwd);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
 	if (!id7->account_name.string) {
 	        DEBUG(5, ("set_user_info_7: failed to get new username\n"));
-		TALLOC_FREE(pwd);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
@@ -3800,7 +3798,6 @@ static NTSTATUS set_user_info_7(TALLOC_CTX *mem_ctx,
 
 	rc = pdb_rename_sam_account(pwd, id7->account_name.string);
 
-	TALLOC_FREE(pwd);
 	return rc;
 }
 
@@ -3813,23 +3810,18 @@ static bool set_user_info_16(struct samr_UserInfo16 *id16,
 {
 	if (id16 == NULL) {
 		DEBUG(5, ("set_user_info_16: NULL id16\n"));
-		TALLOC_FREE(pwd);
 		return False;
 	}
 
 	/* FIX ME: check if the value is really changed --metze */
 	if (!pdb_set_acct_ctrl(pwd, id16->acct_flags, PDB_CHANGED)) {
-		TALLOC_FREE(pwd);
 		return False;
 	}
 
 	if(!NT_STATUS_IS_OK(pdb_update_sam_account(pwd))) {
-		TALLOC_FREE(pwd);
 		return False;
 	}
 
-	TALLOC_FREE(pwd);
-
 	return True;
 }
 
@@ -3842,29 +3834,23 @@ static bool set_user_info_18(struct samr_UserInfo18 *id18,
 {
 	if (id18 == NULL) {
 		DEBUG(2, ("set_user_info_18: id18 is NULL\n"));
-		TALLOC_FREE(pwd);
 		return False;
 	}
 
 	if (!pdb_set_lanman_passwd (pwd, id18->lm_pwd.hash, PDB_CHANGED)) {
-		TALLOC_FREE(pwd);
 		return False;
 	}
 	if (!pdb_set_nt_passwd     (pwd, id18->nt_pwd.hash, PDB_CHANGED)) {
-		TALLOC_FREE(pwd);
 		return False;
 	}
  	if (!pdb_set_pass_last_set_time (pwd, time(NULL), PDB_CHANGED)) {
-		TALLOC_FREE(pwd);
 		return False;
 	}
 
 	if(!NT_STATUS_IS_OK(pdb_update_sam_account(pwd))) {
-		TALLOC_FREE(pwd);
 		return False;
  	}
 
-	TALLOC_FREE(pwd);
 	return True;
 }
 
@@ -3884,12 +3870,9 @@ static bool set_user_info_20(struct samr_UserInfo20 *id20,
 
 	/* write the change out */
 	if(!NT_STATUS_IS_OK(pdb_update_sam_account(pwd))) {
-		TALLOC_FREE(pwd);
 		return False;
  	}
 
-	TALLOC_FREE(pwd);
-
 	return True;
 }
 
@@ -3909,7 +3892,6 @@ static NTSTATUS set_user_info_21(TALLOC_CTX *mem_ctx,
 	}
 
 	if (id21->fields_present & SAMR_FIELD_LAST_PWD_CHANGE) {
-		TALLOC_FREE(pwd);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
@@ -3938,7 +3920,6 @@ static NTSTATUS set_user_info_21(TALLOC_CTX *mem_ctx,
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(0,("set_user_info_21: failed to rename account: %s\n",
 				nt_errstr(status)));
-			TALLOC_FREE(pwd);
 			return status;
 		}
 
@@ -3969,12 +3950,9 @@ static NTSTATUS set_user_info_21(TALLOC_CTX *mem_ctx,
 
 	/* write the change out */
 	if(!NT_STATUS_IS_OK(status = pdb_update_sam_account(pwd))) {
-		TALLOC_FREE(pwd);
 		return status;
  	}
 
-	TALLOC_FREE(pwd);
-
 	return NT_STATUS_OK;
 }
 
@@ -3997,7 +3975,6 @@ static NTSTATUS set_user_info_23(TALLOC_CTX *mem_ctx,
 	}
 
 	if (id23->info.fields_present & SAMR_FIELD_LAST_PWD_CHANGE) {
-		TALLOC_FREE(pwd);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
@@ -4012,12 +3989,10 @@ static NTSTATUS set_user_info_23(TALLOC_CTX *mem_ctx,
 				&plaintext_buf,
 				&len,
 				STR_UNICODE)) {
-		TALLOC_FREE(pwd);
 		return NT_STATUS_WRONG_PASSWORD;
  	}
 
 	if (!pdb_set_plaintext_passwd (pwd, plaintext_buf)) {
-		TALLOC_FREE(pwd);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
@@ -4034,7 +4009,6 @@ static NTSTATUS set_user_info_23(TALLOC_CTX *mem_ctx,
 			struct passwd *passwd;
 			if (pdb_get_username(pwd) == NULL) {
 				DEBUG(1, ("chgpasswd: User without name???\n"));
-				TALLOC_FREE(pwd);
 				return NT_STATUS_ACCESS_DENIED;
 			}
 
@@ -4044,7 +4018,6 @@ static NTSTATUS set_user_info_23(TALLOC_CTX *mem_ctx,
 			}
 
 			if(!chgpasswd(pdb_get_username(pwd), passwd, "", plaintext_buf, True)) {
-				TALLOC_FREE(pwd);
 				return NT_STATUS_ACCESS_DENIED;
 			}
 			TALLOC_FREE(passwd);
@@ -4056,17 +4029,13 @@ static NTSTATUS set_user_info_23(TALLOC_CTX *mem_ctx,
 	if (IS_SAM_CHANGED(pwd, PDB_GROUPSID) &&
 	    (!NT_STATUS_IS_OK(status =  pdb_set_unix_primary_group(mem_ctx,
 								   pwd)))) {
-		TALLOC_FREE(pwd);
 		return status;
 	}
 
 	if(!NT_STATUS_IS_OK(status = pdb_update_sam_account(pwd))) {
-		TALLOC_FREE(pwd);
 		return status;
 	}
 
-	TALLOC_FREE(pwd);
-
 	return NT_STATUS_OK;
 }
 
@@ -4097,12 +4066,10 @@ static bool set_user_info_pw(uint8 *pass, struct samu *pwd,
 				&plaintext_buf,
 				&len,
 				STR_UNICODE)) {
-		TALLOC_FREE(pwd);
 		return False;
  	}
 
 	if (!pdb_set_plaintext_passwd (pwd, plaintext_buf)) {
-		TALLOC_FREE(pwd);
 		return False;
 	}
 
@@ -4118,7 +4085,6 @@ static bool set_user_info_pw(uint8 *pass, struct samu *pwd,
 
 			if (pdb_get_username(pwd) == NULL) {
 				DEBUG(1, ("chgpasswd: User without name???\n"));
-				TALLOC_FREE(pwd);
 				return False;
 			}
 
@@ -4128,7 +4094,6 @@ static bool set_user_info_pw(uint8 *pass, struct samu *pwd,
 			}
 
 			if(!chgpasswd(pdb_get_username(pwd), passwd, "", plaintext_buf, True)) {
-				TALLOC_FREE(pwd);
 				return False;
 			}
 			TALLOC_FREE(passwd);
@@ -4156,12 +4121,9 @@ static bool set_user_info_pw(uint8 *pass, struct samu *pwd,
 
 	/* update the SAMBA password */
 	if(!NT_STATUS_IS_OK(pdb_update_sam_account(pwd))) {
-		TALLOC_FREE(pwd);
 		return False;
  	}
 
-	TALLOC_FREE(pwd);
-
 	return True;
 }
 
@@ -4181,7 +4143,6 @@ static NTSTATUS set_user_info_25(TALLOC_CTX *mem_ctx,
 	}
 
 	if (id25->info.fields_present & SAMR_FIELD_LAST_PWD_CHANGE) {
-		TALLOC_FREE(pwd);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
@@ -4189,7 +4150,6 @@ static NTSTATUS set_user_info_25(TALLOC_CTX *mem_ctx,
 
 	/* write the change out */
 	if(!NT_STATUS_IS_OK(status = pdb_update_sam_account(pwd))) {
-		TALLOC_FREE(pwd);
 		return status;
  	}
 
@@ -4208,9 +4168,6 @@ static NTSTATUS set_user_info_25(TALLOC_CTX *mem_ctx,
 		}
 	}
 
-	/* WARNING: No TALLOC_FREE(pwd), we are about to set the password
-	 * hereafter! */
-
 	return NT_STATUS_OK;
 }
 
@@ -4422,6 +4379,8 @@ NTSTATUS _samr_SetUserInfo(pipes_struct *p,
 
  done:
 
+	TALLOC_FREE(pwd);
+
 	if (has_enough_rights) {
 		unbecome_root();
 	}