From e1daf816f3d809d288313fe2db98b5a731c93a79 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 17 May 2002 14:19:36 +0000 Subject: Make Get_Pwnam use getpwnam_alloc() in an attempt to make it segfault rather than allow silent reuse of stale static buffer. Next step is to make this fn return that allocated buffer. --- source/lib/username.c | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/source/lib/username.c b/source/lib/username.c index da603949bc8..f6ce765b412 100644 --- a/source/lib/username.c +++ b/source/lib/username.c @@ -55,9 +55,10 @@ BOOL split_domain_and_name(const char *name, char *domain, char* username) } else if (lp_winbind_use_default_domain()) { fstrcpy(username, name); fstrcpy(domain, lp_workgroup()); - } else + } else { return False; - + } + DEBUG(10,("split_domain_and_name: all is fine, domain is |%s| and name is |%s|\n", domain, username)); return True; } @@ -238,6 +239,8 @@ BOOL map_username(char *user) * - using lp_usernamelevel() for permutations. ****************************************************************************/ +static struct passwd *Get_Pwnam_ret = NULL; + static struct passwd *Get_Pwnam_internals(const char *user, char *user2) { struct passwd *ret = NULL; @@ -252,14 +255,14 @@ static struct passwd *Get_Pwnam_internals(const char *user, char *user2) common case on UNIX systems */ strlower(user2); DEBUG(5,("Trying _Get_Pwnam(), username as lowercase is %s\n",user2)); - ret = sys_getpwnam(user2); + ret = getpwnam_alloc(user2); if(ret) goto done; /* Try as given, if username wasn't originally lowercase */ if(strcmp(user, user2) != 0) { DEBUG(5,("Trying _Get_Pwnam(), username as given is %s\n", user)); - ret = sys_getpwnam(user); + ret = getpwnam_alloc(user); if(ret) goto done; } @@ -268,7 +271,7 @@ static struct passwd *Get_Pwnam_internals(const char *user, char *user2) strupper(user2); if(strcmp(user, user2) != 0) { DEBUG(5,("Trying _Get_Pwnam(), username as uppercase is %s\n", user2)); - ret = sys_getpwnam(user2); + ret = getpwnam_alloc(user2); if(ret) goto done; } @@ -276,10 +279,31 @@ static struct passwd *Get_Pwnam_internals(const char *user, char *user2) /* Try all combinations up to usernamelevel */ strlower(user2); DEBUG(5,("Checking combinations of %d uppercase letters in %s\n", lp_usernamelevel(), user2)); - ret = uname_string_combinations(user2, sys_getpwnam, lp_usernamelevel()); + ret = uname_string_combinations(user2, getpwnam_alloc, lp_usernamelevel()); 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; } @@ -288,7 +312,7 @@ done: NOTE: This can potentially modify 'user'! ****************************************************************************/ -struct passwd *Get_Pwnam_Modify(char *user) +struct passwd *Get_Pwnam_Modify(fstring user) { fstring user2; struct passwd *ret; @@ -320,8 +344,6 @@ struct passwd *Get_Pwnam(const char *user) ret = Get_Pwnam_internals(user, user2); - DEBUG(5,("Get_Pwnam %s find user [%s]!\n",ret ? "did":"didn't", user)); - return ret; } -- cgit