From fb97da1ce1606f7a2f7c897f5441d1d04020f402 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Wed, 10 Oct 2018 12:22:04 +0200 Subject: [PATCH 1/2] Fix some issues found in Coverity scan. --- lib/commonio.c | 4 +--- lib/spawn.c | 2 +- libmisc/console.c | 5 +++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/commonio.c b/lib/commonio.c index d06b8e7d..52a360cf 100644 --- a/lib/commonio.c +++ b/lib/commonio.c @@ -379,7 +379,7 @@ int commonio_lock_nowait (struct commonio_db *db, bool log) char* lock = NULL; size_t lock_file_len; size_t file_len; - int err; + int err = 0; if (db->locked) { return 1; @@ -388,12 +388,10 @@ int commonio_lock_nowait (struct commonio_db *db, bool log) lock_file_len = strlen(db->filename) + 6; /* sizeof ".lock" */ file = (char*)malloc(file_len); if(file == NULL) { - err = ENOMEM; goto cleanup_ENOMEM; } lock = (char*)malloc(lock_file_len); if(lock == NULL) { - err = ENOMEM; goto cleanup_ENOMEM; } snprintf (file, file_len, "%s.%lu", diff --git a/lib/spawn.c b/lib/spawn.c index da984019..fc4ad95c 100644 --- a/lib/spawn.c +++ b/lib/spawn.c @@ -69,7 +69,7 @@ int run_command (const char *cmd, const char *argv[], do { wpid = waitpid (pid, status, 0); } while ( ((pid_t)-1 == wpid && errno == EINTR) - || (wpid != pid)); + || ((pid_t)-1 != wpid && wpid != pid)); if ((pid_t)-1 == wpid) { fprintf (stderr, "%s: waitpid (status: %d): %s\n", diff --git a/libmisc/console.c b/libmisc/console.c index 70d13903..cb74c2f9 100644 --- a/libmisc/console.c +++ b/libmisc/console.c @@ -50,7 +50,7 @@ static bool is_listed (const char *cfgin, const char *tty, bool def); static bool is_listed (const char *cfgin, const char *tty, bool def) { FILE *fp; - char buf[200], *s; + char buf[1024], *s; const char *cons; /* @@ -70,7 +70,8 @@ static bool is_listed (const char *cfgin, const char *tty, bool def) if (*cons != '/') { char *pbuf; - strcpy (buf, cons); + strncpy (buf, cons, sizeof (buf)); + buf[sizeof (buf) - 1] = '\0'; pbuf = &buf[0]; while ((s = strtok (pbuf, ":")) != NULL) { if (strcmp (s, tty) == 0) { From 10e388efc2c786d1ec4ed007891bfefa8826b6fd Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Fri, 12 Oct 2018 10:14:02 +0200 Subject: [PATCH 2/2] useradd: fix segfault trying to overwrite const data with mkstemp Also fix memory leaks in error paths. --- src/useradd.c | 58 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/src/useradd.c b/src/useradd.c index ca90f076..85fe0ddf 100644 --- a/src/useradd.c +++ b/src/useradd.c @@ -343,7 +343,7 @@ static void fail_exit (int code) static void get_defaults (void) { FILE *fp; - char* default_file = USER_DEFAULTS_FILE; + char *default_file = USER_DEFAULTS_FILE; char buf[1024]; char *cp; @@ -353,6 +353,8 @@ static void get_defaults (void) len = strlen(prefix) + strlen(USER_DEFAULTS_FILE) + 2; default_file = malloc(len); + if (default_file == NULL) + return; wlen = snprintf(default_file, len, "%s/%s", prefix, USER_DEFAULTS_FILE); assert (wlen == (int) len -1); } @@ -363,7 +365,7 @@ static void get_defaults (void) fp = fopen (default_file, "r"); if (NULL == fp) { - return; + goto getdef_err; } /* @@ -474,7 +476,7 @@ static void get_defaults (void) } } (void) fclose (fp); - + getdef_err: if(prefix[0]) { free(default_file); } @@ -509,8 +511,8 @@ static int set_defaults (void) FILE *ifp; FILE *ofp; char buf[1024]; - char* new_file = NEW_USER_FILE; - char* default_file = USER_DEFAULTS_FILE; + char *new_file = NULL; + char *default_file = USER_DEFAULTS_FILE; char *cp; int ofd; int wlen; @@ -521,17 +523,30 @@ static int set_defaults (void) bool out_shell = false; bool out_skel = false; bool out_create_mail_spool = false; + size_t len; + int ret = -1; + + + len = strlen(prefix) + strlen(NEW_USER_FILE) + 2; + new_file = malloc(len); + if (new_file == NULL) { + fprintf (stderr, + _("%s: cannot create new defaults file: %s\n"), + Prog, strerror(errno)); + return -1; + } + wlen = snprintf(new_file, len, "%s%s%s", prefix, prefix[0]?"/":"", NEW_USER_FILE); + assert (wlen <= (int) len -1); if(prefix[0]) { - size_t len; - - len = strlen(prefix) + strlen(NEW_USER_FILE) + 2; - new_file = malloc(len); - wlen = snprintf(new_file, len, "%s/%s", prefix, NEW_USER_FILE); - assert (wlen == (int) len -1); - len = strlen(prefix) + strlen(USER_DEFAULTS_FILE) + 2; default_file = malloc(len); + if (default_file == NULL) { + fprintf (stderr, + _("%s: cannot create new defaults file: %s\n"), + Prog, strerror(errno)); + goto setdef_err; + } wlen = snprintf(default_file, len, "%s/%s", prefix, USER_DEFAULTS_FILE); assert (wlen == (int) len -1); } @@ -544,7 +559,7 @@ static int set_defaults (void) fprintf (stderr, _("%s: cannot create new defaults file\n"), Prog); - return -1; + goto setdef_err; } ofp = fdopen (ofd, "w"); @@ -552,7 +567,7 @@ static int set_defaults (void) fprintf (stderr, _("%s: cannot open new defaults file\n"), Prog); - return -1; + goto setdef_err; } /* @@ -579,7 +594,7 @@ static int set_defaults (void) _("%s: line too long in %s: %s..."), Prog, default_file, buf); (void) fclose (ifp); - return -1; + goto setdef_err; } } @@ -643,7 +658,7 @@ static int set_defaults (void) || (fsync (fileno (ofp)) != 0) || (fclose (ofp) != 0)) { unlink (new_file); - return -1; + goto setdef_err; } /* @@ -658,7 +673,7 @@ static int set_defaults (void) _("%s: Cannot create backup file (%s): %s\n"), Prog, buf, strerror (err)); unlink (new_file); - return -1; + goto setdef_err; } /* @@ -669,7 +684,7 @@ static int set_defaults (void) fprintf (stderr, _("%s: rename: %s: %s\n"), Prog, new_file, strerror (err)); - return -1; + goto setdef_err; } #ifdef WITH_AUDIT audit_logger (AUDIT_USYS_CONFIG, Prog, @@ -683,13 +698,14 @@ static int set_defaults (void) (unsigned int) def_group, def_home, def_shell, def_inactive, def_expire, def_template, def_create_mail_spool)); - + ret = 0; + setdef_err: + free(new_file); if(prefix[0]) { - free(new_file); free(default_file); } - return 0; + return ret; } /*