VesperOS/shadow
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lib/, src/: snprintf(3) already terminates strings with NUL

Browse Source 
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 <alx@kernel.org>
This commit is contained in:
Alejandro Colomar
2023-08-26 11:53:25 +02:00
committed by Iker Pedrosa
parent 93a5c47c2c
commit 9858133cc6
3 changed files with 67 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/shell.c
+3 -4
View File
@@ -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;
}
lib/user_busy.c
+11 -13
View File
@@ -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) {
src/gpasswd.c
+53 -67
View File
@@ -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);
}