From b279ee16e982d419c2205a7f790bd9cb8035d6e5 Mon Sep 17 00:00:00 2001 From: Gerald Carter Date: Tue, 7 Jun 2005 17:52:19 +0000 Subject: r7372: abartet's patch for BUG 2391 (segv caused by free a static pointer) (This used to be commit 4cda2bd035276bd090bf0fbd4e3b2eff657a80cb) --- source3/auth/auth_server.c | 1 + source3/auth/auth_util.c | 11 +++++---- source3/lib/username.c | 57 +++++++++++++++++++++++++++++----------------- source3/smbd/sesssetup.c | 2 ++ 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/source3/auth/auth_server.c b/source3/auth/auth_server.c index bc611ec229..7bce32ef2b 100644 --- a/source3/auth/auth_server.c +++ b/source3/auth/auth_server.c @@ -384,6 +384,7 @@ use this machine as the password server.\n")); real_username, True )) != NULL ) { nt_status = make_server_info_pw(server_info, pass->pw_name, pass); + passwd_free(&pass); } else { diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index 31bfa2fe01..021f780112 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -958,6 +958,7 @@ static NTSTATUS fill_sam_account(TALLOC_CTX *mem_ctx, uid_t *uid, gid_t *gid, SAM_ACCOUNT **sam_account) { + NTSTATUS nt_status; fstring dom_user, lower_username; fstring real_username; struct passwd *passwd; @@ -992,7 +993,9 @@ static NTSTATUS fill_sam_account(TALLOC_CTX *mem_ctx, 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; } /**************************************************************************** @@ -1024,7 +1027,7 @@ struct passwd *smb_getpwnam( char *domuser, fstring save_username, BOOL create ) 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' */ @@ -1055,7 +1058,7 @@ struct passwd *smb_getpwnam( char *domuser, fstring save_username, BOOL create ) /* just lookup a plain username */ - pw = Get_Pwnam(username); + pw = Get_Pwnam_alloc(username); /* Create local user if requested. */ @@ -1065,7 +1068,7 @@ struct passwd *smb_getpwnam( char *domuser, fstring save_username, BOOL create ) return NULL; smb_create_user(NULL, username, NULL); - pw = Get_Pwnam(username); + pw = Get_Pwnam_alloc(username); } /* one last check for a valid passwd struct */ diff --git a/source3/lib/username.c b/source3/lib/username.c index 317935d396..e691e4c1f1 100644 --- a/source3/lib/username.c +++ b/source3/lib/username.c @@ -250,35 +250,16 @@ static struct passwd *Get_Pwnam_internals(const char *user, char *user2) 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; @@ -297,6 +278,40 @@ struct passwd *Get_Pwnam(const char *user) return ret; } +/**************************************************************************** + 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. diff --git a/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c index 48524b472d..6f963fc603 100644 --- a/source3/smbd/sesssetup.c +++ b/source3/smbd/sesssetup.c @@ -267,8 +267,10 @@ static int reply_spnego_kerberos(connection_struct *conn, 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. */ -- cgit