| Message ID | 20210511171753.360387-1-siddhesh@sourceware.org |
|---|---|
| State | New |
| Headers | show |
| Series | login: Limit strncpy to one less than buffer size | expand |
* Siddhesh Poyarekar via Libc-alpha: > Avoid posibility of ut_line being an unterminated string. > --- > login/login.c | 2 +- > login/logout.c | 2 +- > login/logwtmp.c | 6 +++--- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/login/login.c b/login/login.c > index d280c13f1f..0822d36753 100644 > --- a/login/login.c > +++ b/login/login.c > @@ -111,7 +111,7 @@ login (const struct utmp *ut) > ttyp = basename (tty); > > /* Position to record for this tty. */ > - strncpy (copy.ut_line, ttyp, UT_LINESIZE); > + strncpy (copy.ut_line, ttyp, sizeof (copy.ut_line) - 1); ut_line is annotated with __attribute_nonstring__, so an untermined string is expected there. If this came out of a static analysis tool, the tool needs to be taught about the attribute. 8-) Thanks, Florian
On Mai 11 2021, Siddhesh Poyarekar via Libc-alpha wrote:
> Avoid posibility of ut_line being an unterminated string.
But they _are_.
char ut_line[__UT_LINESIZE]
__attribute_nonstring__; /* Devicename. */
char ut_id[4]
__attribute_nonstring__; /* Inittab ID. */
char ut_user[__UT_NAMESIZE]
__attribute_nonstring__; /* Username. */
char ut_host[__UT_HOSTSIZE]
__attribute_nonstring__; /* Hostname for remote login. */
Andreas.
On 5/11/21 10:52 PM, Florian Weimer via Libc-alpha wrote: > * Siddhesh Poyarekar via Libc-alpha: > >> Avoid posibility of ut_line being an unterminated string. >> --- >> login/login.c | 2 +- >> login/logout.c | 2 +- >> login/logwtmp.c | 6 +++--- >> 3 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/login/login.c b/login/login.c >> index d280c13f1f..0822d36753 100644 >> --- a/login/login.c >> +++ b/login/login.c >> @@ -111,7 +111,7 @@ login (const struct utmp *ut) >> ttyp = basename (tty); >> >> /* Position to record for this tty. */ >> - strncpy (copy.ut_line, ttyp, UT_LINESIZE); >> + strncpy (copy.ut_line, ttyp, sizeof (copy.ut_line) - 1); > > ut_line is annotated with __attribute_nonstring__, so an untermined > string is expected there. If this came out of a static analysis tool, > the tool needs to be taught about the attribute. 8-) Uff, yes indeed; I did the lazy thing of just reading the grep result from the header and not actually reading the full definition :/ I'll drop this patch. Siddhesh
diff --git a/login/login.c b/login/login.c index d280c13f1f..0822d36753 100644 --- a/login/login.c +++ b/login/login.c @@ -111,7 +111,7 @@ login (const struct utmp *ut) ttyp = basename (tty); /* Position to record for this tty. */ - strncpy (copy.ut_line, ttyp, UT_LINESIZE); + strncpy (copy.ut_line, ttyp, sizeof (copy.ut_line) - 1); /* Tell that we want to use the UTMP file. */ if (utmpname (_PATH_UTMP) == 0) diff --git a/login/logout.c b/login/logout.c index 3def97fc83..8978bd32bf 100644 --- a/login/logout.c +++ b/login/logout.c @@ -38,7 +38,7 @@ logout (const char *line) /* Fill in search information. */ tmp.ut_type = USER_PROCESS; - strncpy (tmp.ut_line, line, sizeof tmp.ut_line); + strncpy (tmp.ut_line, line, sizeof (tmp.ut_line) - 1); /* Read the record. */ if (getutline_r (&tmp, &utbuf, &ut) >= 0) diff --git a/login/logwtmp.c b/login/logwtmp.c index 1a7c46e9c0..bb8f13852d 100644 --- a/login/logwtmp.c +++ b/login/logwtmp.c @@ -33,9 +33,9 @@ logwtmp (const char *line, const char *name, const char *host) memset (&ut, 0, sizeof (ut)); ut.ut_pid = getpid (); ut.ut_type = name[0] ? USER_PROCESS : DEAD_PROCESS; - strncpy (ut.ut_line, line, sizeof ut.ut_line); - strncpy (ut.ut_name, name, sizeof ut.ut_name); - strncpy (ut.ut_host, host, sizeof ut.ut_host); + strncpy (ut.ut_line, line, sizeof (ut.ut_line) - 1); + strncpy (ut.ut_name, name, sizeof (ut.ut_name) - 1); + strncpy (ut.ut_host, host, sizeof (ut.ut_host) - 1); struct __timespec64 ts; __clock_gettime64 (CLOCK_REALTIME, &ts);