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/argv_parse.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/pam_namespace/argv_parse.c') 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. -- cgit v1.2.3 From 954234f0a477636eab751a6601d34bab1db41b0e Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Thu, 14 Dec 2023 08:00:00 +0000 Subject: treewide: assume free(NULL) is no-op The C standard guarantees that if the argument of free() is a null pointer, no action occurs. --- doc/specs/parse_y.y | 4 +--- libpam/include/security/_pam_macros.h | 9 +++------ libpam/pam_modutil_cleanup.c | 4 +--- libpam/pam_modutil_getgrgid.c | 4 +--- libpam/pam_modutil_getgrnam.c | 4 +--- libpam/pam_modutil_getpwnam.c | 4 +--- libpam/pam_modutil_getpwuid.c | 4 +--- libpam/pam_modutil_getspnam.c | 4 +--- libpamc/pamc_load.c | 6 ++---- libpamc/test/modules/pam_secret.c | 4 +--- modules/pam_filter/pam_filter.c | 3 +-- modules/pam_listfile/pam_listfile.c | 6 +++--- modules/pam_namespace/argv_parse.c | 5 ++--- modules/pam_stress/pam_stress.c | 6 ++---- modules/pam_timestamp/hmac_openssl_wrapper.c | 4 +--- modules/pam_unix/pam_unix_auth.c | 3 +-- modules/pam_xauth/pam_xauth.c | 10 +++------- 17 files changed, 26 insertions(+), 58 deletions(-) (limited to 'modules/pam_namespace/argv_parse.c') diff --git a/doc/specs/parse_y.y b/doc/specs/parse_y.y index b195f5d3..96809cd2 100644 --- a/doc/specs/parse_y.y +++ b/doc/specs/parse_y.y @@ -282,9 +282,7 @@ char *new_counter(const char *key) counter_root = set_key(counter_root, key, new); - if (last_label) { - free(last_label); - } + free(last_label); last_label = strdup(new); return new; diff --git a/libpam/include/security/_pam_macros.h b/libpam/include/security/_pam_macros.h index 8044e451..e36f4023 100644 --- a/libpam/include/security/_pam_macros.h +++ b/libpam/include/security/_pam_macros.h @@ -44,10 +44,8 @@ do { \ #define _pam_drop(X) \ do { \ - if (X) { \ - free(X); \ - X=NULL; \ - } \ + free(X); \ + X=NULL; \ } while (0) /* @@ -64,8 +62,7 @@ do { \ free(reply[reply_i].resp); \ } \ } \ - if (reply) \ - free(reply); \ + free(reply); \ } while (0) /* some debugging code */ diff --git a/libpam/pam_modutil_cleanup.c b/libpam/pam_modutil_cleanup.c index 8224ce67..2077cbd7 100644 --- a/libpam/pam_modutil_cleanup.c +++ b/libpam/pam_modutil_cleanup.c @@ -12,8 +12,6 @@ void pam_modutil_cleanup (pam_handle_t *pamh UNUSED, void *data, int error_status UNUSED) { - if (data) { /* junk it */ - (void) free(data); - } + free(data); } diff --git a/libpam/pam_modutil_getgrgid.c b/libpam/pam_modutil_getgrgid.c index 93a59633..fd495105 100644 --- a/libpam/pam_modutil_getgrgid.c +++ b/libpam/pam_modutil_getgrgid.c @@ -54,9 +54,7 @@ pam_modutil_getgrgid(pam_handle_t *pamh, gid_t gid) D(("out of memory")); /* no memory for the user - so delete the memory */ - if (buffer) { - free(buffer); - } + free(buffer); return NULL; } buffer = new_buffer; diff --git a/libpam/pam_modutil_getgrnam.c b/libpam/pam_modutil_getgrnam.c index f5bc52ee..c7dd175c 100644 --- a/libpam/pam_modutil_getgrnam.c +++ b/libpam/pam_modutil_getgrnam.c @@ -44,9 +44,7 @@ pam_modutil_getgrnam(pam_handle_t *pamh, const char *group) D(("out of memory")); /* no memory for the group - so delete the memory */ - if (buffer) { - free(buffer); - } + free(buffer); return NULL; } buffer = new_buffer; diff --git a/libpam/pam_modutil_getpwnam.c b/libpam/pam_modutil_getpwnam.c index 012c9ec8..9c96150b 100644 --- a/libpam/pam_modutil_getpwnam.c +++ b/libpam/pam_modutil_getpwnam.c @@ -44,9 +44,7 @@ pam_modutil_getpwnam(pam_handle_t *pamh, const char *user) D(("out of memory")); /* no memory for the user - so delete the memory */ - if (buffer) { - free(buffer); - } + free(buffer); return NULL; } buffer = new_buffer; diff --git a/libpam/pam_modutil_getpwuid.c b/libpam/pam_modutil_getpwuid.c index cdd196ee..671fdf23 100644 --- a/libpam/pam_modutil_getpwuid.c +++ b/libpam/pam_modutil_getpwuid.c @@ -54,9 +54,7 @@ pam_modutil_getpwuid(pam_handle_t *pamh, uid_t uid) D(("out of memory")); /* no memory for the user - so delete the memory */ - if (buffer) { - free(buffer); - } + free(buffer); return NULL; } buffer = new_buffer; diff --git a/libpam/pam_modutil_getspnam.c b/libpam/pam_modutil_getspnam.c index 2673668c..8b48db90 100644 --- a/libpam/pam_modutil_getspnam.c +++ b/libpam/pam_modutil_getspnam.c @@ -44,9 +44,7 @@ pam_modutil_getspnam(pam_handle_t *pamh, const char *user) D(("out of memory")); /* no memory for the user - so delete the memory */ - if (buffer) { - free(buffer); - } + free(buffer); return NULL; } buffer = new_buffer; diff --git a/libpamc/pamc_load.c b/libpamc/pamc_load.c index 69fb8b87..f7365990 100644 --- a/libpamc/pamc_load.c +++ b/libpamc/pamc_load.c @@ -393,10 +393,8 @@ static pamc_id_node_t *__pamc_add_node(pamc_id_node_t *root, const char *id, static pamc_id_node_t *__pamc_liberate_nodes(pamc_id_node_t *tree) { if (tree) { - if (tree->agent_id) { - free(tree->agent_id); - tree->agent_id = NULL; - } + free(tree->agent_id); + tree->agent_id = NULL; tree->left = __pamc_liberate_nodes(tree->left); tree->right = __pamc_liberate_nodes(tree->right); diff --git a/libpamc/test/modules/pam_secret.c b/libpamc/test/modules/pam_secret.c index f1c74c6f..c4b32ae9 100644 --- a/libpamc/test/modules/pam_secret.c +++ b/libpamc/test/modules/pam_secret.c @@ -171,9 +171,7 @@ static int converse(pam_handle_t *pamh, struct ps_state_s *new) } } - if (single_reply) { - free(single_reply); - } + free(single_reply); } #ifdef PAM_DEBUG diff --git a/modules/pam_filter/pam_filter.c b/modules/pam_filter/pam_filter.c index a3c3bb24..72c3b480 100644 --- a/modules/pam_filter/pam_filter.c +++ b/modules/pam_filter/pam_filter.c @@ -238,8 +238,7 @@ static void free_evp(char *evp[]) if (evp) for (i=0; i<4; ++i) { - if (evp[i]) - free(evp[i]); + free(evp[i]); } free(evp); } diff --git a/modules/pam_listfile/pam_listfile.c b/modules/pam_listfile/pam_listfile.c index 5561b9ea..a01b5a8a 100644 --- a/modules/pam_listfile/pam_listfile.c +++ b/modules/pam_listfile/pam_listfile.c @@ -92,7 +92,7 @@ pam_sm_authenticate (pam_handle_t *pamh, int flags UNUSED, else if(!strcmp(myval,"fail")) onerr = PAM_SERVICE_ERR; else { - if (ifname) free (ifname); + free(ifname); return PAM_SERVICE_ERR; } else if(!strcmp(mybuf,"sense")) @@ -101,11 +101,11 @@ pam_sm_authenticate (pam_handle_t *pamh, int flags UNUSED, else if(!strcmp(myval,"deny")) sense=1; else { - if (ifname) free (ifname); + free(ifname); return onerr; } else if(!strcmp(mybuf,"file")) { - if (ifname) free (ifname); + free(ifname); ifname = malloc(strlen(myval)+1); if (!ifname) return PAM_BUF_ERR; diff --git a/modules/pam_namespace/argv_parse.c b/modules/pam_namespace/argv_parse.c index 7d418e05..fff93f4c 100644 --- a/modules/pam_namespace/argv_parse.c +++ b/modules/pam_namespace/argv_parse.c @@ -65,7 +65,7 @@ int argv_parse(const char *in_buf, int *ret_argc, char ***ret_argv) new_argv = realloc(argv, (max_argc+1)*sizeof(char *)); if (!new_argv) { - if (argv) free(argv); + free(argv); free(buf); return -1; } @@ -131,8 +131,7 @@ int argv_parse(const char *in_buf, int *ret_argc, char ***ret_argv) void argv_free(char **argv) { if (argv) { - if (*argv) - free(*argv); + free(*argv); free(argv); } } diff --git a/modules/pam_stress/pam_stress.c b/modules/pam_stress/pam_stress.c index b2c55586..d0f54016 100644 --- a/modules/pam_stress/pam_stress.c +++ b/modules/pam_stress/pam_stress.c @@ -476,10 +476,8 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags, resp = NULL; retval = converse(pamh,i,pmsg,&resp); - if (txt) { - free(txt); - txt = NULL; /* clean up */ - } + free(txt); + txt = NULL; /* clean up */ if (retval != PAM_SUCCESS) { return retval; } diff --git a/modules/pam_timestamp/hmac_openssl_wrapper.c b/modules/pam_timestamp/hmac_openssl_wrapper.c index ac434377..a59a8de2 100644 --- a/modules/pam_timestamp/hmac_openssl_wrapper.c +++ b/modules/pam_timestamp/hmac_openssl_wrapper.c @@ -297,9 +297,7 @@ hmac_management(pam_handle_t *pamh, int debug, void **out, size_t *out_length, ret = PAM_SUCCESS; done: - if (hmac_message != NULL) { - free(hmac_message); - } + free(hmac_message); if (key != NULL) { pam_overwrite_n(key, key_length); free(key); diff --git a/modules/pam_unix/pam_unix_auth.c b/modules/pam_unix/pam_unix_auth.c index 4eccff8e..976699d4 100644 --- a/modules/pam_unix/pam_unix_auth.c +++ b/modules/pam_unix/pam_unix_auth.c @@ -86,8 +86,7 @@ do { \ static void setcred_free (pam_handle_t *pamh UNUSED, void *ptr, int err UNUSED) { - if (ptr) - free (ptr); + free (ptr); } int diff --git a/modules/pam_xauth/pam_xauth.c b/modules/pam_xauth/pam_xauth.c index bc8a3d74..d5e99c9f 100644 --- a/modules/pam_xauth/pam_xauth.c +++ b/modules/pam_xauth/pam_xauth.c @@ -197,9 +197,7 @@ run_coprocess(pam_handle_t *pamh, const char *input, char **output, tmp = realloc(buffer, buffer_size + i + 1); if (tmp == NULL) { /* Uh-oh, bail. */ - if (buffer != NULL) { - free(buffer); - } + free(buffer); close(opipe[0]); waitpid(child, NULL, 0); sigaction(SIGCHLD, &oldsa, NULL); /* restore old signal handler */ @@ -545,10 +543,8 @@ pam_sm_open_session (pam_handle_t *pamh, int flags UNUSED, char *t, *screen; size_t tlen, slen; /* Free the useless cookie string. */ - if (cookie != NULL) { - free(cookie); - cookie = NULL; - } + free(cookie); + cookie = NULL; /* Allocate enough space to hold an adjusted name. */ tlen = strlen(display) + LINE_MAX + 1; t = calloc(1, tlen); -- cgit v1.2.3 From fe6287140bc4d37e6ef36ca1387ce1403b6dd742 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Fri, 15 Dec 2023 00:01:09 +0100 Subject: pam_namespace: handle huge namespace.conf lines If a lot of arguments are found in a namespace.conf file, argc might overflow, which is an undefined behavior. In most cases, the realloc will instantly fail due to a wrap around. Protect properly by avoiding the calculation in the first place. Signed-off-by: Tobias Stoeckmann --- modules/pam_namespace/argv_parse.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'modules/pam_namespace/argv_parse.c') diff --git a/modules/pam_namespace/argv_parse.c b/modules/pam_namespace/argv_parse.c index fff93f4c..ac7c9ae0 100644 --- a/modules/pam_namespace/argv_parse.c +++ b/modules/pam_namespace/argv_parse.c @@ -28,6 +28,7 @@ * Version 1.1, modified 2/27/1999 */ +#include #include #include #include @@ -61,6 +62,11 @@ int argv_parse(const char *in_buf, int *ret_argc, char ***ret_argv) /* Not whitespace, so start a new token */ state = STATE_TOKEN; if (argc >= max_argc) { + if (max_argc >= INT_MAX - 3) { + free(argv); + free(buf); + return -1; + } max_argc += 3; new_argv = realloc(argv, (max_argc+1)*sizeof(char *)); -- cgit v1.2.3 From 5d7eefb1883c557c7a027f68e966e2fae294a9b6 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Fri, 30 Aug 2024 08:00:00 +0000 Subject: build: consistently include config.h first Make sure that config.h is included before any system header. --- libpam/pam_prelude.c | 8 ++++---- modules/pam_namespace/argv_parse.c | 2 ++ modules/pam_setquota/pam_setquota.c | 3 ++- modules/pam_timestamp/sha1.c | 2 +- modules/pam_unix/audit.c | 3 +-- modules/pam_unix/bigcrypt_main.c | 2 ++ modules/pam_unix/md5.c | 4 ++-- modules/pam_unix/md5_crypt.c | 2 +- modules/pam_unix/yppasswd.h | 2 ++ 9 files changed, 17 insertions(+), 11 deletions(-) (limited to 'modules/pam_namespace/argv_parse.c') diff --git a/libpam/pam_prelude.c b/libpam/pam_prelude.c index 6c73bf5d..c62e2f2c 100644 --- a/libpam/pam_prelude.c +++ b/libpam/pam_prelude.c @@ -5,17 +5,17 @@ * (C) Sebastien Tricaud 2005 */ -#include -#include - #ifdef PRELUDE +#include "pam_private.h" + +#include +#include #include #include #include #include "pam_prelude.h" -#include "pam_private.h" #define ANALYZER_CLASS "pam" diff --git a/modules/pam_namespace/argv_parse.c b/modules/pam_namespace/argv_parse.c index ac7c9ae0..cbae7831 100644 --- a/modules/pam_namespace/argv_parse.c +++ b/modules/pam_namespace/argv_parse.c @@ -28,6 +28,8 @@ * Version 1.1, modified 2/27/1999 */ +#include "config.h" + #include #include #include diff --git a/modules/pam_setquota/pam_setquota.c b/modules/pam_setquota/pam_setquota.c index c15fc669..73445e29 100644 --- a/modules/pam_setquota/pam_setquota.c +++ b/modules/pam_setquota/pam_setquota.c @@ -8,6 +8,8 @@ Copyright © 2016 Keller Fuchs */ +#include "pam_inline.h" + #include #include #include @@ -22,7 +24,6 @@ #include #include #include -#include "pam_inline.h" #ifndef PATH_LOGIN_DEFS # define PATH_LOGIN_DEFS "/etc/login.defs" diff --git a/modules/pam_timestamp/sha1.c b/modules/pam_timestamp/sha1.c index dff454cf..f21b2870 100644 --- a/modules/pam_timestamp/sha1.c +++ b/modules/pam_timestamp/sha1.c @@ -37,6 +37,7 @@ */ /* See http://www.itl.nist.gov/fipspubs/fip180-1.htm for descriptions. */ +#include "pam_inline.h" #include #include #include @@ -47,7 +48,6 @@ #include #include #include "sha1.h" -#include "pam_inline.h" static const unsigned char padding[SHA1_BLOCK_SIZE] = { diff --git a/modules/pam_unix/audit.c b/modules/pam_unix/audit.c index 1547a652..9513aaa9 100644 --- a/modules/pam_unix/audit.c +++ b/modules/pam_unix/audit.c @@ -1,5 +1,3 @@ -#include "audit.h" - #include "config.h" #ifdef HAVE_LIBAUDIT @@ -11,6 +9,7 @@ #include +#include "audit.h" #include "passverify.h" int audit_log(int type, const char *uname, int retval) diff --git a/modules/pam_unix/bigcrypt_main.c b/modules/pam_unix/bigcrypt_main.c index fab212d9..22d325da 100644 --- a/modules/pam_unix/bigcrypt_main.c +++ b/modules/pam_unix/bigcrypt_main.c @@ -1,3 +1,5 @@ +#include "config.h" + #include #include diff --git a/modules/pam_unix/md5.c b/modules/pam_unix/md5.c index 95b8de4c..78e9af27 100644 --- a/modules/pam_unix/md5.c +++ b/modules/pam_unix/md5.c @@ -18,11 +18,11 @@ * */ +#include "pam_inline.h" + #include #include "md5.h" -#include "pam_inline.h" - #ifndef HIGHFIRST #define byteReverse(buf, len) /* Nothing */ #else diff --git a/modules/pam_unix/md5_crypt.c b/modules/pam_unix/md5_crypt.c index 9a6bd4f9..9451f376 100644 --- a/modules/pam_unix/md5_crypt.c +++ b/modules/pam_unix/md5_crypt.c @@ -12,11 +12,11 @@ * */ +#include "pam_inline.h" #include #include #include #include "md5.h" -#include "pam_inline.h" static const unsigned char itoa64[] = /* 0 ... 63 => ascii - 64 */ "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; diff --git a/modules/pam_unix/yppasswd.h b/modules/pam_unix/yppasswd.h index dc686cd7..3a40c3ea 100644 --- a/modules/pam_unix/yppasswd.h +++ b/modules/pam_unix/yppasswd.h @@ -6,6 +6,8 @@ #ifndef _YPPASSWD_H_RPCGEN #define _YPPASSWD_H_RPCGEN +#include "config.h" + #include -- cgit v1.2.3