Attachment 991 Details for Bug 2391 – Move to allocated passwd structures

[patch] Move to allocated passwd structures

samba-3.0-segfault-prevention.patch (text/plain), 4.47 KB, created by Andrew Bartlett on 2005-02-25 18:13:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2005-02-25 18:13:43 UTC

Size: 4.47 KB

patch

obsolete

>Index: smbd/sesssetup.c
>===================================================================
>--- smbd/sesssetup.c	(revision 5533)
>+++ smbd/sesssetup.c	(working copy)
>@@ -267,8 +267,10 @@
> 		SAFE_FREE(client);
> 		data_blob_free(&ap_rep);
> 		data_blob_free(&session_key);
>+		passwd_free(&pw);
> 		return ERROR_NT(ret);
> 	}
>+	passwd_free(&pw);
> 
>         /* make_server_info_pw does not set the domain. Without this we end up
> 	 * with the local netbios name in substitutions for %D. */
>Index: auth/auth_util.c
>===================================================================
>--- auth/auth_util.c	(revision 5533)
>+++ auth/auth_util.c	(working copy)
>@@ -990,6 +947,7 @@
> 				 uid_t *uid, gid_t *gid,
> 				 SAM_ACCOUNT **sam_account)
> {
>+	NTSTATUS nt_status;
> 	fstring dom_user, lower_username;
> 	fstring real_username;
> 	struct passwd *passwd;
>@@ -1024,7 +982,9 @@
> 	DEBUG(5,("fill_sam_account: located username was [%s]\n",
> 		*found_username));
> 
>-	return pdb_init_sam_pw(sam_account, passwd);
>+	nt_status = pdb_init_sam_pw(sam_account, passwd);
>+	passwd_free(&passwd);
>+	return nt_status;
> }
> 
> /****************************************************************************
>@@ -1056,7 +1016,7 @@
> 	if ( p ) {
> 		fstring strip_username;
> 
>-		pw = Get_Pwnam( domuser );
>+		pw = Get_Pwnam_alloc( domuser );
> 		if ( pw ) {	
> 			/* make sure we get the case of the username correct */
> 			/* work around 'winbind use default domain = yes' */
>@@ -1087,7 +1047,7 @@
> 	
> 	/* just lookup a plain username */
> 	
>-	pw = Get_Pwnam(username);
>+	pw = Get_Pwnam_alloc(username);
> 		
> 	/* Create local user if requested. */
> 	
>@@ -1097,7 +1057,7 @@
> 			return NULL;
> 
> 		auth_add_user_script(NULL, username);
>-		pw = Get_Pwnam(username);
>+		pw = Get_Pwnam_alloc(username);
> 	}
> 	
> 	/* one last check for a valid passwd struct */
>Index: auth/auth_server.c
>===================================================================
>--- auth/auth_server.c	(revision 5533)
>+++ auth/auth_server.c	(working copy)
>@@ -384,6 +384,7 @@
> 			real_username, True )) != NULL ) 
> 		{
> 			nt_status = make_server_info_pw(server_info, pass->pw_name, pass);
>+			passwd_free(&pass);
> 		}
> 		else
> 		{
>Index: lib/username.c
>===================================================================
>--- lib/username.c	(revision 5533)
>+++ lib/username.c	(working copy)
>@@ -250,35 +250,16 @@
> done:
> 	DEBUG(5,("Get_Pwnam_internals %s find user [%s]!\n",ret ? "did":"didn't", user));
> 
>-	/* This call used to just return the 'passwd' static buffer.
>-	   This could then have accidental reuse implications, so 
>-	   we now malloc a copy, and free it in the next use.
>-
>-	   This should cause the (ab)user to segfault if it 
>-	   uses an old struct. 
>-	   
>-	   This is better than useing the wrong data in security
>-	   critical operations.
>-
>-	   The real fix is to make the callers free the returned 
>-	   malloc'ed data.
>-	*/
>-
>-	if (Get_Pwnam_ret) {
>-		passwd_free(&Get_Pwnam_ret);
>-	}
>-	
>-	Get_Pwnam_ret = ret;
>-
> 	return ret;
> }
> 
> /****************************************************************************
>  Get_Pwnam wrapper without modification.
>   NOTE: This with NOT modify 'user'! 
>+  This will return an allocated structure
> ****************************************************************************/
> 
>-struct passwd *Get_Pwnam(const char *user)
>+struct passwd *Get_Pwnam_alloc(const char *user)
> {
> 	fstring user2;
> 	struct passwd *ret;
>@@ -298,6 +279,40 @@
> }
> 
> /****************************************************************************
>+ Get_Pwnam wrapper without modification.
>+  NOTE: This with NOT modify 'user'! 
>+****************************************************************************/
>+
>+struct passwd *Get_Pwnam(const char *user)
>+{
>+	struct passwd *ret;
>+
>+	ret = Get_Pwnam_alloc(user);
>+	
>+	/* This call used to just return the 'passwd' static buffer.
>+	   This could then have accidental reuse implications, so 
>+	   we now malloc a copy, and free it in the next use.
>+
>+	   This should cause the (ab)user to segfault if it 
>+	   uses an old struct. 
>+	   
>+	   This is better than useing the wrong data in security
>+	   critical operations.
>+
>+	   The real fix is to make the callers free the returned 
>+	   malloc'ed data.
>+	*/
>+
>+	if (Get_Pwnam_ret) {
>+		passwd_free(&Get_Pwnam_ret);
>+	}
>+	
>+	Get_Pwnam_ret = ret;
>+
>+	return ret;  
>+}
>+
>+/****************************************************************************
>  Check if a user is in a netgroup user list. If at first we don't succeed,
>  try lower case.
> ****************************************************************************/

Actions: View

Attachments on bug 2391: 983 | 984 | 986 | 988 | 989 | 990 | 991