From 6caedeff52ee6ae5afce19d22798f895f101a1f1 Mon Sep 17 00:00:00 2001 From: Julian Kranz Date: Fri, 2 Jun 2023 04:45:03 +0200 Subject: pam_unix: improve fallback values for "rounds" for yescrypt and blowfish This change improves the fallback values for the "rounds" parameter for yescrypt and blowfish by using the smallest reasonable value if the user sets a too low value and by using the highest reasonable value if the user sets a too high value. This better realizes user intent and is consistent with the approach taken for SHA256. --- modules/pam_unix/support.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 043273d2..7bed0a56 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -189,11 +189,15 @@ unsigned long long _set_ctrl(pam_handle_t *pamh, int flags, int *remember, if (on(UNIX_ALGO_ROUNDS, ctrl)) { if (on(UNIX_GOST_YESCRYPT_PASS, ctrl) || on(UNIX_YESCRYPT_PASS, ctrl)) { - if (*rounds < 3 || *rounds > 11) - *rounds = 5; + if (*rounds < 3) + *rounds = 3; + else if (*rounds > 11) + *rounds = 11; } else if (on(UNIX_BLOWFISH_PASS, ctrl)) { - if (*rounds < 4 || *rounds > 31) - *rounds = 5; + if (*rounds < 4) + *rounds = 4; + else if (*rounds > 31) + *rounds = 31; } else if (on(UNIX_SHA256_PASS, ctrl) || on(UNIX_SHA512_PASS, ctrl)) { if ((*rounds < 1000) || (*rounds == INT_MAX)) { /* don't care about bogus values */ -- cgit v1.2.3 From 43abfff43537092e20bc129f8208d082e73aff1a Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Mon, 7 Aug 2023 12:46:40 +0200 Subject: modules: cast to unsigned char for character handling function Character handling functions, like isspace(3), expect a value representable as unsigned char or equal to EOF. Otherwise the behavior is undefined. See https://wiki.sei.cmu.edu/confluence/display/c/STR37-C.+Arguments+to+character-handling+functions+must+be+representable+as+an+unsigned+char --- modules/pam_access/pam_access.c | 2 +- modules/pam_env/pam_env.c | 2 +- modules/pam_faillock/faillock_config.c | 8 ++++---- modules/pam_filter/upperLOWER/upperLOWER.c | 6 +++--- modules/pam_group/pam_group.c | 16 ++++++++-------- modules/pam_lastlog/pam_lastlog.c | 2 +- modules/pam_limits/pam_limits.c | 8 ++++---- modules/pam_namespace/argv_parse.c | 4 ++-- modules/pam_namespace/pam_namespace.c | 2 +- modules/pam_pwhistory/opasswd.c | 4 ++-- modules/pam_securetty/pam_securetty.c | 2 +- modules/pam_sepermit/pam_sepermit.c | 4 ++-- modules/pam_time/pam_time.c | 14 +++++++------- modules/pam_unix/support.c | 2 +- modules/pam_usertype/pam_usertype.c | 2 +- 15 files changed, 39 insertions(+), 39 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_access/pam_access.c b/modules/pam_access/pam_access.c index 985dc7de..04d7306b 100644 --- a/modules/pam_access/pam_access.c +++ b/modules/pam_access/pam_access.c @@ -456,7 +456,7 @@ login_access (pam_handle_t *pamh, struct login_info *item) } if (line[0] == '#') continue; /* comment line */ - while (end > 0 && isspace(line[end - 1])) + while (end > 0 && isspace((unsigned char)line[end - 1])) end--; line[end] = 0; /* strip trailing whitespace */ if (line[0] == 0) /* skip blank lines */ diff --git a/modules/pam_env/pam_env.c b/modules/pam_env/pam_env.c index d2b4cb10..d2d12ad6 100644 --- a/modules/pam_env/pam_env.c +++ b/modules/pam_env/pam_env.c @@ -965,7 +965,7 @@ _parse_env_file(pam_handle_t *pamh, int ctrl, const char *file) } for ( i = 0 ; key[i] != '=' && key[i] != '\0' ; i++ ) - if (!isalnum(key[i]) && key[i] != '_') { + if (!isalnum((unsigned char)key[i]) && key[i] != '_') { pam_syslog(pamh, LOG_ERR, "non-alphanumeric key '%s' in %s', ignoring", key, file); diff --git a/modules/pam_faillock/faillock_config.c b/modules/pam_faillock/faillock_config.c index 0d14aad1..5d796240 100644 --- a/modules/pam_faillock/faillock_config.c +++ b/modules/pam_faillock/faillock_config.c @@ -121,7 +121,7 @@ read_config_file(pam_handle_t *pamh, struct options *opts, const char *cfgfile) /* drop terminating whitespace including the \n */ while (ptr > linebuf) { - if (!isspace(*(ptr-1))) { + if (!isspace((unsigned char)*(ptr-1))) { *ptr = '\0'; break; } @@ -129,7 +129,7 @@ read_config_file(pam_handle_t *pamh, struct options *opts, const char *cfgfile) } /* skip initial whitespace */ - for (ptr = linebuf; isspace(*ptr); ptr++); + for (ptr = linebuf; isspace((unsigned char)*ptr); ptr++); if (*ptr == '\0') continue; @@ -137,7 +137,7 @@ read_config_file(pam_handle_t *pamh, struct options *opts, const char *cfgfile) eq = 0; name = ptr; while (*ptr != '\0') { - if (isspace(*ptr) || *ptr == '=') { + if (isspace((unsigned char)*ptr) || *ptr == '=') { eq = *ptr == '='; *ptr = '\0'; ++ptr; @@ -149,7 +149,7 @@ read_config_file(pam_handle_t *pamh, struct options *opts, const char *cfgfile) /* grab the key value */ while (*ptr != '\0') { if (*ptr != '=' || eq) { - if (!isspace(*ptr)) { + if (!isspace((unsigned char)*ptr)) { break; } } else { diff --git a/modules/pam_filter/upperLOWER/upperLOWER.c b/modules/pam_filter/upperLOWER/upperLOWER.c index 25e70a5a..700e0ed1 100644 --- a/modules/pam_filter/upperLOWER/upperLOWER.c +++ b/modules/pam_filter/upperLOWER/upperLOWER.c @@ -24,10 +24,10 @@ static void do_transpose(char *buffer,int len) { int i; for (i=0; i 0 && isalpha(times[j]); --len) { + for (marked_day = 0; len > 0 && isalpha((unsigned char)times[j]); --len) { int this_day=-1; D(("%c%c ?", times[j], times[j+1])); for (i=0; days[i].d != NULL; ++i) { - if (tolower(times[j]) == days[i].d[0] - && tolower(times[j+1]) == days[i].d[1] ) { + if (tolower((unsigned char)times[j]) == days[i].d[0] + && tolower((unsigned char)times[j+1]) == days[i].d[1] ) { this_day = days[i].bit; break; } @@ -419,7 +419,7 @@ check_time (const pam_handle_t *pamh, const void *AT, D(("day range = 0%o", marked_day)); time_start = 0; - for (i=0; len > 0 && i < 4 && isdigit(times[i+j]); ++i, --len) { + for (i=0; len > 0 && i < 4 && isdigit((unsigned char)times[i+j]); ++i, --len) { time_start *= 10; time_start += times[i+j]-'0'; /* is this portable? */ } @@ -427,7 +427,7 @@ check_time (const pam_handle_t *pamh, const void *AT, if (times[j] == '-') { time_end = 0; - for (i=1; len > 0 && i < 5 && isdigit(times[i+j]); ++i, --len) { + for (i=1; len > 0 && i < 5 && isdigit((unsigned char)times[i+j]); ++i, --len) { time_end *= 10; time_end += times[i+j]-'0'; /* is this portable? */ } @@ -497,7 +497,7 @@ static int find_member(const char *string, int *at) break; default: - if (isalpha(c) || isdigit(c) || c == '_' || c == '*' + if (isalpha((unsigned char)c) || isdigit((unsigned char)c) || c == '_' || c == '*' || c == '-') { token = 1; } else if (token) { diff --git a/modules/pam_lastlog/pam_lastlog.c b/modules/pam_lastlog/pam_lastlog.c index ec515f56..831dbe3b 100644 --- a/modules/pam_lastlog/pam_lastlog.c +++ b/modules/pam_lastlog/pam_lastlog.c @@ -205,7 +205,7 @@ get_lastlog_uid_max(pam_handle_t *pamh) return uid_max; ep = s + strlen(s); - while (ep > s && isspace(*(--ep))) { + while (ep > s && isspace((unsigned char)*(--ep))) { *ep = '\0'; } errno = 0; diff --git a/modules/pam_limits/pam_limits.c b/modules/pam_limits/pam_limits.c index 8b1755b7..cc41435f 100644 --- a/modules/pam_limits/pam_limits.c +++ b/modules/pam_limits/pam_limits.c @@ -852,7 +852,7 @@ parse_config_file(pam_handle_t *pamh, const char *uname, uid_t uid, gid_t gid, line = buf; /* skip the leading white space */ - while (*line && isspace(*line)) + while (*line && isspace((unsigned char)*line)) line++; /* Rip off the comments */ @@ -874,7 +874,7 @@ parse_config_file(pam_handle_t *pamh, const char *uname, uid_t uid, gid_t gid, i, domain, ltype, item, value)); for(j=0; j < strlen(ltype); j++) - ltype[j]=tolower(ltype[j]); + ltype[j]=tolower((unsigned char)ltype[j]); if ((rngtype=parse_uid_range(pamh, domain, &min_uid, &max_uid)) < 0) { pam_syslog(pamh, LOG_WARNING, "invalid uid range '%s' - skipped", domain); @@ -883,9 +883,9 @@ parse_config_file(pam_handle_t *pamh, const char *uname, uid_t uid, gid_t gid, if (i == 4) { /* a complete line */ for(j=0; j < strlen(item); j++) - item[j]=tolower(item[j]); + item[j]=tolower((unsigned char)item[j]); for(j=0; j < strlen(value); j++) - value[j]=tolower(value[j]); + value[j]=tolower((unsigned char)value[j]); if (strcmp(uname, domain) == 0) /* this user have a limit */ process_limit(pamh, LIMITS_DEF_USER, ltype, item, value, ctrl, pl); diff --git a/modules/pam_namespace/argv_parse.c b/modules/pam_namespace/argv_parse.c index 40510542..7d418e05 100644 --- a/modules/pam_namespace/argv_parse.c +++ b/modules/pam_namespace/argv_parse.c @@ -56,7 +56,7 @@ int argv_parse(const char *in_buf, int *ret_argc, char ***ret_argv) outcp = buf; for (cp = in_buf; (ch = *cp); cp++) { if (state == STATE_WHITESPACE) { - if (isspace((int) ch)) + if (isspace((unsigned char)ch)) continue; /* Not whitespace, so start a new token */ state = STATE_TOKEN; @@ -81,7 +81,7 @@ int argv_parse(const char *in_buf, int *ret_argc, char ***ret_argv) continue; } /* Must be processing characters in a word */ - if (isspace((int) ch)) { + if (isspace((unsigned char)ch)) { /* * Terminate the current word and start * looking for the beginning of the next word. diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index f34ce934..48690b8d 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -485,7 +485,7 @@ static int process_line(char *line, const char *home, const char *rhome, /* * skip the leading white space */ - while (*line && isspace(*line)) + while (*line && isspace((unsigned char)*line)) line++; /* diff --git a/modules/pam_pwhistory/opasswd.c b/modules/pam_pwhistory/opasswd.c index fc610e2f..6e44ed2a 100644 --- a/modules/pam_pwhistory/opasswd.c +++ b/modules/pam_pwhistory/opasswd.c @@ -199,7 +199,7 @@ check_old_pass, const char *user, const char *newpass, const char *filename, int tmp = strchr (cp, '#'); /* remove comments */ if (tmp) *tmp = '\0'; - while (isspace ((int)*cp)) /* remove spaces and tabs */ + while (isspace ((unsigned char)*cp)) /* remove spaces and tabs */ ++cp; if (*cp == '\0') /* ignore empty lines */ continue; @@ -420,7 +420,7 @@ save_old_pass, const char *user, int howmany, const char *filename, int debug UN tmp = strchr (cp, '#'); /* remove comments */ if (tmp) *tmp = '\0'; - while (isspace ((int)*cp)) /* remove spaces and tabs */ + while (isspace ((unsigned char)*cp)) /* remove spaces and tabs */ ++cp; if (*cp == '\0') /* ignore empty lines */ goto write_old_data; diff --git a/modules/pam_securetty/pam_securetty.c b/modules/pam_securetty/pam_securetty.c index 47a5cd9f..837c871b 100644 --- a/modules/pam_securetty/pam_securetty.c +++ b/modules/pam_securetty/pam_securetty.c @@ -148,7 +148,7 @@ securetty_perform_check (pam_handle_t *pamh, int ctrl, return PAM_SERVICE_ERR; } - if (isdigit(uttyname[0])) { + if (isdigit((unsigned char)uttyname[0])) { snprintf(ptname, sizeof(ptname), "pts/%s", uttyname); } else { ptname[0] = '\0'; diff --git a/modules/pam_sepermit/pam_sepermit.c b/modules/pam_sepermit/pam_sepermit.c index 5fbc8fdd..7e00a77f 100644 --- a/modules/pam_sepermit/pam_sepermit.c +++ b/modules/pam_sepermit/pam_sepermit.c @@ -299,10 +299,10 @@ sepermit_match(pam_handle_t *pamh, const char *cfgfile, const char *user, continue; start = line; - while (isspace(*start)) + while (isspace((unsigned char)*start)) ++start; n = strlen(start); - while (n > 0 && isspace(start[n-1])) { + while (n > 0 && isspace((unsigned char)start[n-1])) { --n; } if (n == 0) diff --git a/modules/pam_time/pam_time.c b/modules/pam_time/pam_time.c index 6b7adefc..af276059 100644 --- a/modules/pam_time/pam_time.c +++ b/modules/pam_time/pam_time.c @@ -286,7 +286,7 @@ logic_member(const char *string, int *at) break; default: - if (isalpha(c) || c == '*' || isdigit(c) || c == '_' + if (isalpha((unsigned char)c) || c == '*' || isdigit((unsigned char)c) || c == '_' || c == '-' || c == '.' || c == '/' || c == ':') { token = 1; } else if (token) { @@ -319,7 +319,7 @@ logic_field(pam_handle_t *pamh, const void *me, const char *x, int rule, if (next == VAL) { if (c == '!') not = !not; - else if (isalpha(c) || c == '*' || isdigit(c) || c == '_' + else if (isalpha((unsigned char)c) || c == '*' || isdigit((unsigned char)c) || c == '_' || c == '-' || c == '.' || c == '/' || c == ':') { right = not ^ agrees(pamh, me, x+at, l, rule); if (oper == AND) @@ -449,13 +449,13 @@ check_time(pam_handle_t *pamh, const void *AT, const char *times, not = FALSE; } - for (marked_day = 0; len > 0 && isalpha(times[j]); --len) { + for (marked_day = 0; len > 0 && isalpha((unsigned char)times[j]); --len) { int this_day=-1; D(("%c%c ?", times[j], times[j+1])); for (i=0; days[i].d != NULL; ++i) { - if (tolower(times[j]) == days[i].d[0] - && tolower(times[j+1]) == days[i].d[1] ) { + if (tolower((unsigned char)times[j]) == days[i].d[0] + && tolower((unsigned char)times[j+1]) == days[i].d[1] ) { this_day = days[i].bit; break; } @@ -474,7 +474,7 @@ check_time(pam_handle_t *pamh, const void *AT, const char *times, D(("day range = 0%o", marked_day)); time_start = 0; - for (i=0; len > 0 && i < 4 && isdigit(times[i+j]); ++i, --len) { + for (i=0; len > 0 && i < 4 && isdigit((unsigned char)times[i+j]); ++i, --len) { time_start *= 10; time_start += times[i+j]-'0'; /* is this portable? */ } @@ -482,7 +482,7 @@ check_time(pam_handle_t *pamh, const void *AT, const char *times, if (times[j] == '-') { time_end = 0; - for (i=1; len > 0 && i < 5 && isdigit(times[i+j]); ++i, --len) { + for (i=1; len > 0 && i < 5 && isdigit((unsigned char)times[i+j]); ++i, --len) { time_end *= 10; time_end += times[i+j]-'0'; /* is this portable */ } diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 7bed0a56..a84ced60 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -327,7 +327,7 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, if ((buf[userlen] == ':') && (strncmp(name, buf, userlen) == 0)) { p = buf + strlen(buf) - 1; - while (isspace(*p) && (p >= buf)) { + while (isspace((unsigned char)*p) && (p >= buf)) { *p-- = '\0'; } matched = 1; diff --git a/modules/pam_usertype/pam_usertype.c b/modules/pam_usertype/pam_usertype.c index cfd9c8bb..a50a5a7c 100644 --- a/modules/pam_usertype/pam_usertype.c +++ b/modules/pam_usertype/pam_usertype.c @@ -169,7 +169,7 @@ pam_usertype_get_id(pam_handle_t *pamh, /* taken from get_lastlog_uid_max() */ ep = value + strlen(value); - while (ep > value && isspace(*(--ep))) { + while (ep > value && isspace((unsigned char)*(--ep))) { *ep = '\0'; } -- cgit v1.2.3 From da3bc2fc01c2443486ac1d241c4a09eaa71083c6 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sat, 11 Nov 2023 12:11:50 +0100 Subject: treewide: do not cast calloc/malloc/realloc It is not required to cast the results of calloc, malloc, realloc, etc. Signed-off-by: Tobias Stoeckmann --- libpam/pam_env.c | 6 +++--- libpam/pam_item.c | 3 +-- libpam_misc/misc_conv.c | 3 +-- modules/pam_filter/pam_filter.c | 8 ++++---- modules/pam_group/pam_group.c | 5 ++--- modules/pam_listfile/pam_listfile.c | 2 +- modules/pam_namespace/pam_namespace.c | 4 ++-- modules/pam_time/pam_time.c | 2 +- modules/pam_timestamp/hmac_openssl_wrapper.c | 2 +- modules/pam_unix/support.c | 2 +- 10 files changed, 17 insertions(+), 20 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/libpam/pam_env.c b/libpam/pam_env.c index 276f3d1e..2b3e3953 100644 --- a/libpam/pam_env.c +++ b/libpam/pam_env.c @@ -62,7 +62,7 @@ int _pam_make_env(pam_handle_t *pamh) * get structure memory */ - pamh->env = (struct pam_environ *) malloc(sizeof(struct pam_environ)); + pamh->env = malloc(sizeof(struct pam_environ)); if (pamh->env == NULL) { pam_syslog(pamh, LOG_CRIT, "_pam_make_env: out of memory"); return PAM_BUF_ERR; @@ -72,7 +72,7 @@ int _pam_make_env(pam_handle_t *pamh) * get list memory */ - pamh->env->list = (char **)calloc( PAM_ENV_CHUNK, sizeof(char *) ); + pamh->env->list = calloc( PAM_ENV_CHUNK, sizeof(char *) ); if (pamh->env->list == NULL) { pam_syslog(pamh, LOG_CRIT, "_pam_make_env: no memory for list"); _pam_drop(pamh->env); @@ -333,7 +333,7 @@ static char **_copy_env(pam_handle_t *pamh) D(("now get some memory for dump")); /* allocate some memory for this (plus the null tail-pointer) */ - dump = (char **) calloc(i, sizeof(char *)); + dump = calloc(i, sizeof(char *)); D(("dump = %p", dump)); if (dump == NULL) { return NULL; diff --git a/libpam/pam_item.c b/libpam/pam_item.c index 57327e1c..b504d598 100644 --- a/libpam/pam_item.c +++ b/libpam/pam_item.c @@ -113,8 +113,7 @@ int pam_set_item (pam_handle_t *pamh, int item_type, const void *item) } else { struct pam_conv *tconv; - if ((tconv= - (struct pam_conv *) malloc(sizeof(struct pam_conv)) + if ((tconv = malloc(sizeof(struct pam_conv)) ) == NULL) { pam_syslog(pamh, LOG_CRIT, "pam_set_item: malloc failed for pam_conv"); diff --git a/libpam_misc/misc_conv.c b/libpam_misc/misc_conv.c index dbcd6aec..6c23fcb8 100644 --- a/libpam_misc/misc_conv.c +++ b/libpam_misc/misc_conv.c @@ -285,8 +285,7 @@ int misc_conv(int num_msg, const struct pam_message **msgm, D(("allocating empty response structure array.")); - reply = (struct pam_response *) calloc(num_msg, - sizeof(struct pam_response)); + reply = calloc(num_msg, sizeof(struct pam_response)); if (reply == NULL) { D(("no memory for responses")); return PAM_CONV_ERR; diff --git a/modules/pam_filter/pam_filter.c b/modules/pam_filter/pam_filter.c index 6e6def37..a3c3bb24 100644 --- a/modules/pam_filter/pam_filter.c +++ b/modules/pam_filter/pam_filter.c @@ -103,7 +103,7 @@ static int process_args(pam_handle_t *pamh pam_syslog(pamh, LOG_DEBUG, "will run filter %s", *filtername); } - levp = (char **) malloc(5*sizeof(char *)); + levp = malloc(5*sizeof(char *)); if (levp == NULL) { pam_syslog(pamh, LOG_CRIT, "no memory for environment of filter"); return -1; @@ -152,7 +152,7 @@ static int process_args(pam_handle_t *pamh } size = SERVICE_OFFSET+strlen(tmp); - levp[1] = (char *) malloc(size+1); + levp[1] = malloc(size+1); if (levp[1] == NULL) { pam_syslog(pamh, LOG_CRIT, "no memory for service name"); if (levp) { @@ -176,7 +176,7 @@ static int process_args(pam_handle_t *pamh } size = USER_OFFSET+strlen(user); - levp[2] = (char *) malloc(size+1); + levp[2] = malloc(size+1); if (levp[2] == NULL) { pam_syslog(pamh, LOG_CRIT, "no memory for user's name"); if (levp) { @@ -198,7 +198,7 @@ static int process_args(pam_handle_t *pamh size = TYPE_OFFSET+strlen(type); - levp[3] = (char *) malloc(size+1); + levp[3] = malloc(size+1); if (levp[3] == NULL) { pam_syslog(pamh, LOG_CRIT, "no memory for type"); if (levp) { diff --git a/modules/pam_group/pam_group.c b/modules/pam_group/pam_group.c index 2525e048..038306c7 100644 --- a/modules/pam_group/pam_group.c +++ b/modules/pam_group/pam_group.c @@ -87,7 +87,7 @@ read_field(const pam_handle_t *pamh, int fd, char **buf, int *from, int *state, /* is buf set ? */ if (! *buf) { - *buf = (char *) calloc(1, PAM_GROUP_BUFLEN+1); + *buf = calloc(1, PAM_GROUP_BUFLEN+1); if (! *buf) { pam_syslog(pamh, LOG_CRIT, "out of memory"); D(("no memory")); @@ -530,8 +530,7 @@ static int mkgrplist(pam_handle_t *pamh, char *buf, gid_t **list, int len) gid_t *tmp; D(("allocating new block")); - tmp = (gid_t *) realloc((*list) - , sizeof(gid_t) * (blks += GROUP_BLK)); + tmp = realloc((*list), sizeof(gid_t) * (blks += GROUP_BLK)); if (tmp != NULL) { (*list) = tmp; } else { diff --git a/modules/pam_listfile/pam_listfile.c b/modules/pam_listfile/pam_listfile.c index 937576fd..5561b9ea 100644 --- a/modules/pam_listfile/pam_listfile.c +++ b/modules/pam_listfile/pam_listfile.c @@ -106,7 +106,7 @@ pam_sm_authenticate (pam_handle_t *pamh, int flags UNUSED, } else if(!strcmp(mybuf,"file")) { if (ifname) free (ifname); - ifname = (char *)malloc(strlen(myval)+1); + ifname = malloc(strlen(myval)+1); if (!ifname) return PAM_BUF_ERR; strcpy(ifname,myval); diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 48690b8d..4d24d356 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -645,7 +645,7 @@ static int process_line(char *line, const char *home, const char *rhome, sstr = strchr(ustr, ','); poly->num_uids = count; - poly->uid = (uid_t *) malloc(count * sizeof (uid_t)); + poly->uid = malloc(count * sizeof (uid_t)); uidptr = poly->uid; if (uidptr == NULL) { goto erralloc; @@ -1295,7 +1295,7 @@ static int check_inst_parent(char *ipath, struct instance_data *idata) * admin explicitly instructs to ignore the instance parent * mode by the "ignore_instance_parent_mode" argument). */ - inst_parent = (char *) malloc(strlen(ipath)+1); + inst_parent = malloc(strlen(ipath)+1); if (!inst_parent) { pam_syslog(idata->pamh, LOG_CRIT, "Error allocating pathname string"); return PAM_SESSION_ERR; diff --git a/modules/pam_time/pam_time.c b/modules/pam_time/pam_time.c index 27913b9a..a45d8f49 100644 --- a/modules/pam_time/pam_time.c +++ b/modules/pam_time/pam_time.c @@ -139,7 +139,7 @@ read_field(const pam_handle_t *pamh, int fd, char **buf, int *from, int *state, /* is buf set ? */ if (! *buf) { - *buf = (char *) calloc(1, PAM_TIME_BUFLEN+1); + *buf = calloc(1, PAM_TIME_BUFLEN+1); if (! *buf) { pam_syslog(pamh, LOG_CRIT, "out of memory"); D(("no memory")); diff --git a/modules/pam_timestamp/hmac_openssl_wrapper.c b/modules/pam_timestamp/hmac_openssl_wrapper.c index 2549c1db..52f06158 100644 --- a/modules/pam_timestamp/hmac_openssl_wrapper.c +++ b/modules/pam_timestamp/hmac_openssl_wrapper.c @@ -268,7 +268,7 @@ hmac_management(pam_handle_t *pamh, int debug, void **out, size_t *out_length, goto done; } - hmac_message = (unsigned char*)malloc(sizeof(unsigned char) * MAX_HMAC_LENGTH); + hmac_message = malloc(sizeof(unsigned char) * MAX_HMAC_LENGTH); if (!hmac_message) { pam_syslog(pamh, LOG_CRIT, "Not enough memory"); goto done; diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index a84ced60..31c5ecb6 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -687,7 +687,7 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name retval = get_pwd_hash(pamh, name, &pwd, &salt); - data_name = (char *) malloc(sizeof(FAIL_PREFIX) + strlen(name)); + data_name = malloc(sizeof(FAIL_PREFIX) + strlen(name)); if (data_name == NULL) { pam_syslog(pamh, LOG_CRIT, "no memory for data-name"); } else { -- cgit v1.2.3 From 8d082da1bc993b5b061ae81a9743891328e04ce6 Mon Sep 17 00:00:00 2001 From: Nathan Du Date: Mon, 27 Nov 2023 22:42:46 +0800 Subject: pam_unix: read yescrypt rounds from login.defs Retrieves YESCRYPT_COST_FACTOR from /etc/login.defs for yescrypt in a similar fashion to reading number of rounds for SHA-2. Resolves #607. Signed-off-by: Nathan Du --- modules/pam_unix/support.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 31c5ecb6..cfc3003c 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -99,8 +99,13 @@ unsigned long long _set_ctrl(pam_handle_t *pamh, int flags, int *remember, free (val); /* read number of rounds for crypt algo */ - if (rounds && (on(UNIX_SHA256_PASS, ctrl) || on(UNIX_SHA512_PASS, ctrl))) { - val = pam_modutil_search_key(pamh, LOGIN_DEFS, "SHA_CRYPT_MAX_ROUNDS"); + if (rounds) { + val = NULL; + if (on(UNIX_SHA256_PASS, ctrl) || on(UNIX_SHA512_PASS, ctrl)) { + val = pam_modutil_search_key(pamh, LOGIN_DEFS, "SHA_CRYPT_MAX_ROUNDS"); + } else if (on(UNIX_YESCRYPT_PASS, ctrl)) { + val = pam_modutil_search_key(pamh, LOGIN_DEFS, "YESCRYPT_COST_FACTOR"); + } if (val) { *rounds = strtol(val, NULL, 10); -- cgit v1.2.3 From d611afcbd52bc13c2455375d5c4fb20839f09f4a Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Thu, 7 Dec 2023 18:30:12 +0100 Subject: pam_unix: handle invalid names in _unix_getpwnam It is possible to trigger an out of boundary read with very long usernames (strlen's result is stored in an int) or, with even longer usernames, match other users with same prefix. This would mean that roott[and lots of t's following] could match root user. Also do not allow ':' in names when iterating through the passwd file this way. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/support.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index cfc3003c..287ec5d9 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -321,11 +321,12 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, char buf[16384]; int matched = 0, buflen; char *slogin, *spasswd, *suid, *sgid, *sgecos, *shome, *sshell, *p; + size_t userlen; memset(buf, 0, sizeof(buf)); - if (!matched && files) { - int userlen = strlen(name); + userlen = strlen(name); + if (!matched && files && userlen < sizeof(buf) && strchr(name, ':') == NULL) { passwd = fopen("/etc/passwd", "r"); if (passwd != NULL) { while (fgets(buf, sizeof(buf), passwd) != NULL) { -- cgit v1.2.3 From bf9ebc84c091f9f2d018aac2f9c2c4c4933e319e Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Mon, 11 Dec 2023 14:36:49 +0100 Subject: pam_unix: use correct number of rounds It is possible to have a mismatch between ENCRYPT_METHOD in login.defs and an argument given specifically to pam_unix.so. If pam_unix.so receives the argument "yescrypt" but ENCRYPT_METHOD is set to SHA512, then SHA_CRYPT_MAX_ROUNDS is parsed from login.defs and used as rounds for yescrypt -- except if rounds are specificially given as an argument to pam_unix.so as well. Read the correct rounds from login.defs after all arguments are parsed and no rounds were specified to figure out which one will eventually be used. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/support.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 287ec5d9..ec9a5725 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -97,22 +97,6 @@ unsigned long long _set_ctrl(pam_handle_t *pamh, int flags, int *remember, ctrl |= unix_args[j].flag; /* for turning things on */ } free (val); - - /* read number of rounds for crypt algo */ - if (rounds) { - val = NULL; - if (on(UNIX_SHA256_PASS, ctrl) || on(UNIX_SHA512_PASS, ctrl)) { - val = pam_modutil_search_key(pamh, LOGIN_DEFS, "SHA_CRYPT_MAX_ROUNDS"); - } else if (on(UNIX_YESCRYPT_PASS, ctrl)) { - val = pam_modutil_search_key(pamh, LOGIN_DEFS, "YESCRYPT_COST_FACTOR"); - } - - if (val) { - *rounds = strtol(val, NULL, 10); - set(UNIX_ALGO_ROUNDS, ctrl); - free (val); - } - } } /* now parse the arguments to this module */ @@ -180,6 +164,21 @@ unsigned long long _set_ctrl(pam_handle_t *pamh, int flags, int *remember, set(UNIX__NONULL, ctrl); } + /* Read number of rounds for sha256, sha512 and yescrypt */ + if (off(UNIX_ALGO_ROUNDS, ctrl) && rounds != NULL) { + val = NULL; + if (on(UNIX_YESCRYPT_PASS, ctrl)) { + val = pam_modutil_search_key(pamh, LOGIN_DEFS, "YESCRYPT_COST_FACTOR"); + } else if (on(UNIX_SHA256_PASS, ctrl) || on(UNIX_SHA512_PASS, ctrl)) { + val = pam_modutil_search_key(pamh, LOGIN_DEFS, "SHA_CRYPT_MAX_ROUNDS"); + } + if (val) { + *rounds = strtol(val, NULL, 10); + set(UNIX_ALGO_ROUNDS, ctrl); + free (val); + } + } + /* Set default rounds for blowfish, gost-yescrypt and yescrypt */ if (off(UNIX_ALGO_ROUNDS, ctrl) && rounds != NULL) { if (on(UNIX_BLOWFISH_PASS, ctrl) || -- cgit v1.2.3 From b8429cc8036cd23d075174d13eedc6d857e2b454 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sun, 10 Dec 2023 14:20:32 +0000 Subject: pam_unix: check str to integer conversions Print an error in syslog if an integer could not be converted. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/passverify.c | 10 ++++++-- modules/pam_unix/support.c | 60 ++++++++++++++++++++++++++++++++----------- 2 files changed, 53 insertions(+), 17 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 930c7d3c..98f997d5 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -5,6 +5,7 @@ #include #include #include "support.h" +#include #include #include #include @@ -703,7 +704,8 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, while (fgets(buf, 16380, opwfile)) { if (!strncmp(buf, forwho, len) && strchr(":,\n", buf[len]) != NULL) { - char *sptr = NULL; + char *ep, *sptr = NULL; + long value; found = 1; if (howmany == 0) continue; @@ -724,7 +726,11 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, continue; } s_pas = strtok_r(NULL, ":", &sptr); - npas = strtol(s_npas, NULL, 10) + 1; + value = strtol(s_npas, &ep, 10); + if (value < 0 || value >= INT_MAX || s_npas == ep || *ep != '\0') + npas = 0; + else + npas = (int)value + 1; while (npas > howmany && s_pas != NULL) { s_pas = strpbrk(s_pas, ","); if (s_pas != NULL) diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index ec9a5725..9cc39ad7 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -46,6 +46,18 @@ int _make_remark(pam_handle_t * pamh, unsigned long long ctrl, return retval; } +static int _unix_strtoi(const char *str, int minval, int *result) +{ + char *ep; + long value = strtol(str, &ep, 10); + if (value < minval || value > INT_MAX || str == ep || *ep != '\0') { + *result = minval; + return -1; + } + *result = (int)value; + return 0; +} + /* * set the control flags for the UNIX module. */ @@ -126,9 +138,11 @@ unsigned long long _set_ctrl(pam_handle_t *pamh, int flags, int *remember, "option remember not allowed for this module type"); continue; } - *remember = strtol(str, NULL, 10); - if ((*remember == INT_MIN) || (*remember == INT_MAX)) - *remember = -1; + if (_unix_strtoi(str, -1, remember)) { + pam_syslog(pamh, LOG_ERR, + "option remember invalid [%s]", str); + continue; + } if (*remember > 400) *remember = 400; } else if (j == UNIX_MIN_PASS_LEN) { @@ -137,14 +151,22 @@ unsigned long long _set_ctrl(pam_handle_t *pamh, int flags, int *remember, "option minlen not allowed for this module type"); continue; } - *pass_min_len = atoi(str); + if (_unix_strtoi(str, 0, pass_min_len)) { + pam_syslog(pamh, LOG_ERR, + "option minlen invalid [%s]", str); + continue; + } } else if (j == UNIX_ALGO_ROUNDS) { if (rounds == NULL) { pam_syslog(pamh, LOG_ERR, "option rounds not allowed for this module type"); continue; } - *rounds = strtol(str, NULL, 10); + if (_unix_strtoi(str, 0, rounds)) { + pam_syslog(pamh, LOG_ERR, + "option rounds invalid [%s]", str); + continue; + } } ctrl &= unix_args[j].mask; /* for turning things off */ @@ -166,16 +188,24 @@ unsigned long long _set_ctrl(pam_handle_t *pamh, int flags, int *remember, /* Read number of rounds for sha256, sha512 and yescrypt */ if (off(UNIX_ALGO_ROUNDS, ctrl) && rounds != NULL) { - val = NULL; - if (on(UNIX_YESCRYPT_PASS, ctrl)) { - val = pam_modutil_search_key(pamh, LOGIN_DEFS, "YESCRYPT_COST_FACTOR"); - } else if (on(UNIX_SHA256_PASS, ctrl) || on(UNIX_SHA512_PASS, ctrl)) { - val = pam_modutil_search_key(pamh, LOGIN_DEFS, "SHA_CRYPT_MAX_ROUNDS"); - } - if (val) { - *rounds = strtol(val, NULL, 10); - set(UNIX_ALGO_ROUNDS, ctrl); - free (val); + const char *key = NULL; + if (on(UNIX_YESCRYPT_PASS, ctrl)) + key = "YESCRYPT_COST_FACTOR"; + else if (on(UNIX_SHA256_PASS, ctrl) || on(UNIX_SHA512_PASS, ctrl)) + key = "SHA_CRYPT_MAX_ROUNDS"; + else + key = NULL; + + if (key != NULL) { + val = pam_modutil_search_key(pamh, LOGIN_DEFS, key); + if (val) { + if (_unix_strtoi(val, 0, rounds)) + pam_syslog(pamh, LOG_ERR, + "option %s invalid [%s]", key, val); + else + set(UNIX_ALGO_ROUNDS, ctrl); + free (val); + } } } -- cgit v1.2.3 From 9f733e5f3b8ae092e405d8bffa523a22155a7f6a Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Thu, 14 Dec 2023 19:50:12 +0100 Subject: treewide: store strlen results in size_t Very long strings could overflow the int data type. Make sure to use the correct data type. Signed-off-by: Tobias Stoeckmann --- libpam/pam_misc.c | 3 ++- libpamc/pamc_load.c | 2 +- modules/pam_access/pam_access.c | 12 ++++++------ modules/pam_debug/pam_debug.c | 2 +- modules/pam_namespace/pam_namespace.c | 8 ++++---- modules/pam_unix/support.c | 2 +- 6 files changed, 15 insertions(+), 14 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/libpam/pam_misc.c b/libpam/pam_misc.c index f0b35c28..f4e81a55 100644 --- a/libpam/pam_misc.c +++ b/libpam/pam_misc.c @@ -264,7 +264,8 @@ void _pam_parse_control(int *control_array, char *tok) int ret; while (*tok) { - int act, len; + size_t len; + int act; /* skip leading space */ while (isspace((unsigned char)*tok) && *++tok); diff --git a/libpamc/pamc_load.c b/libpamc/pamc_load.c index f7365990..c1a39f5b 100644 --- a/libpamc/pamc_load.c +++ b/libpamc/pamc_load.c @@ -245,7 +245,7 @@ int pamc_disable(pamc_handle_t pch, const char *agent_id) int pamc_load(pamc_handle_t pch, const char *agent_id) { pamc_agent_t *agent; - int length; + size_t length; /* santity checking */ diff --git a/modules/pam_access/pam_access.c b/modules/pam_access/pam_access.c index 04d7306b..a8efdf30 100644 --- a/modules/pam_access/pam_access.c +++ b/modules/pam_access/pam_access.c @@ -427,8 +427,8 @@ login_access (pam_handle_t *pamh, struct login_info *item) #ifdef HAVE_LIBAUDIT int nonall_match = NO; #endif - int end; - int lineno = 0; /* for diagnostics */ + size_t end; + size_t lineno = 0; /* for diagnostics */ char *sptr; if (item->debug) @@ -450,7 +450,7 @@ login_access (pam_handle_t *pamh, struct login_info *item) lineno++; if (line[end = strlen(line) - 1] != '\n') { pam_syslog(pamh, LOG_ERR, - "%s: line %d: missing newline or line too long", + "%s: line %zu: missing newline or line too long", item->config_file, lineno); continue; } @@ -466,18 +466,18 @@ login_access (pam_handle_t *pamh, struct login_info *item) if (!(perm = strtok_r(line, item->fs, &sptr)) || !(users = strtok_r(NULL, item->fs, &sptr)) || !(froms = strtok_r(NULL, "\n", &sptr))) { - pam_syslog(pamh, LOG_ERR, "%s: line %d: bad field count", + pam_syslog(pamh, LOG_ERR, "%s: line %zu: bad field count", item->config_file, lineno); continue; } if (perm[0] != '+' && perm[0] != '-') { - pam_syslog(pamh, LOG_ERR, "%s: line %d: bad first field", + pam_syslog(pamh, LOG_ERR, "%s: line %zu: bad first field", item->config_file, lineno); continue; } if (item->debug) pam_syslog (pamh, LOG_DEBUG, - "line %d: %s : %s : %s", lineno, perm, users, froms); + "line %zu: %s : %s : %s", lineno, perm, users, froms); match = list_match(pamh, users, NULL, item, user_match); if (item->debug) pam_syslog (pamh, LOG_DEBUG, "user_match=%d, \"%s\"", diff --git a/modules/pam_debug/pam_debug.c b/modules/pam_debug/pam_debug.c index 414806b2..37bf8b95 100644 --- a/modules/pam_debug/pam_debug.c +++ b/modules/pam_debug/pam_debug.c @@ -40,7 +40,7 @@ static int parse_args(int retval, const char *event, int i; for (i=0; iflags |= flag_values[i]; @@ -480,7 +480,7 @@ static int process_line(char *line, const char *home, const char *rhome, static const char *var_names[] = {"HOME", "USER", NULL}; const char *var_values[] = {home, idata->user}; const char *rvar_values[] = {rhome, idata->ruser}; - int len; + size_t len; /* * skip the leading white space diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 9cc39ad7..eb2fff50 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -582,7 +582,7 @@ static int _unix_run_helper_binary(pam_handle_t *pamh, const char *passwd, /* if the stored password is NULL */ int rc=0; if (passwd != NULL) { /* send the password to the child */ - int len = strlen(passwd); + size_t len = strlen(passwd); if (len > PAM_MAX_RESP_SIZE) len = PAM_MAX_RESP_SIZE; -- cgit v1.2.3 From 0fb2978d0e139dc57878d5c82d6eae79273e2031 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Wed, 6 Dec 2023 22:48:34 +0100 Subject: treewide: fix typos in comments and documentation Signed-off-by: Tobias Stoeckmann --- doc/man/pam.3.xml | 4 ++-- doc/man/pam_open_session.3.xml | 2 +- libpam/pam_delay.c | 2 +- libpam/pam_dispatch.c | 2 +- libpam/pam_password.c | 2 +- libpam/pam_private.h | 2 +- libpam_misc/misc_conv.c | 4 ++-- modules/pam_env/pam_env.8.xml | 2 +- modules/pam_limits/pam_limits.c | 2 +- modules/pam_pwhistory/opasswd.c | 2 +- modules/pam_unix/md5_crypt.c | 2 +- modules/pam_unix/pam_unix_auth.c | 2 +- modules/pam_unix/pam_unix_passwd.c | 2 +- modules/pam_unix/support.c | 2 +- 14 files changed, 16 insertions(+), 16 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/doc/man/pam.3.xml b/doc/man/pam.3.xml index 4b828016..a7d13b45 100644 --- a/doc/man/pam.3.xml +++ b/doc/man/pam.3.xml @@ -150,7 +150,7 @@ pam_get_item3 - functions allows applications and PAM service modules to set and + functions allow applications and PAM service modules to set and retrieve PAM information. @@ -169,7 +169,7 @@ pam_get_data3 - functions allows PAM service modules to set and retrieve free-form + function allows PAM service modules to set and retrieve free-form data from one invocation to another. diff --git a/doc/man/pam_open_session.3.xml b/doc/man/pam_open_session.3.xml index d37b3e59..a05e0abe 100644 --- a/doc/man/pam_open_session.3.xml +++ b/doc/man/pam_open_session.3.xml @@ -40,7 +40,7 @@ It should be noted that the effective uid, geteuid2 - . of the application should be of sufficient + , of the application should be of sufficient privilege to perform such tasks as creating or mounting the user's home directory for example. diff --git a/libpam/pam_delay.c b/libpam/pam_delay.c index b6a962d0..b8fddbb6 100644 --- a/libpam/pam_delay.c +++ b/libpam/pam_delay.c @@ -19,7 +19,7 @@ /* ********************************************************************** * initialize the time as unset, this is set on the return from the - * authenticating pair of of the libpam pam_XXX calls. + * authenticating pair of the libpam pam_XXX calls. */ void _pam_reset_timer(pam_handle_t *pamh) diff --git a/libpam/pam_dispatch.c b/libpam/pam_dispatch.c index 31a28be3..ca78a10f 100644 --- a/libpam/pam_dispatch.c +++ b/libpam/pam_dispatch.c @@ -299,7 +299,7 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h, } continue; -decision_made: /* by getting here we have made a decision */ +decision_made: /* by getting here we have made a decision */ while (h->next != NULL && h->next->stack_level >= stack_level) { h = h->next; ++depth; diff --git a/libpam/pam_password.c b/libpam/pam_password.c index 5bda547b..9783dbe0 100644 --- a/libpam/pam_password.c +++ b/libpam/pam_password.c @@ -22,7 +22,7 @@ int pam_chauthtok(pam_handle_t *pamh, int flags) return PAM_SYSTEM_ERR; } - /* applications are not allowed to set this flags */ + /* applications are not allowed to set these flags */ if (flags & (PAM_PRELIM_CHECK | PAM_UPDATE_AUTHTOK)) { pam_syslog (pamh, LOG_ERR, "PAM_PRELIM_CHECK or PAM_UPDATE_AUTHTOK set by application"); diff --git a/libpam/pam_private.h b/libpam/pam_private.h index da268bdf..8069b61c 100644 --- a/libpam/pam_private.h +++ b/libpam/pam_private.h @@ -255,7 +255,7 @@ const char *_pam_dlerror (void); /* For now we just use a stack and linear search for module data. */ /* If it becomes apparent that there is a lot of data, it should */ -/* changed to either a sorted list or a hash table. */ +/* be changed to either a sorted list or a hash table. */ struct pam_data { char *name; diff --git a/libpam_misc/misc_conv.c b/libpam_misc/misc_conv.c index 0f213bce..7410e929 100644 --- a/libpam_misc/misc_conv.c +++ b/libpam_misc/misc_conv.c @@ -97,7 +97,7 @@ static int get_delay(void) expired = 0; /* reset flag */ (void) time(&now); - /* has the quit time past? */ + /* has the quit time passed? */ if (pam_misc_conv_die_time && now >= pam_misc_conv_die_time) { fprintf(stderr,"%s",pam_misc_conv_die_line); @@ -105,7 +105,7 @@ static int get_delay(void) return -1; /* time is up */ } - /* has the warning time past? */ + /* has the warning time passed? */ if (pam_misc_conv_warn_time && now >= pam_misc_conv_warn_time) { fprintf(stderr, "%s", pam_misc_conv_warn_line); pam_misc_conv_warn_time = 0; /* reset warn_time */ diff --git a/modules/pam_env/pam_env.8.xml b/modules/pam_env/pam_env.8.xml index fb172e17..3af52ea6 100644 --- a/modules/pam_env/pam_env.8.xml +++ b/modules/pam_env/pam_env.8.xml @@ -181,7 +181,7 @@ Indicate an alternative .pam_environment - file to override the default.The syntax is the same as + file to override the default. The syntax is the same as for /etc/security/pam_env.conf. The filename is relative to the user home directory. This can be useful when different services need different diff --git a/modules/pam_limits/pam_limits.c b/modules/pam_limits/pam_limits.c index fc541330..53e2e8f2 100644 --- a/modules/pam_limits/pam_limits.c +++ b/modules/pam_limits/pam_limits.c @@ -914,7 +914,7 @@ parse_config_file(pam_handle_t *pamh, const char *uname, uid_t uid, gid_t gid, for(j=0; j < strlen(value); j++) value[j]=tolower((unsigned char)value[j]); - if (strcmp(uname, domain) == 0) /* this user have a limit */ + if (strcmp(uname, domain) == 0) /* this user has a limit */ process_limit(pamh, LIMITS_DEF_USER, ltype, item, value, ctrl, pl); else if (domain[0]=='@') { if (ctrl & PAM_DEBUG_ARG) { diff --git a/modules/pam_pwhistory/opasswd.c b/modules/pam_pwhistory/opasswd.c index 5f577dfd..f1f62aaf 100644 --- a/modules/pam_pwhistory/opasswd.c +++ b/modules/pam_pwhistory/opasswd.c @@ -425,7 +425,7 @@ save_old_pass, const char *user, int howmany, const char *filename, int debug UN /* increase count. */ entry.count++; - /* check that we don't remember to many passwords. */ + /* check that we don't remember too many passwords. */ while (entry.count > howmany && entry.count > 1) { char *p = strpbrk (entry.old_passwords, ","); diff --git a/modules/pam_unix/md5_crypt.c b/modules/pam_unix/md5_crypt.c index ed5ecda4..c3e77c9d 100644 --- a/modules/pam_unix/md5_crypt.c +++ b/modules/pam_unix/md5_crypt.c @@ -38,7 +38,7 @@ char *MD5Name(crypt_md5)(const char *pw, const char *salt) { const char *magic = "$1$"; /* This string is magic for this algorithm. Having - * it this way, we can get get better later on */ + * it this way, we can get better later on */ char *passwd, *p; const char *sp, *ep; unsigned char final[16]; diff --git a/modules/pam_unix/pam_unix_auth.c b/modules/pam_unix/pam_unix_auth.c index 976699d4..ffb61547 100644 --- a/modules/pam_unix/pam_unix_auth.c +++ b/modules/pam_unix/pam_unix_auth.c @@ -118,7 +118,7 @@ pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **argv) /* * Various libraries at various times have had bugs related to * '+' or '-' as the first character of a user name. Don't - * allow this characters here. + * allow these characters here. */ if (name[0] == '-' || name[0] == '+') { pam_syslog(pamh, LOG_NOTICE, "bad username [%s]", name); diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c index c1d547c7..9dde2aee 100644 --- a/modules/pam_unix/pam_unix_passwd.c +++ b/modules/pam_unix/pam_unix_passwd.c @@ -698,7 +698,7 @@ pam_sm_chauthtok(pam_handle_t *pamh, int flags, int argc, const char **argv) } else { D(("process run by root so do nothing this time around")); pass_old = NULL; - retval = PAM_SUCCESS; /* root doesn't have too */ + retval = PAM_SUCCESS; /* root doesn't have to */ } if (retval != PAM_SUCCESS) { diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index eb2fff50..9d8cac7d 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -648,7 +648,7 @@ _unix_blankpasswd (pam_handle_t *pamh, unsigned long long ctrl, const char *name /* * This function does not have to be too smart if something goes - * wrong, return FALSE and let this case to be treated somewhere + * wrong, return FALSE and let this case be treated somewhere * else (CG) */ -- cgit v1.2.3 From 2e375aad04d047e12468f93300ad7e42a8a03ff3 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Fri, 15 Dec 2023 10:14:11 +0100 Subject: treewide: use asprintf to construct strings The asprintf function is considered as given for current code already. Use it instead of calling malloc + strcpy + strcat manually. Reported-by: Benny Baumann Signed-off-by: Tobias Stoeckmann --- conf/pam_conv1/pam_conv_y.y | 15 ++++---- modules/pam_faillock/faillock.c | 16 ++++----- modules/pam_filter/pam_filter.c | 64 ++++++++--------------------------- modules/pam_issue/pam_issue.c | 15 +++----- modules/pam_namespace/pam_namespace.c | 26 ++++++-------- modules/pam_unix/md5_crypt.c | 20 +++++------ modules/pam_unix/support.c | 7 ++-- modules/pam_xauth/pam_xauth.c | 45 +++++++----------------- 8 files changed, 66 insertions(+), 142 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/conf/pam_conv1/pam_conv_y.y b/conf/pam_conv1/pam_conv_y.y index 72a5dab3..e2cf85c3 100644 --- a/conf/pam_conv1/pam_conv_y.y +++ b/conf/pam_conv1/pam_conv_y.y @@ -78,8 +78,10 @@ line /* $1 = service-name */ yyerror("Appending to " PAM_D "/%s", $1); - filename = malloc(strlen($1) + sizeof(PAM_D) + 6); - sprintf(filename, PAM_D_FILE_FMT, $1); + if (asprintf(&filename, PAM_D_FILE_FMT, $1) < 0) { + yyerror("unable to create filename - aborting"); + exit(1); + } conf = fopen(filename, "r"); if (conf == NULL) { /* new file */ @@ -140,12 +142,11 @@ tokenls $$=NULL; } | tokenls tok { - int len; - if ($1) { - len = strlen($1) + strlen($2) + 2; - $$ = malloc(len); - sprintf($$,"%s %s",$1,$2); + if (asprintf(&$$, "%s %s", $1, $2) < 0) { + yyerror("failed to assemble tokenls"); + exit(1); + } free($1); free($2); } else { diff --git a/modules/pam_faillock/faillock.c b/modules/pam_faillock/faillock.c index 091f253a..1d2057e7 100644 --- a/modules/pam_faillock/faillock.c +++ b/modules/pam_faillock/faillock.c @@ -36,6 +36,7 @@ #include "config.h" #include +#include #include #include #include @@ -55,23 +56,20 @@ open_tally (const char *dir, const char *user, uid_t uid, int create) { char *path; int flags = O_RDWR; - int fd; + int fd, r; if (dir == NULL || strstr(user, "../") != NULL) /* just a defensive programming as the user must be a * valid user on the system anyway */ return -1; - path = malloc(strlen(dir) + strlen(user) + 2); - if (path == NULL) + if (*dir && dir[strlen(dir) - 1] != '/') + r = asprintf(&path, "%s/%s", dir, user); + else + r = asprintf(&path, "%s%s", dir, user); + if (r < 0) return -1; - strcpy(path, dir); - if (*dir && dir[strlen(dir) - 1] != '/') { - strcat(path, "/"); - } - strcat(path, user); - if (create) { flags |= O_CREAT; if (access(dir, F_OK) != 0) { diff --git a/modules/pam_filter/pam_filter.c b/modules/pam_filter/pam_filter.c index 72c3b480..4e614498 100644 --- a/modules/pam_filter/pam_filter.c +++ b/modules/pam_filter/pam_filter.c @@ -138,82 +138,46 @@ static int process_args(pam_handle_t *pamh /* the "SERVICE" variable */ -#define SERVICE_NAME "SERVICE=" -#define SERVICE_OFFSET (sizeof(SERVICE_NAME) - 1) - retval = pam_get_item(pamh, PAM_SERVICE, &tmp); if (retval != PAM_SUCCESS || tmp == NULL) { pam_syslog(pamh, LOG_CRIT, "service name not found"); - if (levp) { - free(levp[0]); - free(levp); - } + free(levp[0]); + free(levp); return -1; } - size = SERVICE_OFFSET+strlen(tmp); - levp[1] = malloc(size+1); - if (levp[1] == NULL) { + if (asprintf(&levp[1], "SERVICE=%s", (const char *) tmp) < 0) { pam_syslog(pamh, LOG_CRIT, "no memory for service name"); - if (levp) { - free(levp[0]); - free(levp); - } + free(levp[0]); + free(levp); return -1; } - strcpy(levp[1], SERVICE_NAME); - strcpy(levp[1]+SERVICE_OFFSET, tmp); - levp[1][size] = '\0'; /* terminate */ - /* the "USER" variable */ -#define USER_NAME "USER=" -#define USER_OFFSET (sizeof(USER_NAME) - 1) - if (pam_get_user(pamh, &user, NULL) != PAM_SUCCESS) { user = ""; } - size = USER_OFFSET+strlen(user); - levp[2] = malloc(size+1); - if (levp[2] == NULL) { + if (asprintf(&levp[2], "USER=%s", user) < 0) { pam_syslog(pamh, LOG_CRIT, "no memory for user's name"); - if (levp) { - free(levp[1]); - free(levp[0]); - free(levp); - } + free(levp[1]); + free(levp[0]); + free(levp); return -1; } - strcpy(levp[2], USER_NAME); - strcpy(levp[2]+USER_OFFSET, user); - levp[2][size] = '\0'; /* terminate */ - /* the "USER" variable */ -#define TYPE_NAME "TYPE=" -#define TYPE_OFFSET (sizeof(TYPE_NAME) - 1) - - size = TYPE_OFFSET+strlen(type); - - levp[3] = malloc(size+1); - if (levp[3] == NULL) { + if (asprintf(&levp[3], "TYPE=%s", type) < 0) { pam_syslog(pamh, LOG_CRIT, "no memory for type"); - if (levp) { - free(levp[2]); - free(levp[1]); - free(levp[0]); - free(levp); - } + free(levp[2]); + free(levp[1]); + free(levp[0]); + free(levp); return -1; } - strcpy(levp[3], TYPE_NAME); - strcpy(levp[3]+TYPE_OFFSET, type); - levp[3][size] = '\0'; /* terminate */ - levp[4] = NULL; /* end list */ *evp = levp; diff --git a/modules/pam_issue/pam_issue.c b/modules/pam_issue/pam_issue.c index c08f90c3..aade642e 100644 --- a/modules/pam_issue/pam_issue.c +++ b/modules/pam_issue/pam_issue.c @@ -240,7 +240,6 @@ pam_sm_authenticate(pam_handle_t *pamh, int flags UNUSED, const char *issue_file = NULL; int parse_esc = 1; const void *item = NULL; - const char *cur_prompt; char *issue_prompt = NULL; /* If we've already set the prompt, don't set it again */ @@ -277,10 +276,6 @@ pam_sm_authenticate(pam_handle_t *pamh, int flags UNUSED, return retval; } - cur_prompt = item; - if (cur_prompt == NULL) - cur_prompt = ""; - if (parse_esc) retval = read_issue_quoted(pamh, fp, &issue_prompt); else @@ -291,11 +286,10 @@ pam_sm_authenticate(pam_handle_t *pamh, int flags UNUSED, if (retval != PAM_SUCCESS) goto out; - { - size_t size = strlen(issue_prompt) + strlen(cur_prompt) + 1; - char *new_prompt = realloc(issue_prompt, size); - - if (new_prompt == NULL) { + if (item != NULL) { + const char *cur_prompt = item; + char *new_prompt; + if (asprintf(&new_prompt, "%s%s", issue_prompt, cur_prompt) < 0) { pam_syslog(pamh, LOG_CRIT, "out of memory"); retval = PAM_BUF_ERR; goto out; @@ -303,7 +297,6 @@ pam_sm_authenticate(pam_handle_t *pamh, int flags UNUSED, issue_prompt = new_prompt; } - strcat(issue_prompt, cur_prompt); retval = pam_set_item(pamh, PAM_USER_PROMPT, (const void *) issue_prompt); out: diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index d78111b3..1c42b0f4 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -592,26 +592,20 @@ static int process_line(char *line, const char *home, const char *rhome, * Populate polyinstantiated directory structure with appropriate * pathnames and the method with which to polyinstantiate. */ - if (strlen(dir) >= sizeof(poly->dir) - || strlen(rdir) >= sizeof(poly->rdir) - || strlen(instance_prefix) >= sizeof(poly->instance_prefix)) { - pam_syslog(idata->pamh, LOG_NOTICE, "Pathnames too long"); - goto skipping; - } - strcpy(poly->dir, dir); - strcpy(poly->rdir, rdir); - strcpy(poly->instance_prefix, instance_prefix); - if (parse_method(method, poly, idata) != 0) { goto skipping; } - if (poly->method == TMPDIR) { - if (sizeof(poly->instance_prefix) - strlen(poly->instance_prefix) < 7) { - pam_syslog(idata->pamh, LOG_NOTICE, "Pathnames too long"); - goto skipping; - } - strcat(poly->instance_prefix, "XXXXXX"); +#define COPY_STR(dst, src, apd) \ + (snprintf((dst), sizeof(dst), "%s%s", (src), (apd)) != \ + (ssize_t) (strlen(src) + strlen(apd))) + + if (COPY_STR(poly->dir, dir, "") + || COPY_STR(poly->rdir, rdir, "") + || COPY_STR(poly->instance_prefix, instance_prefix, + poly->method == TMPDIR ? "XXXXXX" : "")) { + pam_syslog(idata->pamh, LOG_NOTICE, "Pathnames too long"); + goto skipping; } /* diff --git a/modules/pam_unix/md5_crypt.c b/modules/pam_unix/md5_crypt.c index c3e77c9d..a5720999 100644 --- a/modules/pam_unix/md5_crypt.c +++ b/modules/pam_unix/md5_crypt.c @@ -13,6 +13,7 @@ */ #include +#include #include #include "md5.h" #include "pam_inline.h" @@ -41,6 +42,7 @@ char *MD5Name(crypt_md5)(const char *pw, const char *salt) * it this way, we can get better later on */ char *passwd, *p; const char *sp, *ep; + char buf[23]; unsigned char final[16]; int sl, pl, i, j; MD5_CTX ctx, ctx1; @@ -49,12 +51,6 @@ char *MD5Name(crypt_md5)(const char *pw, const char *salt) /* Refine the Salt first */ sp = salt; - /* TODO: now that we're using malloc'ed memory, get rid of the - strange constant buffer size. */ - passwd = malloc(120); - if (passwd == NULL) - return NULL; - /* If it starts with the magic string, then skip that */ if ((ep = pam_str_skip_prefix_len(sp, magic, strlen(magic))) != NULL) sp = ep; @@ -96,11 +92,6 @@ char *MD5Name(crypt_md5)(const char *pw, const char *salt) else MD5Name(MD5Update)(&ctx, (unsigned const char *)pw+j, 1); - /* Now make the output string */ - strcpy(passwd, magic); - strncat(passwd, sp, sl); - strcat(passwd, "$"); - MD5Name(MD5Final)(final,&ctx); /* @@ -128,7 +119,7 @@ char *MD5Name(crypt_md5)(const char *pw, const char *salt) MD5Name(MD5Final)(final,&ctx1); } - p = passwd + strlen(passwd); + p = buf; l = (final[0] << 16) | (final[6] << 8) | final[12]; to64(p, l, 4); @@ -150,7 +141,12 @@ char *MD5Name(crypt_md5)(const char *pw, const char *salt) p += 2; *p = '\0'; + /* Now make the output string */ + if (asprintf(&passwd, "%s%.*s$%s", magic, sl, sp, buf) < 0) + passwd = NULL; + /* Don't leave anything around in vm they could use. */ + pam_overwrite_array(buf); pam_overwrite_array(final); return passwd; diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 9d8cac7d..49807873 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -722,12 +722,9 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name retval = get_pwd_hash(pamh, name, &pwd, &salt); - data_name = malloc(sizeof(FAIL_PREFIX) + strlen(name)); - if (data_name == NULL) { + if (asprintf(&data_name, "%s%s", FAIL_PREFIX, name) < 0) { pam_syslog(pamh, LOG_CRIT, "no memory for data-name"); - } else { - strcpy(data_name, FAIL_PREFIX); - strcpy(data_name + sizeof(FAIL_PREFIX) - 1, name); + data_name = NULL; } if (p != NULL && strlen(p) > PAM_MAX_RESP_SIZE) { diff --git a/modules/pam_xauth/pam_xauth.c b/modules/pam_xauth/pam_xauth.c index ed86130e..20ae3e6b 100644 --- a/modules/pam_xauth/pam_xauth.c +++ b/modules/pam_xauth/pam_xauth.c @@ -505,16 +505,9 @@ pam_sm_open_session (pam_handle_t *pamh, int flags UNUSED, retval = PAM_SESSION_ERR; goto cleanup; } - } else { - cookiefile = malloc(strlen(rpwd->pw_dir) + 1 + - strlen(XAUTHDEF) + 1); - if (cookiefile == NULL) { - retval = PAM_SESSION_ERR; - goto cleanup; - } - strcpy(cookiefile, rpwd->pw_dir); - strcat(cookiefile, "/"); - strcat(cookiefile, XAUTHDEF); + } else if (asprintf(&cookiefile, "%s/%s", rpwd->pw_dir, XAUTHDEF) < 0) { + retval = PAM_SESSION_ERR; + goto cleanup; } if (debug) { pam_syslog(pamh, LOG_DEBUG, "reading keys from `%s'", @@ -544,29 +537,18 @@ pam_sm_open_session (pam_handle_t *pamh, int flags UNUSED, if (((cookie == NULL) || (strlen(cookie) == 0)) && (pam_str_skip_prefix(display, "localhost:") != NULL || pam_str_skip_prefix(display, "localhost/unix:") != NULL)) { - char *t, *screen; - size_t tlen, slen; + char hostname[HOST_NAME_MAX + 1]; /* Free the useless cookie string. */ free(cookie); cookie = NULL; - /* Allocate enough space to hold an adjusted name. */ - tlen = strlen(display) + LINE_MAX + 1; - t = calloc(1, tlen); - if (t != NULL) { - if (gethostname(t, tlen - 1) != -1) { - /* Append the protocol and then the - * screen number. */ - if (strlen(t) < tlen - 6) { - strcat(t, "/unix:"); - } - screen = strchr(display, ':'); - if (screen != NULL) { - screen++; - slen = strlen(screen); - if (strlen(t) + slen < tlen) { - strcat(t, screen); - } - } + if (gethostname(hostname, sizeof(hostname)) != -1) { + const char *screen; + char *t; + + /* Append protocol and screen number to host. */ + screen = display + strcspn(display, ":"); + if (asprintf(&t, "%s/unix%s", + hostname, screen) >= 0) { if (debug) { pam_syslog(pamh, LOG_DEBUG, "no key for `%s', " @@ -592,9 +574,8 @@ pam_sm_open_session (pam_handle_t *pamh, int flags UNUSED, xauth, (const char *[]) { xauth, "-f", cookiefile, "nlist", t, NULL}); + free(t); } - free(t); - t = NULL; } } -- cgit v1.2.3 From 2e677446e35898f1539e6522a826d392c6abde50 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 2 Jan 2024 21:13:07 +0100 Subject: pam_unix: use size_t instead of int for sizes Also rename buflen to retlen, since it is not associated with the variable buf, but ret. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/support.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 49807873..bdba3ce5 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -348,9 +348,9 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, { FILE *passwd; char buf[16384]; - int matched = 0, buflen; + int matched = 0; char *slogin, *spasswd, *suid, *sgid, *sgecos, *shome, *sshell, *p; - size_t userlen; + size_t retlen, userlen; memset(buf, 0, sizeof(buf)); @@ -438,17 +438,17 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, } *sshell++ = '\0'; - buflen = sizeof(struct passwd) + + retlen = sizeof(struct passwd) + strlen(slogin) + 1 + strlen(spasswd) + 1 + strlen(sgecos) + 1 + strlen(shome) + 1 + strlen(sshell) + 1; - *ret = malloc(buflen); + *ret = malloc(retlen); if (*ret == NULL) { return matched; } - memset(*ret, '\0', buflen); + memset(*ret, '\0', retlen); (*ret)->pw_uid = strtol(suid, &p, 10); if ((strlen(suid) == 0) || (*p != '\0')) { -- cgit v1.2.3 From 5f51dcfa6b804025ed002fe06a0534316d471c0c Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 2 Jan 2024 21:14:23 +0100 Subject: pam_unix: use calloc instead of malloc/memset Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/support.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index bdba3ce5..aa7000b3 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -444,11 +444,10 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, strlen(sgecos) + 1 + strlen(shome) + 1 + strlen(sshell) + 1; - *ret = malloc(retlen); + *ret = calloc(retlen, sizeof(char)); if (*ret == NULL) { return matched; } - memset(*ret, '\0', retlen); (*ret)->pw_uid = strtol(suid, &p, 10); if ((strlen(suid) == 0) || (*p != '\0')) { -- cgit v1.2.3 From 88759de2aa49171703afd70e3b78ccf17ab7ef95 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 2 Jan 2024 21:19:53 +0100 Subject: pam_unix: unify error handling Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/support.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index aa7000b3..336742a1 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -404,37 +404,37 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, spasswd = strchr(slogin, ':'); if (spasswd == NULL) { - return matched; + goto fail; } *spasswd++ = '\0'; suid = strchr(spasswd, ':'); if (suid == NULL) { - return matched; + goto fail; } *suid++ = '\0'; sgid = strchr(suid, ':'); if (sgid == NULL) { - return matched; + goto fail; } *sgid++ = '\0'; sgecos = strchr(sgid, ':'); if (sgecos == NULL) { - return matched; + goto fail; } *sgecos++ = '\0'; shome = strchr(sgecos, ':'); if (shome == NULL) { - return matched; + goto fail; } *shome++ = '\0'; sshell = strchr(shome, ':'); if (sshell == NULL) { - return matched; + goto fail; } *sshell++ = '\0'; @@ -446,21 +446,17 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, strlen(sshell) + 1; *ret = calloc(retlen, sizeof(char)); if (*ret == NULL) { - return matched; + goto fail; } (*ret)->pw_uid = strtol(suid, &p, 10); if ((strlen(suid) == 0) || (*p != '\0')) { - free(*ret); - *ret = NULL; - return matched; + goto fail; } (*ret)->pw_gid = strtol(sgid, &p, 10); if ((strlen(sgid) == 0) || (*p != '\0')) { - free(*ret); - *ret = NULL; - return matched; + goto fail; } p = ((char*)(*ret)) + sizeof(struct passwd); @@ -478,11 +474,14 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, if (pam_set_data(pamh, buf, *ret, _unix_cleanup) != PAM_SUCCESS) { - free(*ret); - *ret = NULL; + goto fail; } } + return matched; +fail: + free(*ret); + *ret = NULL; return matched; } -- cgit v1.2.3 From 4a2d60e9112b54e36c698379cc67ad299836dcca Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 2 Jan 2024 21:31:25 +0100 Subject: pam_unix: use getline in _unix_getpwnam Use getline instead of fgets to allow arbitrarily long lines. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/support.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 336742a1..5dd3b946 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -347,19 +347,20 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, int files, int nis, struct passwd **ret) { FILE *passwd; - char buf[16384]; + char *buf = NULL; int matched = 0; char *slogin, *spasswd, *suid, *sgid, *sgecos, *shome, *sshell, *p; size_t retlen, userlen; - memset(buf, 0, sizeof(buf)); - userlen = strlen(name); - if (!matched && files && userlen < sizeof(buf) && strchr(name, ':') == NULL) { + if (!matched && files && strchr(name, ':') == NULL) { passwd = fopen("/etc/passwd", "r"); if (passwd != NULL) { - while (fgets(buf, sizeof(buf), passwd) != NULL) { - if ((buf[userlen] == ':') && + size_t n = 0; + ssize_t r; + + while ((r = getline(&buf, &n, passwd)) != -1) { + if ((size_t)r > userlen && (buf[userlen] == ':') && (strncmp(name, buf, userlen) == 0)) { p = buf + strlen(buf) - 1; while (isspace((unsigned char)*p) && (p >= buf)) { @@ -369,6 +370,9 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, break; } } + if (!matched) { + _pam_drop(buf); + } fclose(passwd); } } @@ -385,9 +389,7 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, i = yp_match(domain, "passwd.byname", name, strlen(name), &userinfo, &len); yp_unbind(domain); - if ((i == YPERR_SUCCESS) && ((size_t)len < sizeof(buf))) { - strncpy(buf, userinfo, sizeof(buf) - 1); - buf[sizeof(buf) - 1] = '\0'; + if (i == YPERR_SUCCESS && (buf = strdup(userinfo)) != NULL) { matched = 1; } } @@ -470,7 +472,11 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, p += strlen(p) + 1; (*ret)->pw_shell = strcpy(p, sshell); - snprintf(buf, sizeof(buf), "_pam_unix_getpwnam_%s", name); + _pam_drop(buf); + if (asprintf(&buf, "_pam_unix_getpwnam_%s", name) < 0) { + buf = NULL; + goto fail; + } if (pam_set_data(pamh, buf, *ret, _unix_cleanup) != PAM_SUCCESS) { @@ -480,8 +486,8 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, return matched; fail: - free(*ret); - *ret = NULL; + _pam_drop(buf); + _pam_drop(*ret); return matched; } -- cgit v1.2.3 From 1a2b73ac0befb25b4d297649a96636ca830a79f5 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 2 Jan 2024 21:33:41 +0100 Subject: pam_unix: calculate user length only if needed Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/support.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 5dd3b946..df5044e9 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -350,15 +350,16 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, char *buf = NULL; int matched = 0; char *slogin, *spasswd, *suid, *sgid, *sgecos, *shome, *sshell, *p; - size_t retlen, userlen; + size_t retlen; - userlen = strlen(name); if (!matched && files && strchr(name, ':') == NULL) { passwd = fopen("/etc/passwd", "r"); if (passwd != NULL) { - size_t n = 0; + size_t n = 0, userlen; ssize_t r; + userlen = strlen(name); + while ((r = getline(&buf, &n, passwd)) != -1) { if ((size_t)r > userlen && (buf[userlen] == ':') && (strncmp(name, buf, userlen) == 0)) { -- cgit v1.2.3 From 5a0b61224f538d832d4f88b169524c138648b0c2 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 2 Jan 2024 21:36:01 +0100 Subject: pam_unix: reduce variable visibility Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/support.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index df5044e9..a4e24210 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -346,13 +346,12 @@ static void _unix_cleanup(pam_handle_t *pamh UNUSED, void *data, int error_statu int _unix_getpwnam(pam_handle_t *pamh, const char *name, int files, int nis, struct passwd **ret) { - FILE *passwd; char *buf = NULL; int matched = 0; - char *slogin, *spasswd, *suid, *sgid, *sgecos, *shome, *sshell, *p; - size_t retlen; if (!matched && files && strchr(name, ':') == NULL) { + FILE *passwd; + passwd = fopen("/etc/passwd", "r"); if (passwd != NULL) { size_t n = 0, userlen; @@ -363,6 +362,8 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, while ((r = getline(&buf, &n, passwd)) != -1) { if ((size_t)r > userlen && (buf[userlen] == ':') && (strncmp(name, buf, userlen) == 0)) { + char *p; + p = buf + strlen(buf) - 1; while (isspace((unsigned char)*p) && (p >= buf)) { *p-- = '\0'; @@ -401,6 +402,9 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, #endif if (matched && (ret != NULL)) { + char *slogin, *spasswd, *suid, *sgid, *sgecos, *shome, *sshell, *p; + size_t retlen; + *ret = NULL; slogin = buf; -- cgit v1.2.3 From dfc2532e84cadc9b2a489de9b8b1cd5fb6679fb3 Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Thu, 4 Jan 2024 18:24:00 +0100 Subject: pam_unix: fix memory leak The the allocated line buffer on success. Reported by GCC analyzer. Fixes: 4a2d60e9 ("pam_unix: use getline in _unix_getpwnam") --- modules/pam_unix/support.c | 1 + 1 file changed, 1 insertion(+) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index a4e24210..546ef820 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -489,6 +489,7 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, } } + _pam_drop(buf); return matched; fail: _pam_drop(buf); -- cgit v1.2.3 From 9f19e7da7a014e022cdbba06accca171adef27e0 Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Thu, 4 Jan 2024 18:23:59 +0100 Subject: pam_unix: set close-on-exec Since the module operates on sensitive files set the close-on-exec flag, to avoid file descriptor leaks if there is ever any sibling thread. The fopen(3) mode "e" is supported in glibc since version 2.7 (released in 2007), and ignored prior, see: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=65d834b0add966dbbdb5ed1e916c60b2b2d87f10 --- modules/pam_unix/lckpwdf.-c | 17 +++-------------- modules/pam_unix/pam_unix_passwd.c | 2 +- modules/pam_unix/passverify.c | 16 ++++++++-------- modules/pam_unix/support.c | 2 +- 4 files changed, 13 insertions(+), 24 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/lckpwdf.-c b/modules/pam_unix/lckpwdf.-c index c3e63155..4d0f0ad3 100644 --- a/modules/pam_unix/lckpwdf.-c +++ b/modules/pam_unix/lckpwdf.-c @@ -35,15 +35,6 @@ static int lockfd = -1; -static int set_close_on_exec(int fd) -{ - int flags = fcntl(fd, F_GETFD, 0); - if (flags == -1) - return -1; - flags |= FD_CLOEXEC; - return fcntl(fd, F_SETFD, flags); -} - static int do_lock(int fd) { struct flock fl; @@ -70,7 +61,7 @@ static int lckpwdf(void) #ifdef WITH_SELINUX if(is_selinux_enabled()>0) { - lockfd = open(LOCKFILE, O_WRONLY); + lockfd = open(LOCKFILE, O_WRONLY | O_CLOEXEC); if(lockfd == -1 && errno == ENOENT) { char *create_context_raw; @@ -82,18 +73,16 @@ static int lckpwdf(void) freecon(create_context_raw); if(rc) return -1; - lockfd = open(LOCKFILE, O_CREAT | O_WRONLY, 0600); + lockfd = open(LOCKFILE, O_CREAT | O_WRONLY | O_CLOEXEC, 0600); if(setfscreatecon_raw(NULL)) return -1; } } else #endif - lockfd = open(LOCKFILE, O_CREAT | O_WRONLY, 0600); + lockfd = open(LOCKFILE, O_CREAT | O_WRONLY | O_CLOEXEC, 0600); if (lockfd == -1) return -1; - if (set_close_on_exec(lockfd) == -1) - goto cleanup_fd; memset(&act, 0, sizeof act); act.sa_handler = alarm_catch; diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c index fe3f566a..3a223949 100644 --- a/modules/pam_unix/pam_unix_passwd.c +++ b/modules/pam_unix/pam_unix_passwd.c @@ -346,7 +346,7 @@ static int check_old_password(const char *forwho, const char *newpass) size_t n = 0; size_t len = strlen(forwho); - opwfile = fopen(OLD_PASSWORDS_FILE, "r"); + opwfile = fopen(OLD_PASSWORDS_FILE, "re"); if (opwfile == NULL) return PAM_ABORT; diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 60d9ceca..303929a4 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -400,7 +400,7 @@ crypt_make_salt(char *where, int length) int fd; int rv; - if ((rv = fd = open(PAM_PATH_RANDOMDEV, O_RDONLY)) != -1) { + if ((rv = fd = open(PAM_PATH_RANDOMDEV, O_RDONLY | O_CLOEXEC)) != -1) { while ((rv = read(fd, where, length)) != length && errno == EINTR); close (fd); } @@ -557,7 +557,7 @@ unix_selinux_confined(void) } /* let's try opening shadow read only */ - if ((fd=open("/etc/shadow", O_RDONLY)) != -1) { + if ((fd=open("/etc/shadow", O_RDONLY | O_CLOEXEC)) != -1) { close(fd); confined = 0; return confined; @@ -695,14 +695,14 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, freecon(passwd_context_raw); } #endif - pwfile = fopen(OPW_TMPFILE, "w"); + pwfile = fopen(OPW_TMPFILE, "we"); umask(oldmask); if (pwfile == NULL) { err = 1; goto done; } - opwfile = fopen(OLD_PASSWORDS_FILE, "r"); + opwfile = fopen(OLD_PASSWORDS_FILE, "re"); if (opwfile == NULL) { fclose(pwfile); err = 1; @@ -858,14 +858,14 @@ PAMH_ARG_DECL(int unix_update_passwd, freecon(passwd_context_raw); } #endif - pwfile = fopen(PW_TMPFILE, "w"); + pwfile = fopen(PW_TMPFILE, "we"); umask(oldmask); if (pwfile == NULL) { err = 1; goto done; } - opwfile = fopen("/etc/passwd", "r"); + opwfile = fopen("/etc/passwd", "re"); if (opwfile == NULL) { fclose(pwfile); err = 1; @@ -983,14 +983,14 @@ PAMH_ARG_DECL(int unix_update_shadow, freecon(shadow_context_raw); } #endif - pwfile = fopen(SH_TMPFILE, "w"); + pwfile = fopen(SH_TMPFILE, "we"); umask(oldmask); if (pwfile == NULL) { err = 1; goto done; } - opwfile = fopen("/etc/shadow", "r"); + opwfile = fopen("/etc/shadow", "re"); if (opwfile == NULL) { fclose(pwfile); err = 1; diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 546ef820..d391973f 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -352,7 +352,7 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, if (!matched && files && strchr(name, ':') == NULL) { FILE *passwd; - passwd = fopen("/etc/passwd", "r"); + passwd = fopen("/etc/passwd", "re"); if (passwd != NULL) { size_t n = 0, userlen; ssize_t r; -- cgit v1.2.3 From b7b96362087414e52524d3d9d9b3faa21e1db620 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Wed, 24 Jan 2024 18:57:42 +0100 Subject: pam_unix: try to set uid to 0 for unix_chkpwd The geteuid check does not cover all cases. If a program runs with elevated capabilities like CAP_SETUID then we can still check credentials of other users. Keep logging for future analysis though. Resolves: https://github.com/linux-pam/linux-pam/issues/747 Fixes: b3020da7da38 ("pam_unix/passverify: always run the helper to obtain shadow password file entries") Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/pam_unix_acct.c | 17 +++++++++-------- modules/pam_unix/support.c | 14 +++++++------- 2 files changed, 16 insertions(+), 15 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/pam_unix_acct.c b/modules/pam_unix/pam_unix_acct.c index 8f5ed3e0..7ffcb9e3 100644 --- a/modules/pam_unix/pam_unix_acct.c +++ b/modules/pam_unix/pam_unix_acct.c @@ -110,14 +110,15 @@ int _unix_run_verify_binary(pam_handle_t *pamh, unsigned long long ctrl, _exit(PAM_AUTHINFO_UNAVAIL); } - if (geteuid() == 0) { - /* must set the real uid to 0 so the helper will not error - out if pam is called from setuid binary (su, sudo...) */ - if (setuid(0) == -1) { - pam_syslog(pamh, LOG_ERR, "setuid failed: %m"); - printf("-1\n"); - fflush(stdout); - _exit(PAM_AUTHINFO_UNAVAIL); + /* must set the real uid to 0 so the helper will not error + out if pam is called from setuid binary (su, sudo...) */ + if (setuid(0) == -1) { + uid_t euid = geteuid(); + pam_syslog(pamh, euid == 0 ? LOG_ERR : LOG_DEBUG, "setuid failed: %m"); + if (euid == 0) { + printf("-1\n"); + fflush(stdout); + _exit(PAM_AUTHINFO_UNAVAIL); } } diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index d391973f..69811048 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -562,13 +562,13 @@ static int _unix_run_helper_binary(pam_handle_t *pamh, const char *passwd, _exit(PAM_AUTHINFO_UNAVAIL); } - if (geteuid() == 0) { - /* must set the real uid to 0 so the helper will not error - out if pam is called from setuid binary (su, sudo...) */ - if (setuid(0) == -1) { - D(("setuid failed")); - _exit(PAM_AUTHINFO_UNAVAIL); - } + /* must set the real uid to 0 so the helper will not error + out if pam is called from setuid binary (su, sudo...) */ + if (setuid(0) == -1) { + D(("setuid failed")); + if (geteuid() == 0) { + _exit(PAM_AUTHINFO_UNAVAIL); + } } /* exec binary helper */ -- cgit v1.2.3 From e559de0cdd6134a4b87a79aab6928fb1e8c7194c Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Thu, 22 Feb 2024 16:16:45 +0100 Subject: pam_unix: avoid string formatting of NULL Since the struct member user might be NULL use the same condition for the value as for the preceding key. Reported-by: Yugend --- modules/pam_unix/support.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 69811048..90f6bbe9 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -317,7 +317,8 @@ static void _cleanup_failures(pam_handle_t * pamh, void *fl, int err) tty ? (const char *)tty : "", ruser ? (const char *)ruser : "", rhost ? (const char *)rhost : "", (failure->user && failure->user[0] != '\0') - ? " user=" : "", failure->user + ? " user=" : "", + failure->user ? failure->user : "" ); if (failure->count > UNIX_MAX_RETRIES) { @@ -836,7 +837,7 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name rhost ? (const char *)rhost : "", (new->user && new->user[0] != '\0') ? " user=" : "", - new->user + new->user ? new->user : "" ); new->count = 1; } -- cgit v1.2.3 From f4e016bb697b7807dda2534e2a0d23c8b44de52f Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sun, 3 Mar 2024 22:33:55 +0100 Subject: pam_unix: use yp functions only if nis requested It can happen that yp functions are found in system but their header files are not available. In this case, do not call them. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/support.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 90f6bbe9..d11b2758 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -380,7 +380,7 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, } } -#if defined(HAVE_YP_GET_DEFAULT_DOMAIN) && defined (HAVE_YP_BIND) && defined (HAVE_YP_MATCH) && defined (HAVE_YP_UNBIND) +#if defined(HAVE_NIS) && defined(HAVE_YP_GET_DEFAULT_DOMAIN) && defined (HAVE_YP_BIND) && defined (HAVE_YP_MATCH) && defined (HAVE_YP_UNBIND) if (!matched && nis) { char *userinfo = NULL, *domain = NULL; int len = 0, i; -- cgit v1.2.3 From 8c2bb459417f363bb6a8ad54081ba671d039a8f6 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Thu, 29 Aug 2024 08:00:00 +0000 Subject: pam_unix: do not check for HAVE_PAM_FAIL_DELAY Given that pam_fail_delay is always provided by libpam, checking for HAVE_PAM_FAIL_DELAY may have any sense only in third-party modules. --- modules/pam_unix/support.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'modules/pam_unix/support.c') diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index d11b2758..dfdc9fc1 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -720,12 +720,10 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name D(("called")); -#ifdef HAVE_PAM_FAIL_DELAY if (off(UNIX_NODELAY, ctrl)) { D(("setting delay")); (void) pam_fail_delay(pamh, 2000000); /* 2 sec delay for on failure */ } -#endif /* locate the entry for this user */ -- cgit v1.2.3