login: Limit strncpy to one less than buffer size
Checks
Commit Message
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(-)
Comments
* 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
@@ -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)
@@ -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)
@@ -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);