From fc78bb8523d8d6a2c90ded155b555e956156c2b1 Mon Sep 17 00:00:00 2001 From: "Andrew G. Morgan" Date: Mon, 26 Nov 2001 06:05:24 +0000 Subject: Relevant BUGIDs: 476947 Purpose of commit: cleanup Commit summary: --------------- be more confident that strings are being initialized correctly from Nalin. --- CHANGELOG | 3 ++ modules/pam_cracklib/pam_cracklib.c | 19 +++++++----- modules/pam_listfile/pam_listfile.c | 62 +++++++++++++++++++++---------------- modules/pam_unix/pam_unix_passwd.c | 9 ++++-- modules/pam_unix/support.c | 3 +- modules/pam_unix/unix_chkpwd.c | 5 ++- modules/pam_wheel/pam_wheel.c | 11 ++++--- 7 files changed, 67 insertions(+), 45 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 37fb76b9..9e342d85 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -49,6 +49,9 @@ bug report - outstanding bugs are listed here: 0.76: please submit patches for this section with actual code/doc patches! +* misc string comparison length checking changes from Nalin. Modules + touched, pam_cracklib, pam_listfile, pam_unix, pam_wheel (Bug 476947 - + agmorgan) * pam_userdb: require that all of typed password matches that in database report and fix from Vladimir Pastukhov. (Bug 484252 - agmorgan) * pam_malloc: revived malloc debugging code, now tied to diff --git a/modules/pam_cracklib/pam_cracklib.c b/modules/pam_cracklib/pam_cracklib.c index 1277619e..df17171f 100644 --- a/modules/pam_cracklib/pam_cracklib.c +++ b/modules/pam_cracklib/pam_cracklib.c @@ -55,8 +55,8 @@ extern char *FascistCheck(char *pw, const char *dictpath); #define CRACKLIB_DICTPATH "/usr/share/dict/cracklib_dict" #endif -#define PROMPT1 "New %s password: " -#define PROMPT2 "Retype new %s password: " +#define PROMPT1 "New %s%spassword: " +#define PROMPT2 "Retype new %s%spassword: " #define MISTYPED_PASS "Sorry, passwords do not match" /* @@ -128,7 +128,7 @@ static int _pam_parse(struct cracklib_options *opt, int argc, const char **argv) if (!strcmp(*argv,"debug")) ctrl |= PAM_DEBUG_ARG; else if (!strncmp(*argv,"type=",5)) - strcpy(opt->prompt_type, *argv+5); + strncpy(opt->prompt_type, *argv+5, sizeof(opt->prompt_type) - 1); else if (!strncmp(*argv,"retry=",6)) { opt->retry_times = strtol(*argv+6,&ep,10); if (!ep || (opt->retry_times < 1)) @@ -167,6 +167,7 @@ static int _pam_parse(struct cracklib_options *opt, int argc, const char **argv) _pam_log(LOG_ERR,"pam_parse: unknown option; %s",*argv); } } + opt->prompt_type[sizeof(opt->prompt_type) - 1] = '\0'; return ctrl; } @@ -465,7 +466,7 @@ static int _pam_unix_approve_pass(pam_handle_t *pamh, char remark[BUFSIZ]; memset(remark,0,sizeof(remark)); - sprintf(remark,"BAD PASSWORD: %s",msg); + snprintf(remark,sizeof(remark),"BAD PASSWORD: %s",msg); if (ctrl && PAM_DEBUG_ARG) _pam_log(LOG_NOTICE, "new passwd fails strength check: %s", msg); @@ -510,7 +511,7 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t *pamh, int flags, D(("prelim check")); memset(buf,0,sizeof(buf)); /* zero the buffer */ - sprintf(buf,"%s.pwd",CRACKLIB_DICTPATH); + snprintf(buf,sizeof(buf),"%s.pwd",CRACKLIB_DICTPATH); if (!stat(buf,&st) && st.st_size) return PAM_SUCCESS; @@ -581,7 +582,8 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t *pamh, int flags, } else { /* Prepare to ask the user for the first time */ memset(prompt,0,sizeof(prompt)); - sprintf(prompt,PROMPT1,options.prompt_type); + snprintf(prompt,sizeof(prompt),PROMPT1, + options.prompt_type, options.prompt_type[0]?" ":""); pmsg[0] = &msg[0]; msg[0].msg_style = PAM_PROMPT_ECHO_OFF; msg[0].msg = prompt; @@ -625,7 +627,7 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t *pamh, int flags, if ((crack_msg = FascistCheck(token1, cracklib_dictpath))) { if (ctrl && PAM_DEBUG_ARG) _pam_log(LOG_DEBUG,"bad password: %s",crack_msg); - sprintf(remark,"BAD PASSWORD: %s", crack_msg); + snprintf(remark,sizeof(remark),"BAD PASSWORD: %s", crack_msg); make_remark(pamh, ctrl, PAM_ERROR_MSG, remark); if (getuid() || (flags & PAM_CHANGE_EXPIRED_AUTHTOK)) retval = PAM_AUTHTOK_ERR; @@ -661,7 +663,8 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t *pamh, int flags, if (options.use_authtok == 0) { bzero(prompt,sizeof(prompt)); - sprintf(prompt,PROMPT2,options.prompt_type); + sprintf(prompt,sizeof(prompt),PROMPT2, + options.prompt_type, options.prompt_type[0]?" ":""); pmsg[0] = &msg[0]; msg[0].msg_style = PAM_PROMPT_ECHO_OFF; msg[0].msg = prompt; diff --git a/modules/pam_listfile/pam_listfile.c b/modules/pam_listfile/pam_listfile.c index 5a8c83e0..b560b4b6 100644 --- a/modules/pam_listfile/pam_listfile.c +++ b/modules/pam_listfile/pam_listfile.c @@ -75,20 +75,16 @@ static int is_on_group(const char *user_name, const char *group_name) if (!strlen(group_name)) return 0; bzero(uname, sizeof(uname)); - strncpy(uname, user_name, BUFSIZ-1); + strncpy(uname, user_name, sizeof(uname)-1); bzero(gname, sizeof(gname)); - strncpy(gname, group_name, BUFSIZ-1); + strncpy(gname, group_name, sizeof(gname)-1); - setpwent(); pwd = getpwnam(uname); - endpwent(); if (!pwd) return 0; /* the info about this group */ - setgrent(); grp = getgrnam(gname); - endgrent(); if (!grp) return 0; @@ -97,9 +93,7 @@ static int is_on_group(const char *user_name, const char *group_name) return 1; /* next check: user primary group is group_name ? */ - setgrent(); pgrp = getgrgid(pwd->pw_gid); - endgrent(); if (!pgrp) return 0; if (!strcmp(pgrp->gr_name, gname)) @@ -120,6 +114,8 @@ static int is_on_group(const char *user_name, const char *group_name) #define APPLY_TYPE_USER 2 #define APPLY_TYPE_GROUP 3 +#define LESSER(a, b) ((a) < (b) ? (a) : (b)) + PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **argv) { @@ -145,15 +141,18 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **ar for(i=0; i < argc; i++) { { - char *junk; - junk = (char *) malloc(strlen(argv[i])+1); - if (junk == NULL) { - return PAM_BUF_ERR; + const char *junk; + + memset(mybuf,'\0',sizeof(mybuf)); + memset(myval,'\0',sizeof(mybuf)); + junk = strchr(argv[i], '='); + if((junk == NULL) || (junk - argv[i]) >= sizeof(mybuf)) { + _pam_log(LOG_ERR,LOCAL_LOG_PREFIX "Bad option: \"%s\"", + argv[i]); + continue; } - strcpy(junk,argv[i]); - strncpy(mybuf,strtok(junk,"="),255); - strncpy(myval,strtok(NULL,"="),255); - free(junk); + strncpy(mybuf, argv[i], LESSER(junk - argv[i], sizeof(mybuf) - 1)); + strncpy(myval, junk + 1, sizeof(myval) - 1); } if(!strcmp(mybuf,"onerr")) if(!strcmp(myval,"succeed")) @@ -192,6 +191,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **ar citem = 0; } else if(!strcmp(mybuf,"apply")) { apply_type=APPLY_TYPE_NONE; + memset(apply_val,'\0',sizeof(apply_val)); if (myval[0]=='@') { apply_type=APPLY_TYPE_GROUP; strncpy(apply_val,myval+1,sizeof(apply_val)-1); @@ -290,10 +290,18 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **ar if(extitem) { switch(extitem) { case EI_GROUP: - setpwent(); userinfo = getpwnam(citemp); - setgrent(); + if (userinfo == NULL) { + _pam_log(LOG_ERR,LOCAL_LOG_PREFIX "getpwnam(%s) failed", + citemp); + return onerr; + } grpinfo = getgrgid(userinfo->pw_gid); + if (grpinfo == NULL) { + _pam_log(LOG_ERR,LOCAL_LOG_PREFIX "getgrgid(%d) failed", + (int)userinfo->pw_gid); + return onerr; + } itemlist[0] = x_strdup(grpinfo->gr_name); setgrent(); for (i=1; (i < sizeof(itemlist)/sizeof(itemlist[0])-1) && @@ -302,18 +310,20 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **ar itemlist[i++] = x_strdup(grpinfo->gr_name); } } - itemlist[i] = NULL; endgrent(); - endpwent(); + itemlist[i] = NULL; break; case EI_SHELL: - setpwent(); - userinfo = getpwnam(citemp); /* Assume that we have already gotten - PAM_USER in pam_get_item() - a valid - assumption since citem gets set to - PAM_USER in the extitem switch */ + /* Assume that we have already gotten PAM_USER in + pam_get_item() - a valid assumption since citem + gets set to PAM_USER in the extitem switch */ + userinfo = getpwnam(citemp); + if (userinfo == NULL) { + _pam_log(LOG_ERR,LOCAL_LOG_PREFIX "getpwnam(%s) failed", + citemp); + return onerr; + } citemp = userinfo->pw_shell; - endpwent(); break; default: _pam_log(LOG_ERR, diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c index 7f8f6e03..7870f320 100644 --- a/modules/pam_unix/pam_unix_passwd.c +++ b/modules/pam_unix/pam_unix_passwd.c @@ -285,9 +285,11 @@ static int save_old_password(const char *forwho, const char *oldpass, int howman } pass = crypt_md5_wrapper(oldpass); if (s_pas == NULL) - sprintf(nbuf, "%s:%s:%d:%s\n", s_luser, s_uid, npas, pass); + snprintf(nbuf, sizeof(nbuf), "%s:%s:%d:%s\n", + s_luser, s_uid, npas, pass); else - sprintf(nbuf, "%s:%s:%d:%s,%s\n", s_luser, s_uid, npas, s_pas, pass); + snprintf(nbuf, sizeof(nbuf),"%s:%s:%d:%s,%s\n", + s_luser, s_uid, npas, s_pas, pass); _pam_delete(pass); if (fputs(nbuf, pwfile) < 0) { retval = PAM_AUTHTOK_ERR; @@ -309,7 +311,8 @@ static int save_old_password(const char *forwho, const char *oldpass, int howman err = 1; } else { pass = crypt_md5_wrapper(oldpass); - sprintf(nbuf, "%s:%d:1:%s\n", forwho, pwd->pw_uid, pass); + snprintf(nbuf, sizeof(nbuf), "%s:%d:1:%s\n", + forwho, pwd->pw_uid, pass); _pam_delete(pass); if (fputs(nbuf, pwfile) < 0) { retval = PAM_AUTHTOK_ERR; diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 6b8b8020..9b6b19a2 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -123,9 +123,10 @@ char *PAM_getlogin(void) D(("PAM_getlogin ttyname: %s", curr_tty)); curr_tty += 5; setutent(); - strncpy(line.ut_line, curr_tty, sizeof line.ut_line); + strncpy(line.ut_line, curr_tty, sizeof(line.ut_line)); if ((ut = getutline(&line)) != NULL) { strncpy(curr_user, ut->ut_user, sizeof(ut->ut_user)); + curr_user[sizeof(curr_user) - 1] = '\0'; retval = curr_user; } endutent(); diff --git a/modules/pam_unix/unix_chkpwd.c b/modules/pam_unix/unix_chkpwd.c index b0509e47..592ea5b3 100644 --- a/modules/pam_unix/unix_chkpwd.c +++ b/modules/pam_unix/unix_chkpwd.c @@ -172,9 +172,8 @@ static char *getuidname(uid_t uid) if (pw == NULL) return NULL; - memset(username, 0, 32); - strncpy(username, pw->pw_name, 32); - username[31] = '\0'; + strncpy(username, pw->pw_name, sizeof(username)); + username[sizeof(username) - 1] = '\0'; return username; } diff --git a/modules/pam_wheel/pam_wheel.c b/modules/pam_wheel/pam_wheel.c index add72bc4..d629819f 100644 --- a/modules/pam_wheel/pam_wheel.c +++ b/modules/pam_wheel/pam_wheel.c @@ -75,10 +75,13 @@ static int is_on_list(char * const *list, const char *member) #define PAM_TRUST_ARG 0x0004 #define PAM_DENY_ARG 0x0010 -static int _pam_parse(int argc, const char **argv, char *use_group) +static int _pam_parse(int argc, const char **argv, char *use_group, + size_t group_length) { int ctrl=0; + memset(use_group, '\0', group_length); + /* step through arguments */ for (ctrl=0; argc-- > 0; ++argv) { @@ -93,7 +96,7 @@ static int _pam_parse(int argc, const char **argv, char *use_group) else if (!strcmp(*argv,"deny")) ctrl |= PAM_DENY_ARG; else if (!strncmp(*argv,"group=",6)) - strcpy(use_group,*argv+6); + strncpy(use_group,*argv+6,group_length-1); else { _pam_log(LOG_ERR,"pam_parse: unknown option; %s",*argv); } @@ -120,8 +123,8 @@ int pam_sm_authenticate(pam_handle_t *pamh,int flags,int argc /* Init the optional group */ bzero(use_group,BUFSIZ); - ctrl = _pam_parse(argc, argv, use_group); - retval = pam_get_user(pamh,&username,NULL); + ctrl = _pam_parse(argc, argv, use_group, sizeof(use_group)); + retval = pam_get_user(pamh, &username, NULL); if ((retval != PAM_SUCCESS) || (!username)) { if (ctrl & PAM_DEBUG_ARG) _pam_log(LOG_DEBUG,"can not get the username"); -- cgit v1.2.3