From a55ef44cb6e234ba935dff347cdfa6a9288573fa Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Mon, 12 Aug 2024 11:34:20 +0200 Subject: [PATCH 1/8] lib/, src/, tests/: Use countof() or sizeof_a() where appropriate sizeof() is dangerous with arrays. We usually want to know the length of the array, with countof(), and in very few cases, we want to know the size (in bytes) of the array, with sizeof_a(). Use sizeof() only with variables that are not arrays. Signed-off-by: Alejandro Colomar --- lib/commonio.c | 3 ++- lib/copydir.c | 3 ++- lib/env.c | 3 ++- lib/idmapping.c | 2 +- lib/loginprompt.c | 3 ++- lib/pwauth.c | 5 +++-- lib/shadow/passwd/sgetpwent.c | 1 + lib/subordinateio.c | 3 ++- lib/utmp.c | 2 +- src/chage.c | 13 +++++++------ src/chfn.c | 10 +++++----- src/chsh.c | 3 ++- src/login.c | 3 ++- src/login_nopam.c | 2 +- src/newgidmap.c | 3 ++- tests/unit/test_strncpy.c | 15 ++++++++------- 16 files changed, 43 insertions(+), 31 deletions(-) diff --git a/lib/commonio.c b/lib/commonio.c index b203883183..40e7e9a1e0 100644 --- a/lib/commonio.c +++ b/lib/commonio.c @@ -31,6 +31,7 @@ #endif /* WITH_TCB */ #include "prototypes.h" #include "shadowlog.h" +#include "sizeof.h" #include "sssd.h" #include "string/memset/memzero.h" #include "string/sprintf/aprintf.h" @@ -154,7 +155,7 @@ static int do_lock_file (const char *file, const char *lock, bool log) errno = EINVAL; return 0; } - len = read(fd, buf, sizeof(buf) - 1); + len = read(fd, buf, sizeof_a(buf) - 1); close (fd); if (len <= 0) { if (log) { diff --git a/lib/copydir.c b/lib/copydir.c index 3976bf8e34..187158daba 100644 --- a/lib/copydir.c +++ b/lib/copydir.c @@ -30,6 +30,7 @@ #include #endif /* WITH_ACL */ #include "shadowlog.h" +#include "sizeof.h" #include "string/sprintf/aprintf.h" #include "string/strcmp/streq.h" #include "string/strcmp/strprefix.h" @@ -679,7 +680,7 @@ static int copy_file (const struct path_info *src, const struct path_info *dst, char buf[8192]; ssize_t cnt; - cnt = read(ifd, buf, sizeof(buf)); + cnt = read(ifd, buf, sizeof_a(buf)); if (cnt < 0) { if (errno == EINTR) { continue; diff --git a/lib/env.c b/lib/env.c index 37edc05907..5ce52c229a 100644 --- a/lib/env.c +++ b/lib/env.c @@ -18,6 +18,7 @@ #include "prototypes.h" #include "defines.h" #include "shadowlog.h" +#include "sizeof.h" #include "string/sprintf/aprintf.h" #include "string/sprintf/stprintf.h" #include "string/strcmp/strprefix.h" @@ -163,7 +164,7 @@ void set_env (int argc, char *const *argv) char *cp; for (; argc > 0; argc--, argv++) { - if (strlen(*argv) >= sizeof(variable)) { + if (strlen(*argv) >= countof(variable)) { continue; /* ignore long entries */ } diff --git a/lib/idmapping.c b/lib/idmapping.c index 77631ec18a..fdc7ce80bf 100644 --- a/lib/idmapping.c +++ b/lib/idmapping.c @@ -161,7 +161,7 @@ void write_mapping(int proc_dir_fd, int ranges, const struct map_range *mappings } /* Lockdown new{g,u}idmap by dropping all unneeded capabilities. */ - bzero(data, sizeof(data)); + bzero(data, sizeof_a(data)); data[0].effective = CAP_TO_MASK(cap); /* * When uid 0 from the ancestor userns is supposed to be mapped into diff --git a/lib/loginprompt.c b/lib/loginprompt.c index 9eeae3ddb0..4c5d9b5552 100644 --- a/lib/loginprompt.c +++ b/lib/loginprompt.c @@ -18,6 +18,7 @@ #include "getdef.h" #include "io/fgets/fgets.h" #include "prototypes.h" +#include "sizeof.h" #include "string/memset/memzero.h" #include "string/strcpy/strtcpy.h" #include "string/strspn/stpspn.h" @@ -73,7 +74,7 @@ login_prompt(char *name, int namesize) (void) fclose (fp); } } - (void) gethostname(buf, sizeof(buf)); + (void) gethostname(buf, countof(buf)); printf (_("\n%s login: "), buf); (void) fflush (stdout); diff --git a/lib/pwauth.c b/lib/pwauth.c index d8f61e3ea0..813ff29efc 100644 --- a/lib/pwauth.c +++ b/lib/pwauth.c @@ -21,9 +21,10 @@ #include "agetpass.h" #include "defines.h" +#include "getdef.h" #include "prototypes.h" #include "pwauth.h" -#include "getdef.h" +#include "sizeof.h" #include "string/memset/memzero.h" #include "string/sprintf/stprintf.h" #include "string/strcmp/streq.h" @@ -89,7 +90,7 @@ pw_auth(const char *cipher, const char *user) * Some BSD updates to the S/KEY API adds a fourth parameter; the * sizeof of the challenge info buffer. */ -# define skeychallenge(s,u,c) skeychallenge(s,u,c,sizeof(c)) +# define skeychallenge(s,u,c) skeychallenge(s,u,c,countof(c)) # endif if (skeychallenge (&skey, user, challenge_info) == 0) { diff --git a/lib/shadow/passwd/sgetpwent.c b/lib/shadow/passwd/sgetpwent.c index 531b4aac1a..327113ba8a 100644 --- a/lib/shadow/passwd/sgetpwent.c +++ b/lib/shadow/passwd/sgetpwent.c @@ -19,6 +19,7 @@ #include "atoi/getnum.h" #include "defines.h" #include "prototypes.h" +#include "sizeof.h" #include "string/strcmp/streq.h" #include "string/strtok/stpsep.h" #include "string/strtok/strsep2arr.h" diff --git a/lib/subordinateio.c b/lib/subordinateio.c index 357ec54621..b55a85bc70 100644 --- a/lib/subordinateio.c +++ b/lib/subordinateio.c @@ -24,6 +24,7 @@ #include "atoi/a2i.h" #include "atoi/getnum.h" #include "shadow/passwd/getpw.h" +#include "sizeof.h" #include "string/ctype/strisascii/strisdigit.h" #include "string/sprintf/stprintf.h" #include "string/strcmp/streq.h" @@ -94,7 +95,7 @@ subordinate_parse(const char *line) * Copy the string to a temporary buffer so the substrings can * be modified to be NULL terminated. */ - if (strlen(line) >= sizeof(rangebuf)) + if (strlen(line) >= countof(rangebuf)) return NULL; /* fail if too long */ strcpy (rangebuf, line); diff --git a/lib/utmp.c b/lib/utmp.c index dc9870a7cf..92da03716c 100644 --- a/lib/utmp.c +++ b/lib/utmp.c @@ -301,7 +301,7 @@ prepare_utmp(const char *name, const char *line, const char *host, #endif #if defined(HAVE_STRUCT_UTMPX_UT_SYSLEN) utent->ut_syslen = MIN(strlen(hostname), - sizeof(utent->ut_host)); + countof(utent->ut_host)); #endif #if defined(HAVE_STRUCT_UTMPX_UT_ADDR) || defined(HAVE_STRUCT_UTMPX_UT_ADDR_V6) if (getaddrinfo (hostname, NULL, NULL, &info) == 0) { diff --git a/src/chage.c b/src/chage.c index ae739ee694..484c082371 100644 --- a/src/chage.c +++ b/src/chage.c @@ -28,6 +28,7 @@ #include "pwio.h" #include "shadowio.h" #include "shadowlog.h" +#include "sizeof.h" #include "string/memset/memzero.h" #include "string/sprintf/stprintf.h" #include "string/strcmp/streq.h" @@ -171,12 +172,12 @@ static int new_fields (void) (void) puts (""); stprintf_a(buf, "%ld", mindays); - change_field(buf, sizeof(buf), _("Minimum Password Age")); + change_field(buf, countof(buf), _("Minimum Password Age")); if (a2sl(&mindays, buf, NULL, 0, -1, LONG_MAX) == -1) return 0; stprintf_a(buf, "%ld", maxdays); - change_field(buf, sizeof(buf), _("Maximum Password Age")); + change_field(buf, countof(buf), _("Maximum Password Age")); if (a2sl(&maxdays, buf, NULL, 0, -1, LONG_MAX) == -1) return 0; @@ -185,7 +186,7 @@ static int new_fields (void) else day_to_str_a(buf, lstchgdate); - change_field(buf, sizeof(buf), _("Last Password Change (YYYY-MM-DD)")); + change_field(buf, countof(buf), _("Last Password Change (YYYY-MM-DD)")); if (streq(buf, "-1")) { lstchgdate = -1; @@ -197,12 +198,12 @@ static int new_fields (void) } stprintf_a(buf, "%ld", warndays); - change_field(buf, sizeof(buf), _("Password Expiration Warning")); + change_field(buf, countof(buf), _("Password Expiration Warning")); if (a2sl(&warndays, buf, NULL, 0, -1, LONG_MAX) == -1) return 0; stprintf_a(buf, "%ld", inactdays); - change_field(buf, sizeof(buf), _("Password Inactive")); + change_field(buf, countof(buf), _("Password Inactive")); if (a2sl(&inactdays, buf, NULL, 0, -1, LONG_MAX) == -1) return 0; @@ -211,7 +212,7 @@ static int new_fields (void) else day_to_str_a(buf, expdate); - change_field(buf, sizeof(buf), + change_field(buf, countof(buf), _("Account Expiration Date (YYYY-MM-DD)")); if (streq(buf, "-1")) { diff --git a/src/chfn.c b/src/chfn.c index 71875253e9..40d44b77a5 100644 --- a/src/chfn.c +++ b/src/chfn.c @@ -180,31 +180,31 @@ static void new_fields (void) puts (_("Enter the new value, or press ENTER for the default")); if (may_change_field ('f')) { - change_field(fullnm, sizeof(fullnm), _("Full Name")); + change_field(fullnm, countof(fullnm), _("Full Name")); } else { printf (_("\t%s: %s\n"), _("Full Name"), fullnm); } if (may_change_field ('r')) { - change_field(roomno, sizeof(roomno), _("Room Number")); + change_field(roomno, countof(roomno), _("Room Number")); } else { printf (_("\t%s: %s\n"), _("Room Number"), roomno); } if (may_change_field ('w')) { - change_field(workph, sizeof(workph), _("Work Phone")); + change_field(workph, countof(workph), _("Work Phone")); } else { printf (_("\t%s: %s\n"), _("Work Phone"), workph); } if (may_change_field ('h')) { - change_field(homeph, sizeof(homeph), _("Home Phone")); + change_field(homeph, countof(homeph), _("Home Phone")); } else { printf (_("\t%s: %s\n"), _("Home Phone"), homeph); } if (amroot) { - change_field(slop, sizeof(slop), _("Other")); + change_field(slop, countof(slop), _("Other")); } } diff --git a/src/chsh.c b/src/chsh.c index fafa9759d6..17e3c66185 100644 --- a/src/chsh.c +++ b/src/chsh.c @@ -31,6 +31,7 @@ #include "pam_defs.h" #endif #include "shadowlog.h" +#include "sizeof.h" #include "sssd.h" #include "string/strcmp/streq.h" #include "string/strcpy/strtcpy.h" @@ -123,7 +124,7 @@ usage (int status) static void new_fields (void) { puts (_("Enter the new value, or press ENTER for the default")); - change_field(loginsh, sizeof(loginsh), _("Login Shell")); + change_field(loginsh, countof(loginsh), _("Login Shell")); } /* diff --git a/src/login.c b/src/login.c index 59e39b2b4d..d78caf4d5e 100644 --- a/src/login.c +++ b/src/login.c @@ -38,6 +38,7 @@ #include "pwauth.h" #include "shadow/gshadow/endsgent.h" #include "shadowlog.h" +#include "sizeof.h" #include "string/memset/memzero.h" #include "string/sprintf/stprintf.h" #include "string/strcmp/streq.h" @@ -647,7 +648,7 @@ int main (int argc, char **argv) unsigned int failcount = 0; /* Make the login prompt look like we want it */ - if (gethostname(hostn, sizeof(hostn)) == 0) { + if (gethostname(hostn, countof(hostn)) == 0) { stprintf_a(loginprompt, _("%s login: "), hostn); } else { strtcpy_a(loginprompt, _("login: ")); diff --git a/src/login_nopam.c b/src/login_nopam.c index 3269e74f74..242f1ffd57 100644 --- a/src/login_nopam.c +++ b/src/login_nopam.c @@ -182,7 +182,7 @@ static char *myhostname (void) static char name[MAXHOSTNAMELEN + 1] = ""; if (streq(name, "")) { - gethostname(name, sizeof(name)); + gethostname(name, countof(name)); stpcpy(&name[MAXHOSTNAMELEN], ""); } return (name); diff --git a/src/newgidmap.c b/src/newgidmap.c index afda66657a..76f99bda14 100644 --- a/src/newgidmap.c +++ b/src/newgidmap.c @@ -19,6 +19,7 @@ #include "idmapping.h" #include "prototypes.h" #include "shadowlog.h" +#include "sizeof.h" #include "string/strcmp/strprefix.h" #include "string/strerrno.h" #include "subordinateio.h" @@ -113,7 +114,7 @@ static void write_setgroups(int proc_dir_fd, bool allow_setgroups) * is write-once, so attempting to write after it's already written to will * fail. */ - if (read(setgroups_fd, policy_buffer, sizeof(policy_buffer)) < 0) { + if (read(setgroups_fd, policy_buffer, sizeof_a(policy_buffer)) < 0) { fprintf(stderr, _("%s: failed to read setgroups: %s\n"), Prog, strerrno()); exit(EXIT_FAILURE); diff --git a/tests/unit/test_strncpy.c b/tests/unit/test_strncpy.c index e20a08d720..fd2485be38 100644 --- a/tests/unit/test_strncpy.c +++ b/tests/unit/test_strncpy.c @@ -6,6 +6,7 @@ #include "config.h" +#include #include #include // Required by @@ -43,11 +44,11 @@ test_strncpy_a_trunc(MAYBE_UNUSED void ** _1) char src1[4] = {'f', 'o', 'o', 'o'}; char res1[3] = {'f', 'o', 'o'}; - assert_true(memcmp(res1, strncpy_a(buf, src1), sizeof(buf)) == 0); + assert_true(memcmp(res1, strncpy_a(buf, src1), countof(buf)) == 0); char src2[5] = "barb"; char res2[3] = {'b', 'a', 'r'}; - assert_true(memcmp(res2, strncpy_a(buf, src2), sizeof(buf)) == 0); + assert_true(memcmp(res2, strncpy_a(buf, src2), countof(buf)) == 0); } @@ -58,11 +59,11 @@ test_strncpy_a_fit(MAYBE_UNUSED void ** _1) char src1[3] = {'b', 'a', 'z'}; char res1[3] = {'b', 'a', 'z'}; - assert_true(memcmp(res1, strncpy_a(buf, src1), sizeof(buf)) == 0); + assert_true(memcmp(res1, strncpy_a(buf, src1), countof(buf)) == 0); char src2[4] = "qwe"; char res2[3] = {'q', 'w', 'e'}; - assert_true(memcmp(res2, strncpy_a(buf, src2), sizeof(buf)) == 0); + assert_true(memcmp(res2, strncpy_a(buf, src2), countof(buf)) == 0); } @@ -73,13 +74,13 @@ test_strncpy_a_pad(MAYBE_UNUSED void ** _1) char src1[3] = "as"; char res1[3] = {'a', 's', 0}; - assert_true(memcmp(res1, strncpy_a(buf, src1), sizeof(buf)) == 0); + assert_true(memcmp(res1, strncpy_a(buf, src1), countof(buf)) == 0); char src2[3] = ""; char res2[3] = {0, 0, 0}; - assert_true(memcmp(res2, strncpy_a(buf, src2), sizeof(buf)) == 0); + assert_true(memcmp(res2, strncpy_a(buf, src2), countof(buf)) == 0); char src3[3] = {'a', 0, 'b'}; char res3[3] = {'a', 0, 0}; - assert_true(memcmp(res3, strncpy_a(buf, src3), sizeof(buf)) == 0); + assert_true(memcmp(res3, strncpy_a(buf, src3), countof(buf)) == 0); } From 8dcccac740d6597114263cc41baab07f77edf154 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Mon, 12 Aug 2024 12:03:24 +0200 Subject: [PATCH 2/8] lib/salt.c: crypt_make_salt(): Use countof() instead of a hardcoded value Signed-off-by: Alejandro Colomar --- lib/salt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/salt.c b/lib/salt.c index 4a66d0132c..1d288736ea 100644 --- a/lib/salt.c +++ b/lib/salt.c @@ -354,7 +354,7 @@ static /*@observer@*/const char *gensalt (size_t salt_size) const char *method; unsigned long rounds = 0; - bzero(result, GENSALT_SETTING_SIZE); + bzero(result, countof(result)); method = meth ?: getdef_str("ENCRYPT_METHOD") ?: "DES"; @@ -393,7 +393,7 @@ static /*@observer@*/const char *gensalt (size_t salt_size) method); salt_len = MAX_SALT_SIZE; rounds = 0; - bzero(result, GENSALT_SETTING_SIZE); + bzero(result, countof(result)); } #if USE_XCRYPT_GENSALT @@ -403,7 +403,7 @@ static /*@observer@*/const char *gensalt (size_t salt_size) */ if (streq(result, "")) { /* Avoid -Wunused-but-set-variable. */ - salt_len = GENSALT_SETTING_SIZE - 1; + salt_len = countof(result) - 1; rounds = 0; memset(result, '.', salt_len); stpcpy(&result[salt_len], ""); From 59b70f98479243e7005ad2ad37ebcfa960009df5 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Mon, 12 Aug 2024 12:46:09 +0200 Subject: [PATCH 3/8] lib/sizeof.h: endof(): Add macro endof() returns a pointer to the end of an array, that is, one after the last element. It's similar to C++11's std::end(). Signed-off-by: Alejandro Colomar --- lib/sizeof.h | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/sizeof.h b/lib/sizeof.h index 1fc38873dd..425b11c47f 100644 --- a/lib/sizeof.h +++ b/lib/sizeof.h @@ -27,6 +27,7 @@ // sizeof_a - sizeof array #define sizeof_a(a) (countof(a) * sizeof((a)[0])) +#define endof(a) (&(a)[countof(a)]) #define STRLEN(s) (countof("" s "") - 1) From e5e223861df75906daa52e02173b4b8fc9e68591 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Mon, 12 Aug 2024 13:27:52 +0200 Subject: [PATCH 4/8] lib/fields.[ch]: change_field_a(): Add macro It calculates the length of the array internally. Rename the variable that stores the length. Signed-off-by: Alejandro Colomar --- lib/fields.c | 4 ++-- lib/fields.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/fields.c b/lib/fields.c index 759dea755f..d059e07ae5 100644 --- a/lib/fields.c +++ b/lib/fields.c @@ -60,10 +60,10 @@ valid_field_(const char *field, const char *illegal) * current value. */ void -change_field(char *buf, size_t maxsize, const char *prompt) +change_field(char *buf, size_t n, const char *prompt) { char *cp; - char newf[MIN(200, maxsize)]; + char newf[MIN(200, n)]; printf ("\t%s [%s]: ", prompt, buf); (void) fflush (stdout); diff --git a/lib/fields.h b/lib/fields.h index 8527737f35..ec8e0c0d63 100644 --- a/lib/fields.h +++ b/lib/fields.h @@ -11,10 +11,11 @@ #define valid_field(field, illegal) valid_field_(field, "" illegal "") +#define change_field_a(buf, prompt) change_field(buf, countof(buf), prompt) int valid_field_(const char *field, const char *illegal); -void change_field(char *buf, size_t maxsize, const char *prompt); +void change_field(char *buf, size_t n, const char *prompt); #endif // include guard From cde6ecb1d6f1ddaf7cd7288536da48bb3231005e Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Mon, 12 Aug 2024 13:34:15 +0200 Subject: [PATCH 5/8] src/: Use change_field_a() instead of its pattern Signed-off-by: Alejandro Colomar --- src/chage.c | 13 ++++++------- src/chfn.c | 13 ++++++------- src/chsh.c | 2 +- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/chage.c b/src/chage.c index 484c082371..04c0eb6c58 100644 --- a/src/chage.c +++ b/src/chage.c @@ -172,12 +172,12 @@ static int new_fields (void) (void) puts (""); stprintf_a(buf, "%ld", mindays); - change_field(buf, countof(buf), _("Minimum Password Age")); + change_field_a(buf, _("Minimum Password Age")); if (a2sl(&mindays, buf, NULL, 0, -1, LONG_MAX) == -1) return 0; stprintf_a(buf, "%ld", maxdays); - change_field(buf, countof(buf), _("Maximum Password Age")); + change_field_a(buf, _("Maximum Password Age")); if (a2sl(&maxdays, buf, NULL, 0, -1, LONG_MAX) == -1) return 0; @@ -186,7 +186,7 @@ static int new_fields (void) else day_to_str_a(buf, lstchgdate); - change_field(buf, countof(buf), _("Last Password Change (YYYY-MM-DD)")); + change_field_a(buf, _("Last Password Change (YYYY-MM-DD)")); if (streq(buf, "-1")) { lstchgdate = -1; @@ -198,12 +198,12 @@ static int new_fields (void) } stprintf_a(buf, "%ld", warndays); - change_field(buf, countof(buf), _("Password Expiration Warning")); + change_field_a(buf, _("Password Expiration Warning")); if (a2sl(&warndays, buf, NULL, 0, -1, LONG_MAX) == -1) return 0; stprintf_a(buf, "%ld", inactdays); - change_field(buf, countof(buf), _("Password Inactive")); + change_field_a(buf, _("Password Inactive")); if (a2sl(&inactdays, buf, NULL, 0, -1, LONG_MAX) == -1) return 0; @@ -212,8 +212,7 @@ static int new_fields (void) else day_to_str_a(buf, expdate); - change_field(buf, countof(buf), - _("Account Expiration Date (YYYY-MM-DD)")); + change_field_a(buf, _("Account Expiration Date (YYYY-MM-DD)")); if (streq(buf, "-1")) { expdate = -1; diff --git a/src/chfn.c b/src/chfn.c index 40d44b77a5..85ce8c61fa 100644 --- a/src/chfn.c +++ b/src/chfn.c @@ -180,32 +180,31 @@ static void new_fields (void) puts (_("Enter the new value, or press ENTER for the default")); if (may_change_field ('f')) { - change_field(fullnm, countof(fullnm), _("Full Name")); + change_field_a(fullnm, _("Full Name")); } else { printf (_("\t%s: %s\n"), _("Full Name"), fullnm); } if (may_change_field ('r')) { - change_field(roomno, countof(roomno), _("Room Number")); + change_field_a(roomno, _("Room Number")); } else { printf (_("\t%s: %s\n"), _("Room Number"), roomno); } if (may_change_field ('w')) { - change_field(workph, countof(workph), _("Work Phone")); + change_field_a(workph, _("Work Phone")); } else { printf (_("\t%s: %s\n"), _("Work Phone"), workph); } if (may_change_field ('h')) { - change_field(homeph, countof(homeph), _("Home Phone")); + change_field_a(homeph, _("Home Phone")); } else { printf (_("\t%s: %s\n"), _("Home Phone"), homeph); } - if (amroot) { - change_field(slop, countof(slop), _("Other")); - } + if (amroot) + change_field_a(slop, _("Other")); } /* diff --git a/src/chsh.c b/src/chsh.c index 17e3c66185..0beada96ca 100644 --- a/src/chsh.c +++ b/src/chsh.c @@ -124,7 +124,7 @@ usage (int status) static void new_fields (void) { puts (_("Enter the new value, or press ENTER for the default")); - change_field(loginsh, countof(loginsh), _("Login Shell")); + change_field_a(loginsh, _("Login Shell")); } /* From e592ba7e6239de2179fa645f68559442f97ae9dd Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Sun, 27 Oct 2024 22:44:41 +0100 Subject: [PATCH 6/8] lib/string/memset/: bzero_a(): Add macro This is a bzero() wrapper for arrays. Signed-off-by: Alejandro Colomar --- lib/Makefile.am | 2 ++ lib/string/README | 3 +++ lib/string/memset/bzero.c | 7 +++++++ lib/string/memset/bzero.h | 20 ++++++++++++++++++++ 4 files changed, 32 insertions(+) create mode 100644 lib/string/memset/bzero.c create mode 100644 lib/string/memset/bzero.h diff --git a/lib/Makefile.am b/lib/Makefile.am index 28ec53ef6b..2a4b64bee4 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -199,6 +199,8 @@ libshadow_la_SOURCES = \ string/ctype/strisascii/strisprint.h \ string/ctype/strtoascii/strtolower.c \ string/ctype/strtoascii/strtolower.h \ + string/memset/bzero.c \ + string/memset/bzero.h \ string/memset/memzero.c \ string/memset/memzero.h \ string/sprintf/aprintf.c \ diff --git a/lib/string/README b/lib/string/README index d36388f694..c3dff12861 100644 --- a/lib/string/README +++ b/lib/string/README @@ -88,6 +88,9 @@ ctype/ - Character classification and conversion functions memset/ - Memory zeroing + bzero_a() + Like bzero(3), but takes an array. + memzero() Synonym of explicit_bzero(3). memzero_a() diff --git a/lib/string/memset/bzero.c b/lib/string/memset/bzero.c new file mode 100644 index 0000000000..a81d7601ed --- /dev/null +++ b/lib/string/memset/bzero.c @@ -0,0 +1,7 @@ +// SPDX-FileCopyrightText: 2024, Alejandro Colomar +// SPDX-License-Identifier: BSD-3-Clause + + +#include "config.h" + +#include "string/memset/bzero.h" diff --git a/lib/string/memset/bzero.h b/lib/string/memset/bzero.h new file mode 100644 index 0000000000..6f1b19c767 --- /dev/null +++ b/lib/string/memset/bzero.h @@ -0,0 +1,20 @@ +// SPDX-FileCopyrightText: 2024, Alejandro Colomar +// SPDX-License-Identifier: BSD-3-Clause + + +#ifndef SHADOW_INCLUDE_LIB_STRING_MEMSET_BZERO_H_ +#define SHADOW_INCLUDE_LIB_STRING_MEMSET_BZERO_H_ + + +#include "config.h" + +#include + +#include "sizeof.h" + + +// bzero_a - byte zero array +#define bzero_a(a) bzero(a, sizeof_a(a)) + + +#endif // include guard From 11f603e92042110b42d1eb42100ec786b31655a4 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Sun, 27 Oct 2024 22:47:38 +0100 Subject: [PATCH 7/8] lib/: Use bzero_a() instead of its pattern Signed-off-by: Alejandro Colomar --- lib/idmapping.c | 3 ++- lib/salt.c | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/idmapping.c b/lib/idmapping.c index fdc7ce80bf..de19e104ae 100644 --- a/lib/idmapping.c +++ b/lib/idmapping.c @@ -27,6 +27,7 @@ #include "prototypes.h" #include "shadowlog.h" #include "sizeof.h" +#include "string/memset/bzero.h" #include "string/sprintf/seprintf.h" #include "string/strcmp/streq.h" #include "string/strerrno.h" @@ -161,7 +162,7 @@ void write_mapping(int proc_dir_fd, int ranges, const struct map_range *mappings } /* Lockdown new{g,u}idmap by dropping all unneeded capabilities. */ - bzero(data, sizeof_a(data)); + bzero_a(data); data[0].effective = CAP_TO_MASK(cap); /* * When uid 0 from the ancestor userns is supposed to be mapped into diff --git a/lib/salt.c b/lib/salt.c index 1d288736ea..8fa8ed2ba6 100644 --- a/lib/salt.c +++ b/lib/salt.c @@ -23,6 +23,7 @@ #include "getdef.h" #include "prototypes.h" #include "shadowlog.h" +#include "string/memset/bzero.h" #include "string/sprintf/stprintf.h" #include "string/strcmp/streq.h" @@ -319,7 +320,7 @@ static /*@observer@*/const char *gensalt (size_t salt_size) { static char salt[MAX_SALT_SIZE + 6]; - bzero(salt, MAX_SALT_SIZE + 6); + bzero_a(salt); assert (salt_size >= MIN_SALT_SIZE && salt_size <= MAX_SALT_SIZE); @@ -354,7 +355,7 @@ static /*@observer@*/const char *gensalt (size_t salt_size) const char *method; unsigned long rounds = 0; - bzero(result, countof(result)); + bzero_a(result); method = meth ?: getdef_str("ENCRYPT_METHOD") ?: "DES"; @@ -393,7 +394,7 @@ static /*@observer@*/const char *gensalt (size_t salt_size) method); salt_len = MAX_SALT_SIZE; rounds = 0; - bzero(result, countof(result)); + bzero_a(result); } #if USE_XCRYPT_GENSALT From 53e44dcaa61d178dc40a8540c2728b03a3c3a825 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 29 Jul 2025 12:24:01 +0200 Subject: [PATCH 8/8] lib/audit_help.c: Use countof() instead of sizeof() This is a count of the number of elements. sizeof() works because sizeof(char)==1, but it's more dangerous, as it blindly accepts other types. Signed-off-by: Alejandro Colomar --- lib/audit_help.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/audit_help.c b/lib/audit_help.c index 987465c389..351f2c407f 100644 --- a/lib/audit_help.c +++ b/lib/audit_help.c @@ -25,8 +25,10 @@ #include "attr.h" #include "prototypes.h" #include "shadowlog.h" +#include "sizeof.h" #include "string/sprintf/stprintf.h" + int audit_fd; void audit_help_open (void) @@ -102,7 +104,7 @@ audit_logger_with_group(int type, const char *op, const char *name, if (audit_fd < 0) return; - len = strnlen(grp, sizeof(enc_group)/2); + len = strnlen(grp, countof(enc_group)/2); if (audit_value_needs_encoding(grp, len)) { stprintf_a(buf, "%s %s=%s", op, grp_type, audit_encode_value(enc_group, grp, len));