From 154c00e1a480d2bac7e8aba3b13888eb909f8e7f Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Fri, 24 Jan 2014 23:53:09 +0000 Subject: Fix gratuitous use of strdup and x_strdup There is no need to copy strings passed as arguments to execve, the only potentially noticeable effect of using strdup/x_strdup would be a malformed argument list in case of memory allocation error. Also, x_strdup, being a thin wrapper around strdup, is of no benefit when its argument is known to be non-NULL, and should not be used in such cases. * modules/pam_cracklib/pam_cracklib.c (password_check): Use strdup instead of x_strdup, the latter is of no benefit in this case. * modules/pam_ftp/pam_ftp.c (lookup): Likewise. * modules/pam_userdb/pam_userdb.c (user_lookup): Likewise. * modules/pam_userdb/pam_userdb.h (x_strdup): Remove. * modules/pam_mkhomedir/pam_mkhomedir.c (create_homedir): Do not use x_strdup for strings passed as arguments to execve. * modules/pam_unix/pam_unix_acct.c (_unix_run_verify_binary): Likewise. * modules/pam_unix/pam_unix_passwd.c (_unix_run_update_binary): Likewise. * modules/pam_unix/support.c (_unix_run_helper_binary): Likewise. (_unix_verify_password): Use strdup instead of x_strdup, the latter is of no benefit in this case. * modules/pam_xauth/pam_xauth.c (run_coprocess): Do not use strdup for strings passed as arguments to execv. --- modules/pam_cracklib/pam_cracklib.c | 6 +++--- modules/pam_ftp/pam_ftp.c | 2 +- modules/pam_mkhomedir/pam_mkhomedir.c | 12 ++++++------ modules/pam_unix/pam_unix_acct.c | 10 +++++----- modules/pam_unix/pam_unix_passwd.c | 16 ++++++++-------- modules/pam_unix/support.c | 16 ++++++++-------- modules/pam_userdb/pam_userdb.c | 2 +- modules/pam_userdb/pam_userdb.h | 3 --- modules/pam_xauth/pam_xauth.c | 12 +++++------- 9 files changed, 37 insertions(+), 42 deletions(-) (limited to 'modules') diff --git a/modules/pam_cracklib/pam_cracklib.c b/modules/pam_cracklib/pam_cracklib.c index 56913477..5eefd0ba 100644 --- a/modules/pam_cracklib/pam_cracklib.c +++ b/modules/pam_cracklib/pam_cracklib.c @@ -619,16 +619,16 @@ static const char *password_check(pam_handle_t *pamh, struct cracklib_options *o return msg; } - newmono = str_lower(x_strdup(new)); + newmono = str_lower(strdup(new)); if (!newmono) msg = _("memory allocation error"); - usermono = str_lower(x_strdup(user)); + usermono = str_lower(strdup(user)); if (!usermono) msg = _("memory allocation error"); if (!msg && old) { - oldmono = str_lower(x_strdup(old)); + oldmono = str_lower(strdup(old)); if (oldmono) wrapped = malloc(strlen(oldmono) * 2 + 1); if (wrapped) { diff --git a/modules/pam_ftp/pam_ftp.c b/modules/pam_ftp/pam_ftp.c index 896a1dda..221d8f87 100644 --- a/modules/pam_ftp/pam_ftp.c +++ b/modules/pam_ftp/pam_ftp.c @@ -81,7 +81,7 @@ static int lookup(const char *name, const char *list, const char **_user) char *list_copy, *x; char *sptr = NULL; - list_copy = x_strdup(list); + list_copy = strdup(list); x = list_copy; while (list_copy && (l = strtok_r(x, ",", &sptr))) { x = NULL; diff --git a/modules/pam_mkhomedir/pam_mkhomedir.c b/modules/pam_mkhomedir/pam_mkhomedir.c index 0b5fc752..a867a738 100644 --- a/modules/pam_mkhomedir/pam_mkhomedir.c +++ b/modules/pam_mkhomedir/pam_mkhomedir.c @@ -134,7 +134,7 @@ create_homedir (pam_handle_t *pamh, options_t *opt, int i; struct rlimit rlim; static char *envp[] = { NULL }; - char *args[] = { NULL, NULL, NULL, NULL, NULL }; + const char *args[] = { NULL, NULL, NULL, NULL, NULL }; if (getrlimit(RLIMIT_NOFILE, &rlim)==0) { if (rlim.rlim_max >= MAX_FD_NO) @@ -145,12 +145,12 @@ create_homedir (pam_handle_t *pamh, options_t *opt, } /* exec the mkhomedir helper */ - args[0] = x_strdup(MKHOMEDIR_HELPER); - args[1] = (char *) user; - args[2] = x_strdup(opt->umask); - args[3] = x_strdup(opt->skeldir); + args[0] = MKHOMEDIR_HELPER; + args[1] = user; + args[2] = opt->umask; + args[3] = opt->skeldir; - execve(MKHOMEDIR_HELPER, args, envp); + execve(MKHOMEDIR_HELPER, (char *const *) args, envp); /* should not get here: exit with error */ D(("helper binary is not available")); diff --git a/modules/pam_unix/pam_unix_acct.c b/modules/pam_unix/pam_unix_acct.c index 8ec44492..dc505e73 100644 --- a/modules/pam_unix/pam_unix_acct.c +++ b/modules/pam_unix/pam_unix_acct.c @@ -101,7 +101,7 @@ int _unix_run_verify_binary(pam_handle_t *pamh, unsigned int ctrl, int i=0; struct rlimit rlim; static char *envp[] = { NULL }; - char *args[] = { NULL, NULL, NULL, NULL }; + const char *args[] = { NULL, NULL, NULL, NULL }; /* reopen stdout as pipe */ dup2(fds[1], STDOUT_FILENO); @@ -130,11 +130,11 @@ int _unix_run_verify_binary(pam_handle_t *pamh, unsigned int ctrl, } /* exec binary helper */ - args[0] = x_strdup(CHKPWD_HELPER); - args[1] = x_strdup(user); - args[2] = x_strdup("chkexpiry"); + args[0] = CHKPWD_HELPER; + args[1] = user; + args[2] = "chkexpiry"; - execve(CHKPWD_HELPER, args, envp); + execve(CHKPWD_HELPER, (char *const *) args, envp); pam_syslog(pamh, LOG_ERR, "helper binary execve failed: %m"); /* should not get here: exit with error */ diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c index 0cfc0f4d..5f3a3db3 100644 --- a/modules/pam_unix/pam_unix_passwd.c +++ b/modules/pam_unix/pam_unix_passwd.c @@ -204,7 +204,7 @@ static int _unix_run_update_binary(pam_handle_t *pamh, unsigned int ctrl, const int i=0; struct rlimit rlim; static char *envp[] = { NULL }; - char *args[] = { NULL, NULL, NULL, NULL, NULL, NULL }; + const char *args[] = { NULL, NULL, NULL, NULL, NULL, NULL }; char buffer[16]; /* XXX - should really tidy up PAM here too */ @@ -222,18 +222,18 @@ static int _unix_run_update_binary(pam_handle_t *pamh, unsigned int ctrl, const } /* exec binary helper */ - args[0] = x_strdup(UPDATE_HELPER); - args[1] = x_strdup(user); - args[2] = x_strdup("update"); + args[0] = UPDATE_HELPER; + args[1] = user; + args[2] = "update"; if (on(UNIX_SHADOW, ctrl)) - args[3] = x_strdup("1"); + args[3] = "1"; else - args[3] = x_strdup("0"); + args[3] = "0"; snprintf(buffer, sizeof(buffer), "%d", remember); - args[4] = x_strdup(buffer); + args[4] = buffer; - execve(UPDATE_HELPER, args, envp); + execve(UPDATE_HELPER, (char *const *) args, envp); /* should not get here: exit with error */ D(("helper binary is not available")); diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 19d72e66..3a849c81 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -567,7 +567,7 @@ static int _unix_run_helper_binary(pam_handle_t *pamh, const char *passwd, int i=0; struct rlimit rlim; static char *envp[] = { NULL }; - char *args[] = { NULL, NULL, NULL, NULL }; + const char *args[] = { NULL, NULL, NULL, NULL }; /* XXX - should really tidy up PAM here too */ @@ -593,15 +593,15 @@ static int _unix_run_helper_binary(pam_handle_t *pamh, const char *passwd, } /* exec binary helper */ - args[0] = strdup(CHKPWD_HELPER); - args[1] = x_strdup(user); + args[0] = CHKPWD_HELPER; + args[1] = user; if (off(UNIX__NONULL, ctrl)) { /* this means we've succeeded */ - args[2]=strdup("nullok"); + args[2]="nullok"; } else { - args[2]=strdup("nonull"); + args[2]="nonull"; } - execve(CHKPWD_HELPER, args, envp); + execve(CHKPWD_HELPER, (char *const *) args, envp); /* should not get here: exit with error */ D(("helper binary is not available")); @@ -788,10 +788,10 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name login_name = ""; } - new->user = x_strdup(name ? name : ""); + new->user = strdup(name ? name : ""); new->uid = getuid(); new->euid = geteuid(); - new->name = x_strdup(login_name); + new->name = strdup(login_name); /* any previous failures for this user ? */ if (pam_get_data(pamh, data_name, &void_old) diff --git a/modules/pam_userdb/pam_userdb.c b/modules/pam_userdb/pam_userdb.c index ff040e6f..ba36ebf2 100644 --- a/modules/pam_userdb/pam_userdb.c +++ b/modules/pam_userdb/pam_userdb.c @@ -184,7 +184,7 @@ user_lookup (pam_handle_t *pamh, const char *database, const char *cryptmode, else key.dsize = strlen(key.dptr); } else { - key.dptr = x_strdup(user); + key.dptr = strdup(user); key.dsize = strlen(user); } diff --git a/modules/pam_userdb/pam_userdb.h b/modules/pam_userdb/pam_userdb.h index 3cd8fee0..86e9b47f 100644 --- a/modules/pam_userdb/pam_userdb.h +++ b/modules/pam_userdb/pam_userdb.h @@ -15,9 +15,6 @@ #define PAM_USE_FPASS_ARG 0x0040 #define PAM_TRY_FPASS_ARG 0x0080 -/* Useful macros */ -#define x_strdup(s) ( (s) ? strdup(s):NULL ) - /* The name of the module we are compiling */ #ifndef MODULE_NAME #define MODULE_NAME "pam_userdb" diff --git a/modules/pam_xauth/pam_xauth.c b/modules/pam_xauth/pam_xauth.c index e1c00646..70755475 100644 --- a/modules/pam_xauth/pam_xauth.c +++ b/modules/pam_xauth/pam_xauth.c @@ -127,8 +127,7 @@ run_coprocess(pam_handle_t *pamh, const char *input, char **output, if (child == 0) { /* We're the child. */ size_t j; - char *args[10]; - const char *tmp; + const char *args[10]; int maxopened; /* Drop privileges. */ if (setgid(gid) == -1) @@ -166,16 +165,15 @@ run_coprocess(pam_handle_t *pamh, const char *input, char **output, } /* Convert the varargs list into a regular array of strings. */ va_start(ap, command); - args[0] = strdup(command); + args[0] = command; for (j = 1; j < ((sizeof(args) / sizeof(args[0])) - 1); j++) { - tmp = va_arg(ap, const char*); - if (tmp == NULL) { + args[j] = va_arg(ap, const char*); + if (args[j] == NULL) { break; } - args[j] = strdup(tmp); } /* Run the command. */ - execv(command, args); + execv(command, (char *const *) args); /* Never reached. */ _exit(1); } -- cgit v1.2.3