From: Alejandro Colomar Date: Sat, 26 Aug 2023 09:53:25 +0000 (+0200) Subject: lib/, src/: snprintf(3) already terminates strings with NUL X-Git-Tag: 4.15.0-rc1~87 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9858133cc68365a7a26a847258a353df6dc40f38;p=thirdparty%2Fshadow.git lib/, src/: snprintf(3) already terminates strings with NUL We don't need to terminate them manually after the call. Remove all that paranoid code, which in some cases was even wrong. While at it, let's do a few more things: - Use sizeof(buf) for the size of the buffer. I found that a few cases were passing one less byte (probably because the last one was manually zeroed later). This caused a double NUL. snprintf(3) wants the size of the entire buffer to properly terminate it. Passing the exact value hardcoded is brittle, so use sizeof(). - Align and improve style of variable declarations. This makes them appear in this diff, which will help review the patch. Signed-off-by: Alejandro Colomar --- diff --git a/lib/shell.c b/lib/shell.c index b0d7e87d0..b6f00f471 100644 --- a/lib/shell.c +++ b/lib/shell.c @@ -30,8 +30,8 @@ extern size_t newenvc; int shell (const char *file, /*@null@*/const char *arg, char *const envp[]) { - char arg0[1024]; - int err; + int err; + char arg0[1024]; if (file == NULL) { errno = EINVAL; @@ -45,8 +45,7 @@ int shell (const char *file, /*@null@*/const char *arg, char *const envp[]) * don't want to tell us what it is themselves. */ if (arg == NULL) { - (void) snprintf (arg0, sizeof arg0, "-%s", Basename (file)); - arg0[sizeof arg0 - 1] = '\0'; + snprintf(arg0, sizeof(arg0), "-%s", Basename(file)); arg = arg0; } diff --git a/lib/user_busy.c b/lib/user_busy.c index eef645636..43d619845 100644 --- a/lib/user_busy.c +++ b/lib/user_busy.c @@ -151,16 +151,16 @@ static int check_status (const char *name, const char *sname, uid_t uid) static int user_busy_processes (const char *name, uid_t uid) { - DIR *proc; - struct dirent *ent; - char *tmp_d_name; - pid_t pid; - DIR *task_dir; + DIR *proc; + DIR *task_dir; + char *tmp_d_name; /* 22: /proc/xxxxxxxxxx/task + \0 */ - char task_path[22]; - char root_path[22]; - struct stat sbroot; - struct stat sbroot_process; + char task_path[22]; + char root_path[22]; + pid_t pid; + struct stat sbroot; + struct stat sbroot_process; + struct dirent *ent; #ifdef ENABLE_SUBIDS sub_uid_open (O_RDONLY); @@ -205,8 +205,7 @@ static int user_busy_processes (const char *name, uid_t uid) } /* Check if the process is in our chroot */ - snprintf (root_path, 22, "/proc/%lu/root", (unsigned long) pid); - root_path[21] = '\0'; + snprintf(root_path, sizeof(root_path), "/proc/%lu/root", (unsigned long) pid); if (stat (root_path, &sbroot_process) != 0) { continue; } @@ -226,8 +225,7 @@ static int user_busy_processes (const char *name, uid_t uid) return 1; } - snprintf (task_path, 22, "/proc/%lu/task", (unsigned long) pid); - task_path[21] = '\0'; + snprintf(task_path, sizeof(task_path), "/proc/%lu/task", (unsigned long) pid); task_dir = opendir (task_path); if (task_dir != NULL) { while ((ent = readdir (task_dir)) != NULL) { diff --git a/src/gpasswd.c b/src/gpasswd.c index 3936b85e6..390b04f92 100644 --- a/src/gpasswd.c +++ b/src/gpasswd.c @@ -383,17 +383,16 @@ static void open_files (void) static void log_gpasswd_failure (const char *suffix) { #ifdef WITH_AUDIT - char buf[1024]; + char buf[1024]; #endif if (aflg) { SYSLOG ((LOG_ERR, "%s failed to add user %s to group %s%s", myname, user, group, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "%s failed to add user %s to group %s%s", - myname, user, group, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "%s failed to add user %s to group %s%s", + myname, user, group, suffix); audit_logger (AUDIT_USER_ACCT, Prog, buf, group, AUDIT_NO_ID, @@ -404,10 +403,9 @@ static void log_gpasswd_failure (const char *suffix) "%s failed to remove user %s from group %s%s", myname, user, group, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "%s failed to remove user %s from group %s%s", - myname, user, group, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "%s failed to remove user %s from group %s%s", + myname, user, group, suffix); audit_logger (AUDIT_USER_ACCT, Prog, buf, group, AUDIT_NO_ID, @@ -418,10 +416,9 @@ static void log_gpasswd_failure (const char *suffix) "%s failed to remove password of group %s%s", myname, group, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "%s failed to remove password of group %s%s", - myname, group, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "%s failed to remove password of group %s%s", + myname, group, suffix); audit_logger (AUDIT_USER_CHAUTHTOK, Prog, buf, group, AUDIT_NO_ID, @@ -432,10 +429,9 @@ static void log_gpasswd_failure (const char *suffix) "%s failed to restrict access to group %s%s", myname, group, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "%s failed to restrict access to group %s%s", - myname, group, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "%s failed to restrict access to group %s%s", + myname, group, suffix); audit_logger (AUDIT_USER_CHAUTHTOK, Prog, buf, group, AUDIT_NO_ID, @@ -448,10 +444,9 @@ static void log_gpasswd_failure (const char *suffix) "%s failed to set the administrators of group %s to %s%s", myname, group, admins, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "%s failed to set the administrators of group %s to %s%s", - myname, group, admins, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "%s failed to set the administrators of group %s to %s%s", + myname, group, admins, suffix); audit_logger (AUDIT_USER_ACCT, Prog, buf, group, AUDIT_NO_ID, @@ -464,10 +459,9 @@ static void log_gpasswd_failure (const char *suffix) "%s failed to set the members of group %s to %s%s", myname, group, members, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "%s failed to set the members of group %s to %s%s", - myname, group, members, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "%s failed to set the members of group %s to %s%s", + myname, group, members, suffix); audit_logger (AUDIT_USER_ACCT, Prog, buf, group, AUDIT_NO_ID, @@ -479,10 +473,9 @@ static void log_gpasswd_failure (const char *suffix) "%s failed to change password of group %s%s", myname, group, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "%s failed to change password of group %s%s", - myname, group, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "%s failed to change password of group %s%s", + myname, group, suffix); audit_logger (AUDIT_USER_CHAUTHTOK, Prog, buf, group, AUDIT_NO_ID, @@ -498,18 +491,18 @@ static void log_gpasswd_failure_system (unused void *arg) static void log_gpasswd_failure_group (unused void *arg) { - char buf[1024]; - snprintf (buf, 1023, " in %s", gr_dbname ()); - buf[1023] = '\0'; + char buf[1024]; + + snprintf(buf, sizeof(buf), " in %s", gr_dbname()); log_gpasswd_failure (buf); } #ifdef SHADOWGRP static void log_gpasswd_failure_gshadow (unused void *arg) { - char buf[1024]; - snprintf (buf, 1023, " in %s", sgr_dbname ()); - buf[1023] = '\0'; + char buf[1024]; + + snprintf(buf, sizeof(buf), " in %s", sgr_dbname()); log_gpasswd_failure (buf); } #endif /* SHADOWGRP */ @@ -517,17 +510,16 @@ static void log_gpasswd_failure_gshadow (unused void *arg) static void log_gpasswd_success (const char *suffix) { #ifdef WITH_AUDIT - char buf[1024]; + char buf[1024]; #endif if (aflg) { SYSLOG ((LOG_INFO, "user %s added by %s to group %s%s", user, myname, group, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "user %s added by %s to group %s%s", - user, myname, group, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "user %s added by %s to group %s%s", + user, myname, group, suffix); audit_logger (AUDIT_USER_ACCT, Prog, buf, group, AUDIT_NO_ID, @@ -538,10 +530,9 @@ static void log_gpasswd_success (const char *suffix) "user %s removed by %s from group %s%s", user, myname, group, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "user %s removed by %s from group %s%s", - user, myname, group, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "user %s removed by %s from group %s%s", + user, myname, group, suffix); audit_logger (AUDIT_USER_ACCT, Prog, buf, group, AUDIT_NO_ID, @@ -552,10 +543,9 @@ static void log_gpasswd_success (const char *suffix) "password of group %s removed by %s%s", group, myname, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "password of group %s removed by %s%s", - group, myname, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "password of group %s removed by %s%s", + group, myname, suffix); audit_logger (AUDIT_USER_CHAUTHTOK, Prog, buf, group, AUDIT_NO_ID, @@ -566,10 +556,9 @@ static void log_gpasswd_success (const char *suffix) "access to group %s restricted by %s%s", group, myname, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "access to group %s restricted by %s%s", - group, myname, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "access to group %s restricted by %s%s", + group, myname, suffix); audit_logger (AUDIT_USER_CHAUTHTOK, Prog, buf, group, AUDIT_NO_ID, @@ -582,10 +571,9 @@ static void log_gpasswd_success (const char *suffix) "administrators of group %s set by %s to %s%s", group, myname, admins, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "administrators of group %s set by %s to %s%s", - group, myname, admins, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "administrators of group %s set by %s to %s%s", + group, myname, admins, suffix); audit_logger (AUDIT_USER_ACCT, Prog, buf, group, AUDIT_NO_ID, @@ -598,10 +586,9 @@ static void log_gpasswd_success (const char *suffix) "members of group %s set by %s to %s%s", group, myname, members, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "members of group %s set by %s to %s%s", - group, myname, members, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "members of group %s set by %s to %s%s", + group, myname, members, suffix); audit_logger (AUDIT_USER_ACCT, Prog, buf, group, AUDIT_NO_ID, @@ -613,10 +600,9 @@ static void log_gpasswd_success (const char *suffix) "password of group %s changed by %s%s", group, myname, suffix)); #ifdef WITH_AUDIT - snprintf (buf, 1023, - "password of group %s changed by %s%s", - group, myname, suffix); - buf[1023] = '\0'; + snprintf(buf, sizeof(buf), + "password of group %s changed by %s%s", + group, myname, suffix); audit_logger (AUDIT_USER_CHAUTHTOK, Prog, buf, group, AUDIT_NO_ID, @@ -632,9 +618,9 @@ static void log_gpasswd_success_system (unused void *arg) static void log_gpasswd_success_group (unused void *arg) { - char buf[1024]; - snprintf (buf, 1023, " in %s", gr_dbname ()); - buf[1023] = '\0'; + char buf[1024]; + + snprintf(buf, sizeof(buf), " in %s", gr_dbname()); log_gpasswd_success (buf); }