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_namespace/pam_namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/pam_namespace/pam_namespace.c') 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++; /* -- 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_namespace/pam_namespace.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 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_namespace/pam_namespace.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 04134cc04a6c36acd52aa92c955ae0eba72fd038 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 5 Dec 2023 23:20:46 +0100 Subject: treewide: fix typos Typos found with codespell Signed-off-by: Tobias Stoeckmann --- libpamc/pamc_load.c | 2 +- libpamc/test/agents/secret@here | 2 +- libpamc/test/modules/pam_secret.c | 2 +- modules/pam_access/pam_access.c | 2 +- modules/pam_limits/pam_limits.c | 2 +- modules/pam_namespace/pam_namespace.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/libpamc/pamc_load.c b/libpamc/pamc_load.c index c1a39f5b..cbf4b994 100644 --- a/libpamc/pamc_load.c +++ b/libpamc/pamc_load.c @@ -247,7 +247,7 @@ int pamc_load(pamc_handle_t pch, const char *agent_id) pamc_agent_t *agent; size_t length; - /* santity checking */ + /* sanity checking */ if (pch == NULL) { D(("pch is NULL")); diff --git a/libpamc/test/agents/secret@here b/libpamc/test/agents/secret@here index 8d82c013..2bef6474 100755 --- a/libpamc/test/agents/secret@here +++ b/libpamc/test/agents/secret@here @@ -83,7 +83,7 @@ sub HandleAgentSelection ($) { return (0x04, ""); } - # the selection request is acompanied with a hexadecimal cookie + # the selection request is accompanied with a hexadecimal cookie my @tokens = split '\|', $payload; unless ((scalar @tokens) == 2) { diff --git a/libpamc/test/modules/pam_secret.c b/libpamc/test/modules/pam_secret.c index c4b32ae9..2fe7066c 100644 --- a/libpamc/test/modules/pam_secret.c +++ b/libpamc/test/modules/pam_secret.c @@ -378,7 +378,7 @@ static int auth_sequence(pam_handle_t *pamh, /* expect to receive the following {|} */ if (new->current_reply == NULL) { - D(("converstation returned [%s] but gave no reply", + D(("conversation returned [%s] but gave no reply", pam_strerror(pamh, retval))); new->state = PS_STATE_DEAD; return PAM_CONV_ERR; diff --git a/modules/pam_access/pam_access.c b/modules/pam_access/pam_access.c index a8efdf30..85775114 100644 --- a/modules/pam_access/pam_access.c +++ b/modules/pam_access/pam_access.c @@ -158,7 +158,7 @@ parse_args(pam_handle_t *pamh, struct login_info *loginfo, return 1; /* OK */ } -/* --- evaluting all files in VENDORDIR/security/access.d and /etc/security/access.d --- */ +/* --- evaluating all files in VENDORDIR/security/access.d and /etc/security/access.d --- */ static const char *base_name(const char *path) { const char *base = strrchr(path, '/'); diff --git a/modules/pam_limits/pam_limits.c b/modules/pam_limits/pam_limits.c index d9ce6cbe..fc541330 100644 --- a/modules/pam_limits/pam_limits.c +++ b/modules/pam_limits/pam_limits.c @@ -1122,7 +1122,7 @@ static int setup_limits(pam_handle_t *pamh, return retval; } -/* --- evaluting all files in VENDORDIR/security/limits.d and /etc/security/limits.d --- */ +/* --- evaluating all files in VENDORDIR/security/limits.d and /etc/security/limits.d --- */ static const char * base_name(const char *path) { diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 36c8261b..6937b6ac 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -39,7 +39,7 @@ #include "pam_namespace.h" #include "argv_parse.h" -/* --- evaluting all files in VENDORDIR/security/namespace.d and /etc/security/namespace.d --- */ +/* --- evaluating all files in VENDORDIR/security/namespace.d and /etc/security/namespace.d --- */ static const char *base_name(const char *path) { const char *base = strrchr(path, '/'); -- cgit v1.2.3 From c2fafe1be0fb72aa1bd521efe2f524074bf143c7 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Fri, 15 Dec 2023 10:14:11 +0100 Subject: treewide: replace malloc followed by strcpy with strdup Suggested-by: Benny Baumann Signed-off-by: Tobias Stoeckmann --- libpam/pam_misc.c | 8 +------- libpamc/pamc_load.c | 9 +++------ modules/pam_listfile/pam_listfile.c | 3 +-- modules/pam_namespace/pam_namespace.c | 3 +-- 4 files changed, 6 insertions(+), 17 deletions(-) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/libpam/pam_misc.c b/libpam/pam_misc.c index a843ebc7..721b36f3 100644 --- a/libpam/pam_misc.c +++ b/libpam/pam_misc.c @@ -116,14 +116,8 @@ char *_pam_strdup(const char *x) register char *new=NULL; if (x != NULL) { - register size_t len; - - len = strlen (x) + 1; /* length of string including NUL */ - if ((new = malloc(len)) == NULL) { - len = 0; + if ((new = strdup(x)) == NULL) { pam_syslog(NULL, LOG_CRIT, "_pam_strdup: failed to get memory"); - } else { - strcpy (new, x); } x = NULL; } diff --git a/libpamc/pamc_load.c b/libpamc/pamc_load.c index cbf4b994..7efd5721 100644 --- a/libpamc/pamc_load.c +++ b/libpamc/pamc_load.c @@ -224,14 +224,13 @@ int pamc_disable(pamc_handle_t pch, const char *agent_id) return PAM_BPC_FALSE; } - block->id = malloc(1 + strlen(agent_id)); + block->id = strdup(agent_id); if (block->id == NULL) { D(("no memory for agent id")); free(block); return PAM_BPC_FALSE; } - strcpy(block->id, agent_id); block->next = pch->blocked_agents; pch->blocked_agents = block; @@ -372,10 +371,8 @@ static pamc_id_node_t *__pamc_add_node(pamc_id_node_t *root, const char *id, pamc_id_node_t *node = calloc(1, sizeof(pamc_id_node_t)); if (node) { - node->agent_id = malloc(1+strlen(id)); - if (node->agent_id) { - strcpy(node->agent_id, id); - } else { + node->agent_id = strdup(id); + if (node->agent_id == NULL) { free(node); node = NULL; } diff --git a/modules/pam_listfile/pam_listfile.c b/modules/pam_listfile/pam_listfile.c index a01b5a8a..3e6a7092 100644 --- a/modules/pam_listfile/pam_listfile.c +++ b/modules/pam_listfile/pam_listfile.c @@ -106,10 +106,9 @@ pam_sm_authenticate (pam_handle_t *pamh, int flags UNUSED, } else if(!strcmp(mybuf,"file")) { free(ifname); - ifname = malloc(strlen(myval)+1); + ifname = strdup(myval); if (!ifname) return PAM_BUF_ERR; - strcpy(ifname,myval); } else if(!strcmp(mybuf,"item")) if(!strcmp(myval,"user")) citem = PAM_USER; diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 6937b6ac..d78111b3 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -1295,13 +1295,12 @@ 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 = malloc(strlen(ipath)+1); + inst_parent = strdup(ipath); if (!inst_parent) { pam_syslog(idata->pamh, LOG_CRIT, "Error allocating pathname string"); return PAM_SESSION_ERR; } - strcpy(inst_parent, ipath); trailing_slash = strrchr(inst_parent, '/'); if (trailing_slash) *trailing_slash = '\0'; -- 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_namespace/pam_namespace.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 c3d2861800f9af0723e18609ae9852951453d65c Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Wed, 27 Dec 2023 13:29:26 +0100 Subject: pam_namespace: fix double-free on parse error in namespace.conf If a line in namespace.conf only consists of one field then the error handling logic in process_line() ends up in a double-free, resulting in a process abort in libc. It looks like instead of NULLing the `dir` variable, the `instance_prefix` is NULLed, without purpose. Fix this. --- modules/pam_namespace/pam_namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 1c42b0f4..b7cdcfa4 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -529,7 +529,7 @@ static int process_line(char *line, const char *home, const char *rhome, instance_prefix = config_options[1]; if (instance_prefix == NULL) { pam_syslog(idata->pamh, LOG_NOTICE, "Invalid line missing instance_prefix"); - instance_prefix = NULL; + dir = NULL; goto skipping; } method = config_options[2]; -- cgit v1.2.3 From ddfc1301282fe87e245716b04437422476e8bc35 Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Wed, 27 Dec 2023 14:22:48 +0100 Subject: pam_namespace: cleanup_tmpdirs(): use proper error message --- modules/pam_namespace/pam_namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index b7cdcfa4..40edc9f7 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -1833,7 +1833,7 @@ static int cleanup_tmpdirs(struct instance_data *idata) } } else if (pid < 0) { pam_syslog(idata->pamh, LOG_ERR, - "Cannot fork to run namespace init script, %m"); + "Cannot fork to cleanup temporary directory, %m"); rc = PAM_SESSION_ERR; goto out; } -- cgit v1.2.3 From c48622d95e3d441fcee6228be1952fe7ee299f6d Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Tue, 2 Jan 2024 12:13:19 +0100 Subject: pam_namespace: close unnecessary file descriptors before exec() Currently the `rm` subprocess and the namespace init script inherit a random set of open file descriptors from the process running PAM. Depending on the actual PAM stack configuration these can even be security sensitive files. In any case it is unclean to inherit unexpected open file descriptors to child processes like this. To address this close all file descriptors except stdio before executing a new program. --- modules/pam_namespace/pam_namespace.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 40edc9f7..92372ab4 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -53,6 +53,14 @@ compare_filename(const void *a, const void *b) base_name(* (char * const *) b)); } +static void close_fds_pre_exec(struct instance_data *idata) +{ + if (pam_modutil_sanitize_helper_fds(idata->pamh, PAM_MODUTIL_IGNORE_FD, + PAM_MODUTIL_IGNORE_FD, PAM_MODUTIL_IGNORE_FD) < 0) { + _exit(1); + } +} + /* Evaluating a list of files which have to be parsed in the right order: * * - If etc/security/namespace.d/@filename@.conf exists, then @@ -1379,6 +1387,8 @@ static int inst_init(const struct polydir_s *polyptr, const char *ipath, /* ignore failures, they don't matter */ } + close_fds_pre_exec(idata); + if (execle(init_script, init_script, polyptr->dir, ipath, newdir?"1":"0", idata->user, NULL, envp) < 0) _exit(1); @@ -1817,6 +1827,7 @@ static int cleanup_tmpdirs(struct instance_data *idata) _exit(1); } #endif + close_fds_pre_exec(idata); if (execle("/bin/rm", "/bin/rm", "-rf", pptr->instance_prefix, NULL, envp) < 0) _exit(1); } else if (pid > 0) { -- cgit v1.2.3 From 353aa13b9c81beb4ba7f02ae57b0722de6a4d565 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 2 Jan 2024 18:34:45 +0100 Subject: pam_namespace: validate amount of uids in config If more than INT_MAX uids are found in a configuration line, the variable `count` would trigger a signed integer overflow. If more than UINT_MAX uids are found in a configuration line, then the `num_uids` counter is invalid, which could eventually lead to out of boundary accesses. Also make sure that size multiplication for malloc does not overflow. Signed-off-by: Tobias Stoeckmann --- modules/pam_namespace/pam_namespace.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 92372ab4..09d19ca0 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -637,7 +637,7 @@ static int process_line(char *line, const char *home, const char *rhome, if (uids) { uid_t *uidptr; const char *ustr, *sstr; - int count, i; + size_t count, i; if (*uids == '~') { poly->flags |= POLYDIR_EXCLUSIVE; @@ -646,6 +646,11 @@ static int process_line(char *line, const char *home, const char *rhome, for (count = 0, ustr = sstr = uids; sstr; ustr = sstr + 1, count++) sstr = strchr(ustr, ','); + if (count > UINT_MAX || count > SIZE_MAX / sizeof(uid_t)) { + pam_syslog(idata->pamh, LOG_ERR, "Too many uids encountered in configuration"); + goto skipping; + } + poly->num_uids = count; poly->uid = malloc(count * sizeof (uid_t)); uidptr = poly->uid; -- cgit v1.2.3 From 94c798c2d82a3df4d1e98e0a6855d92a5a4b1450 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Fri, 5 Jan 2024 22:54:54 +0100 Subject: treewide: fix typos in comments Signed-off-by: Tobias Stoeckmann --- libpam/include/security/_pam_macros.h | 2 +- libpam/pam_audit.c | 2 +- libpam/pam_dispatch.c | 2 +- libpam/pam_end.c | 2 +- libpam/pam_misc.c | 2 +- modules/pam_env/pam_env.c | 2 +- modules/pam_filter/pam_filter.c | 4 ++-- modules/pam_namespace/pam_namespace.c | 2 +- modules/pam_namespace/pam_namespace.h | 2 +- 9 files changed, 10 insertions(+), 10 deletions(-) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/libpam/include/security/_pam_macros.h b/libpam/include/security/_pam_macros.h index e36f4023..01274428 100644 --- a/libpam/include/security/_pam_macros.h +++ b/libpam/include/security/_pam_macros.h @@ -49,7 +49,7 @@ do { \ } while (0) /* - * WARNING: Do NOT use this macro, as it does not reliable override the memory. + * WARNING: Do NOT use this macro, as it does not reliably override the memory. */ #define _pam_drop_reply(/* struct pam_response * */ reply, /* int */ replies) \ diff --git a/libpam/pam_audit.c b/libpam/pam_audit.c index 97a9a929..5619ba0c 100644 --- a/libpam/pam_audit.c +++ b/libpam/pam_audit.c @@ -203,7 +203,7 @@ int _pam_audit_end(pam_handle_t *pamh, int status UNUSED) { if (! (pamh->audit_state & PAMAUDIT_LOGGED)) { - /* PAM library is being shut down without any of the auditted + /* PAM library is being shut down without any of the audited * stacks having been run. Assume that this is sshd faking * things for an unknown user. */ diff --git a/libpam/pam_dispatch.c b/libpam/pam_dispatch.c index d237e68d..60ec85bd 100644 --- a/libpam/pam_dispatch.c +++ b/libpam/pam_dispatch.c @@ -28,7 +28,7 @@ /* * walk a stack of modules. Interpret the administrator's instructions - * when combining the return code of each module. + * when combining the return codes of each module. */ static int _pam_dispatch_aux(pam_handle_t *pamh, int flags, struct handler *h, diff --git a/libpam/pam_end.c b/libpam/pam_end.c index c374dbca..c728f1da 100644 --- a/libpam/pam_end.c +++ b/libpam/pam_end.c @@ -26,7 +26,7 @@ int pam_end(pam_handle_t *pamh, int pam_status) _pam_audit_end(pamh, pam_status); #endif - /* first liberate the modules (it is not inconcevible that the + /* first liberate the modules (it is not inconceivable that the modules may need to use the service_name etc. to clean up) */ _pam_free_data(pamh, pam_status); diff --git a/libpam/pam_misc.c b/libpam/pam_misc.c index c91aa693..e379d2f9 100644 --- a/libpam/pam_misc.c +++ b/libpam/pam_misc.c @@ -206,7 +206,7 @@ size_t _pam_mkargv(const char *s, char ***argv, int *argc) /* * this function is used to protect the modules from accidental or - * semi-mallicious harm that an application may do to confuse the API. + * semi-malicious harm that an application may do to confuse the API. */ void _pam_sanitize(pam_handle_t *pamh) diff --git a/modules/pam_env/pam_env.c b/modules/pam_env/pam_env.c index 9604207a..a214593c 100644 --- a/modules/pam_env/pam_env.c +++ b/modules/pam_env/pam_env.c @@ -749,7 +749,7 @@ _check_var(pam_handle_t *pamh, VAR *var) return retval; } - /* Now its easy */ + /* Now it's easy */ if (var->override && *(var->override)) { /* if there is a non-empty string in var->override, we use it */ diff --git a/modules/pam_filter/pam_filter.c b/modules/pam_filter/pam_filter.c index 62b118bb..ed315b13 100644 --- a/modules/pam_filter/pam_filter.c +++ b/modules/pam_filter/pam_filter.c @@ -318,7 +318,7 @@ set_filter (pam_handle_t *pamh, int flags UNUSED, int ctrl, close(t); } - /* make this process it's own process leader */ + /* make this process its own process leader */ if (setsid() == -1) { pam_syslog(pamh, LOG_ERR, "child cannot become new session: %m"); @@ -468,7 +468,7 @@ set_filter (pam_handle_t *pamh, int flags UNUSED, int ctrl, } else if (chid == child2) { /* - * if the filter has exited. Let the child die + * if the filter has exited, let the child die * naturally below */ if (WIFEXITED(lstatus) || WIFSIGNALED(lstatus)) diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 09d19ca0..2528cff8 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -473,7 +473,7 @@ static int parse_method(char *method, struct polydir_s *poly, * of the namespace configuration file. It skips over comments and incomplete * or malformed lines. It processes a valid line with information on * polyinstantiating a directory by populating appropriate fields of a - * polyinstatiated directory structure and then calling add_polydir_entry to + * polyinstantiated directory structure and then calling add_polydir_entry to * add that entry to the linked list of polyinstantiated directories. */ static int process_line(char *line, const char *home, const char *rhome, diff --git a/modules/pam_namespace/pam_namespace.h b/modules/pam_namespace/pam_namespace.h index a991b4c4..fd393d17 100644 --- a/modules/pam_namespace/pam_namespace.h +++ b/modules/pam_namespace/pam_namespace.h @@ -114,7 +114,7 @@ #define PAMNS_MOUNT_PRIVATE 0x00080000 /* Make the polydir mounts private */ /* polydir flags */ -#define POLYDIR_EXCLUSIVE 0x00000001 /* polyinstatiate exclusively for override uids */ +#define POLYDIR_EXCLUSIVE 0x00000001 /* polyinstantiate exclusively for override uids */ #define POLYDIR_CREATE 0x00000002 /* create the polydir */ #define POLYDIR_NOINIT 0x00000004 /* no init script */ #define POLYDIR_SHARED 0x00000008 /* share context/level instances among users */ -- cgit v1.2.3 From 031bb5a5d0d950253b68138b498dc93be69a64cb Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Wed, 27 Dec 2023 14:01:59 +0100 Subject: pam_namespace: protect_dir(): use O_DIRECTORY to prevent local DoS situations Without O_DIRECTORY the path crawling logic is subject to e.g. FIFOs being placed in user controlled directories, causing the PAM module to block indefinitely during `openat()`. Pass O_DIRECTORY to cause the `openat()` to fail if the path does not refer to a directory. With this the check whether the final path element is a directory becomes unnecessary, drop it. --- modules/pam_namespace/pam_namespace.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 2528cff8..f72d6718 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -1201,7 +1201,7 @@ static int protect_dir(const char *path, mode_t mode, int do_mkdir, int dfd = AT_FDCWD; int dfd_next; int save_errno; - int flags = O_RDONLY; + int flags = O_RDONLY | O_DIRECTORY; int rv = -1; struct stat st; @@ -1255,22 +1255,6 @@ static int protect_dir(const char *path, mode_t mode, int do_mkdir, rv = openat(dfd, dir, flags); } - if (rv != -1) { - if (fstat(rv, &st) != 0) { - save_errno = errno; - close(rv); - rv = -1; - errno = save_errno; - goto error; - } - if (!S_ISDIR(st.st_mode)) { - close(rv); - errno = ENOTDIR; - rv = -1; - goto error; - } - } - if (flags & O_NOFOLLOW) { /* we are inside user-owned dir - protect */ if (protect_mount(rv, p, idata) == -1) { -- cgit v1.2.3 From cc9d40b7cdbd3e15ccaa324a0dda1680ef9dea13 Mon Sep 17 00:00:00 2001 From: Jacob Heider Date: Wed, 17 Jan 2024 11:49:26 -0500 Subject: pam_namespace: include stdint.h pam_namespace.c makes use of SIZE_MAX but doesn't include stdint.h, resulting in the following build failures on 1.6.0: pam_namespace.c: In function 'process_line': pam_namespace.c:649:41: error: 'SIZE_MAX' undeclared (first use in this function) 649 | if (count > UINT_MAX || count > SIZE_MAX / sizeof(uid_t)) { | ^~~~~~~~ pam_namespace.c:41:1: note: 'SIZE_MAX' is defined in header ''; did you forget to '#include '? 40 | #include "argv_parse.h" +++ |+#include 41 | pam_namespace.c:649:41: note: each undeclared identifier is reported only once for each function it appears in 649 | if (count > UINT_MAX || count > SIZE_MAX / sizeof(uid_t)) { | ^~~~~~~~ Fixes: v1.6.0~100 ("pam_namespace: validate amount of uids in config") Resolves: https://github.com/linux-pam/linux-pam/issues/733 --- modules/pam_namespace/pam_namespace.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index f72d6718..b16731c2 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -34,6 +34,8 @@ #define _ATFILE_SOURCE +#include "config.h" +#include #include "pam_cc_compat.h" #include "pam_inline.h" #include "pam_namespace.h" -- cgit v1.2.3 From dc370deab4752c264c20e16684d02692532c351b Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Thu, 22 Feb 2024 17:04:15 +0100 Subject: conf/modules: constify read-only data arrays --- conf/pam_conv1/pam_conv_y.y | 2 +- modules/pam_ftp/pam_ftp.c | 2 +- modules/pam_limits/pam_limits.c | 4 ++-- modules/pam_namespace/pam_namespace.c | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/conf/pam_conv1/pam_conv_y.y b/conf/pam_conv1/pam_conv_y.y index e2cf85c3..a800cebc 100644 --- a/conf/pam_conv1/pam_conv_y.y +++ b/conf/pam_conv1/pam_conv_y.y @@ -178,7 +178,7 @@ tok const char *old_to_new_ctrl_flag(const char *old) { - static const char *clist[] = { + static const char *const clist[] = { "requisite", "required", "sufficient", diff --git a/modules/pam_ftp/pam_ftp.c b/modules/pam_ftp/pam_ftp.c index 41fb9f48..cfb0f177 100644 --- a/modules/pam_ftp/pam_ftp.c +++ b/modules/pam_ftp/pam_ftp.c @@ -84,7 +84,7 @@ static int lookup(const char *name, const char *list, char **_user) } } else { #define MAX_L 2 - static const char *l[MAX_L] = { "ftp", "anonymous" }; + static const char *const l[MAX_L] = { "ftp", "anonymous" }; int i; for (i=0; iuser}; const char *rvar_values[] = {rhome, idata->ruser}; size_t len; -- cgit v1.2.3 From 667204d7e3e4a0341c529f7566d62dd64dd80866 Mon Sep 17 00:00:00 2001 From: Iker Pedrosa Date: Wed, 22 May 2024 12:25:34 +0200 Subject: pam_namespace: free SELinux context * modules/pam_namespace/pam_namespace.c [WITH_SELINUX] (form_context): Free SELinux context before returning. ``` Error: RESOURCE_LEAK (CWE-772): Linux-PAM-1.6.0/modules/pam_namespace/pam_namespace.c:928: alloc_arg: "getexeccon" allocates memory that is stored into "scon". Linux-PAM-1.6.0/modules/pam_namespace/pam_namespace.c:1004: leaked_storage: Variable "scon" going out of scope leaks the storage it points to. 1002| } 1003| /* Should never get here */ 1004|-> return PAM_SUCCESS; 1005| } 1006| #endif ``` Resolves: https://issues.redhat.com/browse/RHEL-36475 Signed-off-by: Iker Pedrosa --- modules/pam_namespace/pam_namespace.c | 1 + 1 file changed, 1 insertion(+) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index e499d95a..781dac20 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -1003,6 +1003,7 @@ static int form_context(const struct polydir_s *polyptr, return rc; } /* Should never get here */ + freecon(scon); return PAM_SUCCESS; } #endif -- cgit v1.2.3 From bd2f695b3d89efe0c52bba975f9540634125178a Mon Sep 17 00:00:00 2001 From: Iker Pedrosa Date: Wed, 22 May 2024 12:29:07 +0200 Subject: pam_namespace: free SELinux context on error path * modules/pam_namespace/pam_namespace.c (create_polydir) [WITH_SELINUX]: Free SELinux context in case of an error. ``` Error: RESOURCE_LEAK (CWE-772): Linux-PAM-1.6.0/modules/pam_namespace/pam_namespace.c:1433: alloc_arg: "getfscreatecon_raw" allocates memory that is stored into "oldcon_raw". Linux-PAM-1.6.0/modules/pam_namespace/pam_namespace.c:1462: leaked_storage: Variable "oldcon_raw" going out of scope leaks the storage it points to. 1460| pam_syslog(idata->pamh, LOG_ERR, 1461| "Error creating directory %s: %m", dir); 1462|-> return PAM_SESSION_ERR; 1463| } 1464| ``` Resolves: https://issues.redhat.com/browse/RHEL-36475 Signed-off-by: Iker Pedrosa --- modules/pam_namespace/pam_namespace.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 781dac20..2dab49ef 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -1462,6 +1462,9 @@ static int create_polydir(struct polydir_s *polyptr, if (rc == -1) { pam_syslog(idata->pamh, LOG_ERR, "Error creating directory %s: %m", dir); +#ifdef WITH_SELINUX + freecon(oldcon_raw); +#endif return PAM_SESSION_ERR; } -- cgit v1.2.3 From 5d548fec5a6a1c7016ce7de108164f100583ba49 Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Fri, 24 May 2024 17:38:31 +0200 Subject: pam_namespace: log getfscreatecon(3) failure Log in case the current fscreate context could not be retrieved. --- modules/pam_namespace/pam_namespace.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'modules/pam_namespace/pam_namespace.c') diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 2dab49ef..ba7910f6 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -1433,7 +1433,9 @@ static int create_polydir(struct polydir_s *polyptr, #ifdef WITH_SELINUX if (idata->flags & PAMNS_SELINUX_ENABLED) { - getfscreatecon_raw(&oldcon_raw); + if (getfscreatecon_raw(&oldcon_raw) != 0) + pam_syslog(idata->pamh, LOG_NOTICE, + "Error retrieving fs create context: %m"); label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); if (!label_handle) { -- cgit v1.2.3