diff -ruN mod_auth_pam.orig/mod_auth_sys_group.c mod_auth_pam/mod_auth_sys_group.c --- mod_auth_pam.orig/mod_auth_sys_group.c 2002-08-31 07:09:52.000000000 -0400 +++ mod_auth_pam/mod_auth_sys_group.c 2006-05-25 16:33:56.000000000 -0400 @@ -98,11 +98,19 @@ #include "http_log.h" #include "http_protocol.h" #include "http_request.h" +#include "safe_string.h" +#include #define VERSION "2.0-1.1" +#define PSTRING_LEN 1024 +#define FSTRING_LEN 256 + module auth_sys_group_module; +typedef char pstring[PSTRING_LEN]; +typedef char fstring[FSTRING_LEN]; + /* * configuration directive handling */ @@ -159,6 +167,47 @@ { NULL } }; +/** + Safe string copy into a known length string. maxlength does not + include the terminating zero. +**/ + +char *safe_strcpy_fn(const char *fn, int line, char *dest,const char *src, size_t maxlength) +{ + setlogmask (LOG_UPTO (LOG_NOTICE)); + openlog ("messages", LOG_CONS | LOG_PID | LOG_NDELAY, LOG_LOCAL1); + + size_t len; + + if (!dest) { + syslog (LOG_ERR,"ERROR: NULL dest in safe_strcpy, called from [%s][%d]\n", fn, line); + closelog (); + return NULL; + } + + if (!src) { + *dest = 0; + return dest; + } + + len = strnlen(src, maxlength+1); + + if (len > maxlength) { + syslog (LOG_ERR,"ERROR: string overflow by %lu (%lu - %lu) in safe_strcpy [%.50s]\n", + (unsigned long)(len-maxlength), (unsigned long)len, + (unsigned long)maxlength, src); + closelog (); + + len = maxlength; + } + + memmove(dest, src, len); + dest[len] = 0; + return dest; +} + + + /* * These functions return 0 if client is OK, and proper error status * if not... either AUTH_REQUIRED, if we made a check, and it failed, or @@ -181,6 +230,8 @@ (auth_sys_group_dir_config*) ap_get_module_config( r->per_dir_config, &auth_sys_group_module); struct passwd *pwent; + struct passwd *pwent_tmp; + struct passwd pwent_struct; /* check for allowed users/group */ const apr_array_header_t *reqs_arr = ap_requires (r); @@ -192,7 +243,10 @@ /* retrieve user info from passwd and use that info instead of * server supplied info */ - pwent = getpwnam(r->user); + pwent_tmp = getpwnam(r->user); + memcpy((char *)&pwent_struct, pwent_tmp, sizeof(struct passwd)); + pwent = &pwent_struct; + if(pwent == NULL) return DECLINED; @@ -214,27 +268,72 @@ type = ap_getword(r->pool, (const char**)&line, ' '); if(!strcmp(type, "group") && (r->user)) { - struct group *grent; - char **members; + struct group *grent; + char **members; + + while (*line) { + char* groupname = ap_getword_conf(r->pool, (const char**)&line); + + if((grent = getgrnam(groupname)) && grent->gr_mem) { + members = grent->gr_mem; - while (*line) { - char* groupname = ap_getword_conf(r->pool, (const char**)&line); - - if((grent = getgrnam(groupname)) && grent->gr_mem) { - members = grent->gr_mem; - - /* maybe its the primary group? saves the comparisons */ - if(pwent->pw_gid == grent->gr_gid) - return OK; - - while (*members) { - if (strcmp (*members, pwent->pw_name) == 0) - return OK; - - members ++; - } - } - } + /* maybe its the primary group? saves the comparisons */ + if(pwent->pw_gid == grent->gr_gid) + return OK; + + + struct group *gptr; + setgrent(); + while ((gptr = (struct group *)getgrent())) { + if (gptr->gr_gid == grent->gr_gid) + break; + } + + /* + * As user_ok can recurse doing a getgrent(), we must + * copy the member list into a pstring on the stack before + * use. Bug pointed out by leon@eatworms.swmed.edu. + */ + if (gptr) { + pstring member_list; + char *member; + size_t copied_len = 0; + int i; + + *member_list = '\0'; + member = member_list; + + for(i = 0; gptr->gr_mem && gptr->gr_mem[i]; i++) { + size_t member_len = strlen(gptr->gr_mem[i]) + 1; + if( copied_len + member_len < sizeof(pstring)) { + safe_strcpy(member, gptr->gr_mem[i], sizeof(pstring) - copied_len - 1); + copied_len += member_len; + member += member_len; + } else { + *member = '\0'; + } + } + + endgrent(); + member = member_list; + + while (*member) { + static fstring name; + fstrcpy(name,member); + struct passwd *pwent2; + pwent2 = getpwnam(name); + + if (pwent2 != NULL && pwent2->pw_uid == pwent->pw_uid) { + endgrent(); + return OK; + } + member += strlen(member) + 1; + } + } else { + endgrent(); + } + } + } } /* end if group */ } /* method mask */ } diff -ruN mod_auth_pam.orig/safe_string.h mod_auth_pam/safe_string.h --- mod_auth_pam.orig/safe_string.h 1969-12-31 19:00:00.000000000 -0500 +++ mod_auth_pam/safe_string.h 2006-03-27 19:43:15.000000000 -0500 @@ -0,0 +1,228 @@ +/* + Unix SMB/CIFS implementation. + Safe string handling routines. + Copyright (C) Andrew Tridgell 1994-1998 + Copyright (C) Andrew Bartlett 2003 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +*/ + +#ifndef _SAFE_STRING_H +#define _SAFE_STRING_H + +#ifndef _SPLINT_ /* http://www.splint.org */ + +/* Some macros to ensure people don't use buffer overflow vulnerable string + functions. */ + +#ifdef bcopy +#undef bcopy +#endif /* bcopy */ +#define bcopy(src,dest,size) __ERROR__XX__NEVER_USE_BCOPY___; + +#ifdef strcpy +#undef strcpy +#endif /* strcpy */ +#define strcpy(dest,src) __ERROR__XX__NEVER_USE_STRCPY___; + +#ifdef strcat +#undef strcat +#endif /* strcat */ +#define strcat(dest,src) __ERROR__XX__NEVER_USE_STRCAT___; + +#ifdef sprintf +#undef sprintf +#endif /* sprintf */ +#define sprintf __ERROR__XX__NEVER_USE_SPRINTF__; + +/* + * strcasecmp/strncasecmp aren't an error, but it means you're not thinking about + * multibyte. Don't use them. JRA. + */ +#ifdef strcasecmp +#undef strcasecmp +#endif +#define strcasecmp __ERROR__XX__NEVER_USE_STRCASECMP__; + +#ifdef strncasecmp +#undef strncasecmp +#endif +#define strncasecmp __ERROR__XX__NEVER_USE_STRNCASECMP__; + +#endif /* !_SPLINT_ */ + +#ifdef DEVELOPER +#define SAFE_STRING_FUNCTION_NAME FUNCTION_MACRO +#define SAFE_STRING_LINE __LINE__ +#else +#define SAFE_STRING_FUNCTION_NAME ("") +#define SAFE_STRING_LINE (0) +#endif + +/* We need a number of different prototypes for our + non-existant fuctions */ +char * __unsafe_string_function_usage_here__(void); + +size_t __unsafe_string_function_usage_here_size_t__(void); + +size_t __unsafe_string_function_usage_here_char__(void); + +#ifdef HAVE_COMPILER_WILL_OPTIMIZE_OUT_FNS + +/* if the compiler will optimize out function calls, then use this to tell if we are + have the correct types (this works only where sizeof() returns the size of the buffer, not + the size of the pointer). */ + +#define CHECK_STRING_SIZE(d, len) (sizeof(d) != (len) && sizeof(d) != sizeof(char *)) + +#define fstrterminate(d) (CHECK_STRING_SIZE(d, sizeof(fstring)) \ + ? __unsafe_string_function_usage_here_char__() \ + : (((d)[sizeof(fstring)-1]) = '\0')) +#define pstrterminate(d) (CHECK_STRING_SIZE(d, sizeof(pstring)) \ + ? __unsafe_string_function_usage_here_char__() \ + : (((d)[sizeof(pstring)-1]) = '\0')) + +#define wpstrcpy(d,s) ((sizeof(d) != sizeof(wpstring) && sizeof(d) != sizeof(smb_ucs2_t *)) \ + ? __unsafe_string_function_usage_here__() \ + : safe_strcpy_w((d),(s),sizeof(wpstring))) +#define wpstrcat(d,s) ((sizeof(d) != sizeof(wpstring) && sizeof(d) != sizeof(smb_ucs2_t *)) \ + ? __unsafe_string_function_usage_here__() \ + : safe_strcat_w((d),(s),sizeof(wpstring))) +#define wfstrcpy(d,s) ((sizeof(d) != sizeof(wfstring) && sizeof(d) != sizeof(smb_ucs2_t *)) \ + ? __unsafe_string_function_usage_here__() \ + : safe_strcpy_w((d),(s),sizeof(wfstring))) +#define wfstrcat(d,s) ((sizeof(d) != sizeof(wfstring) && sizeof(d) != sizeof(smb_ucs2_t *)) \ + ? __unsafe_string_function_usage_here__() \ + : safe_strcat_w((d),(s),sizeof(wfstring))) + +#define push_pstring_base(dest, src, pstring_base) \ + (CHECK_STRING_SIZE(pstring_base, sizeof(pstring)) \ + ? __unsafe_string_function_usage_here_size_t__() \ + : push_ascii(dest, src, sizeof(pstring)-PTR_DIFF(dest,pstring_base)-1, STR_TERMINATE)) + +#else /* HAVE_COMPILER_WILL_OPTIMIZE_OUT_FNS */ + +#define fstrterminate(d) (((d)[sizeof(fstring)-1]) = '\0') +#define pstrterminate(d) (((d)[sizeof(pstring)-1]) = '\0') + +#define wpstrcpy(d,s) safe_strcpy_w((d),(s),sizeof(wpstring)) +#define wpstrcat(d,s) safe_strcat_w((d),(s),sizeof(wpstring)) +#define wfstrcpy(d,s) safe_strcpy_w((d),(s),sizeof(wfstring)) +#define wfstrcat(d,s) safe_strcat_w((d),(s),sizeof(wfstring)) + +#define push_pstring_base(dest, src, pstring_base) \ + push_ascii(dest, src, sizeof(pstring)-PTR_DIFF(dest,pstring_base)-1, STR_TERMINATE) + +#endif /* HAVE_COMPILER_WILL_OPTIMIZE_OUT_FNS */ + +#define safe_strcpy_base(dest, src, base, size) \ + safe_strcpy(dest, src, size-PTR_DIFF(dest,base)-1) + +/* String copy functions - macro hell below adds 'type checking' (limited, + but the best we can do in C) and may tag with function name/number to + record the last 'clobber region' on that string */ + +#define pstrcpy(d,s) safe_strcpy((d), (s),sizeof(pstring)-1) +#define pstrcat(d,s) safe_strcat((d), (s),sizeof(pstring)-1) +#define fstrcpy(d,s) safe_strcpy((d),(s),sizeof(fstring)-1) +#define fstrcat(d,s) safe_strcat((d),(s),sizeof(fstring)-1) +#define nstrcpy(d,s) safe_strcpy((d), (s),sizeof(nstring)-1) +#define unstrcpy(d,s) safe_strcpy((d), (s),sizeof(unstring)-1) + +/* the addition of the DEVELOPER checks in safe_strcpy means we must + * update a lot of code. To make this a little easier here are some + * functions that provide the lengths with less pain */ +#define pstrcpy_base(dest, src, pstring_base) \ + safe_strcpy(dest, src, sizeof(pstring)-PTR_DIFF(dest,pstring_base)-1) + + +/* Inside the _fn variants of these is a call to clobber_region(), - + * which might destroy the stack on a buggy function. We help the + * debugging process by putting the function and line who last caused + * a clobbering into a static buffer. If the program crashes at + * address 0xf1f1f1f1 then this function is probably, but not + * necessarily, to blame. */ + +/* overmalloc_safe_strcpy: DEPRECATED! Used when you know the + * destination buffer is longer than maxlength, but you don't know how + * long. This is not a good situation, because we can't do the normal + * sanity checks. Don't use in new code! */ + +#define overmalloc_safe_strcpy(dest,src,maxlength) safe_strcpy_fn(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE,dest,src,maxlength) +#define safe_strcpy(dest,src,maxlength) safe_strcpy_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE,dest,src,maxlength) +#define safe_strcat(dest,src,maxlength) safe_strcat_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE,dest,src,maxlength) +#define push_string(base_ptr, dest, src, dest_len, flags) push_string_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, base_ptr, dest, src, dest_len, flags) +#define pull_string(base_ptr, dest, src, dest_len, src_len, flags) pull_string_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, base_ptr, dest, src, dest_len, src_len, flags) +#define clistr_push(cli, dest, src, dest_len, flags) clistr_push_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, cli, dest, src, dest_len, flags) +#define clistr_pull(cli, dest, src, dest_len, src_len, flags) clistr_pull_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, cli, dest, src, dest_len, src_len, flags) +#define srvstr_push(base_ptr, dest, src, dest_len, flags) srvstr_push_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, base_ptr, dest, src, dest_len, flags) + +#define alpha_strcpy(dest,src,other_safe_chars,maxlength) alpha_strcpy_fn(SAFE_STRING_FUNCTION_NAME,SAFE_STRING_LINE,dest,src,other_safe_chars,maxlength) +#define StrnCpy(dest,src,n) StrnCpy_fn(SAFE_STRING_FUNCTION_NAME,SAFE_STRING_LINE,dest,src,n) + +#ifdef HAVE_COMPILER_WILL_OPTIMIZE_OUT_FNS + +/* if the compiler will optimize out function calls, then use this to tell if we are + have the correct types (this works only where sizeof() returns the size of the buffer, not + the size of the pointer). */ + +#define safe_strcpy_fn2(fn_name, fn_line, d, s, max_len) \ + (CHECK_STRING_SIZE(d, max_len+1) \ + ? __unsafe_string_function_usage_here__() \ + : safe_strcpy_fn(fn_name, fn_line, (d), (s), (max_len))) + +#define safe_strcat_fn2(fn_name, fn_line, d, s, max_len) \ + (CHECK_STRING_SIZE(d, max_len+1) \ + ? __unsafe_string_function_usage_here__() \ + : safe_strcat_fn(fn_name, fn_line, (d), (s), (max_len))) + +#define push_string_fn2(fn_name, fn_line, base_ptr, dest, src, dest_len, flags) \ + (CHECK_STRING_SIZE(dest, dest_len) \ + ? __unsafe_string_function_usage_here_size_t__() \ + : push_string_fn(fn_name, fn_line, base_ptr, dest, src, dest_len, flags)) + +#define pull_string_fn2(fn_name, fn_line, base_ptr, dest, src, dest_len, src_len, flags) \ + (CHECK_STRING_SIZE(dest, dest_len) \ + ? __unsafe_string_function_usage_here_size_t__() \ + : pull_string_fn(fn_name, fn_line, base_ptr, dest, src, dest_len, src_len, flags)) + +#define clistr_push_fn2(fn_name, fn_line, cli, dest, src, dest_len, flags) \ + (CHECK_STRING_SIZE(dest, dest_len) \ + ? __unsafe_string_function_usage_here_size_t__() \ + : clistr_push_fn(fn_name, fn_line, cli, dest, src, dest_len, flags)) + +#define clistr_pull_fn2(fn_name, fn_line, cli, dest, src, dest_len, srclen, flags) \ + (CHECK_STRING_SIZE(dest, dest_len) \ + ? __unsafe_string_function_usage_here_size_t__() \ + : clistr_pull_fn(fn_name, fn_line, cli, dest, src, dest_len, srclen, flags)) + +#define srvstr_push_fn2(fn_name, fn_line, base_ptr, dest, src, dest_len, flags) \ + (CHECK_STRING_SIZE(dest, dest_len) \ + ? __unsafe_string_function_usage_here_size_t__() \ + : srvstr_push_fn(fn_name, fn_line, base_ptr, dest, src, dest_len, flags)) + +#else + +#define safe_strcpy_fn2 safe_strcpy_fn +#define safe_strcat_fn2 safe_strcat_fn +#define push_string_fn2 push_string_fn +#define pull_string_fn2 pull_string_fn +#define clistr_push_fn2 clistr_push_fn +#define clistr_pull_fn2 clistr_pull_fn +#define srvstr_push_fn2 srvstr_push_fn + +#endif + +#endif