[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
some sprintf cleanups for "who"
From: |
Paul Eggert |
Subject: |
some sprintf cleanups for "who" |
Date: |
Sun, 13 Jun 2004 00:32:49 -0700 |
User-agent: |
Gnus/5.1006 (Gnus v5.10.6) Emacs/21.3 (gnu/linux) |
I noticed that who.c's make_id_equals_comment uses sprintf with an
"%.*s" format to copy a value that might not be null-terminated; this
results in undefined behavior. I don't know of any host where it
doesn't work but it's easy to be safe.
I then audited for other sprintf glitches and/or bugs and propose the
following patch for them all. Each hunk is pretty much independent.
2004-06-13 Paul Eggert <address@hidden>
* src/who.c (PIDSTR_DECL_AND_INIT): Don't assume pid_t fits
in int.
(UT_ID) [!HAVE_STRUCT_XTMP_UT_ID]: Remove bogus comment,
as (sizeof "??") reliably returns 3.
(print_line): Guard against idle and pid being too long
(which is possible when printing headers).
(print_user): Allocate enough bytes for idlestr. Use IDLESTR_LEN.
Avoid unnecessary cast of sizeof to int.
(make_id_equals_comment): Do not assume that UT_ID returns
a string; it might return a non-null-terminated array.
Use strncat instead. It's not very often where
strncat is exactly what you want, but this is one of those rare cases.
Index: src/who.c
===================================================================
RCS file: /home/meyering/coreutils/cu/src/who.c,v
retrieving revision 1.93
diff -p -u -r1.93 who.c
--- src/who.c 12 Jun 2004 08:05:59 -0000 1.93
+++ src/who.c 13 Jun 2004 07:14:45 -0000
@@ -82,7 +82,7 @@
# define UT_PID(U) ((U)->ut_pid)
# define PIDSTR_DECL_AND_INIT(Var, Utmp_ent) \
char Var[INT_STRLEN_BOUND (Utmp_ent->ut_pid) + 1]; \
- sprintf (Var, "%d", (int) (Utmp_ent->ut_pid))
+ sprintf (Var, "%ld", (long int) (Utmp_ent->ut_pid))
#else
# define UT_PID(U) 0
# define PIDSTR_DECL_AND_INIT(Var, Utmp_ent) \
@@ -92,8 +92,6 @@
#if HAVE_STRUCT_XTMP_UT_ID
# define UT_ID(U) ((U)->ut_id)
#else
- /* Of course, sizeof "whatever" is the size of a pointer (often 4),
- but that's ok, since the actual string has a length of only 2. */
# define UT_ID(U) "??"
#endif
@@ -257,12 +255,12 @@ print_line (const char *user, const char
mesg[1] = state;
- if (include_idle && !short_output)
+ if (include_idle && !short_output && strlen (idle) < sizeof x_idle - 1)
sprintf (x_idle, " %-6s", idle);
else
*x_idle = '\0';
- if (!short_output)
+ if (!short_output && strlen (pid) < sizeof x_pid - 1)
sprintf (x_pid, " %10s", pid);
else
*x_pid = '\0';
@@ -319,7 +317,7 @@ print_user (const STRUCT_UTMP *utmp_ent)
struct stat stats;
time_t last_change;
char mesg;
- char idlestr[IDLESTR_LEN];
+ char idlestr[IDLESTR_LEN + 1];
static char *hoststr;
static size_t hostlen;
@@ -357,7 +355,7 @@ print_user (const STRUCT_UTMP *utmp_ent)
}
if (last_change)
- sprintf (idlestr, "%.6s", idle_string (last_change));
+ sprintf (idlestr, "%.*s", IDLESTR_LEN, idle_string (last_change));
else
sprintf (idlestr, " ?");
@@ -368,7 +366,7 @@ print_user (const STRUCT_UTMP *utmp_ent)
char *host = 0, *display = 0;
/* Copy the host name into UT_HOST, and ensure it's nul terminated. */
- strncpy (ut_host, utmp_ent->ut_host, (int) sizeof (utmp_ent->ut_host));
+ strncpy (ut_host, utmp_ent->ut_host, sizeof (utmp_ent->ut_host));
ut_host[sizeof (utmp_ent->ut_host)] = '\0';
/* Look for an X display. */
@@ -431,9 +429,8 @@ make_id_equals_comment (STRUCT_UTMP cons
{
char *comment = xmalloc (strlen (_("id=")) + sizeof UT_ID (utmp_ent) + 1);
- /* Cast field width argument to `int' to avoid warning from gcc. */
- sprintf (comment, "%s%.*s", _("id="), (int) sizeof UT_ID (utmp_ent),
- UT_ID (utmp_ent));
+ strcpy (comment, _("id="));
+ strncat (comment, UT_ID (utmp_ent), sizeof UT_ID (utmp_ent));
return comment;
}
- some sprintf cleanups for "who",
Paul Eggert <=