Subject: bin/24070: moduser uses fgetln() incorrectly
To: None <gnats-bugs@gnats.NetBSD.org>
From: None <Todd.Miller@courtesan.com>
List: netbsd-bugs
Date: 01/12/2004 19:41:30
>Number: 24070
>Category: bin
>Synopsis: moduser uses fgetln() incorrectly
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: bin-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Mon Jan 12 19:42:00 UTC 2004
>Closed-Date:
>Last-Modified:
>Originator: Todd Miller
>Release: -current
>Organization:
OpenBSD
>Environment:
I don't run NetBSD...
>Description:
moduser() in usr.sbin/user.c uses the string returned by fgetln() as if it is a NUL-terminated string but it is not. For instance, if a passwd file entry were to contain no ':' then strchr() would go past the end of the string. A similar problem exists with the strncmp() when comparing a very long username to a very short (possibly corrupt) passwd entry. Also, there is an unneeded cast of asprintf() to int (len used to be int but no longer is).
>How-To-Repeat:
inspect the code...
>Fix:
Either use fgets() instead of fgetln() or use memchr() instead of strchr() and MIN(colonc, loginc) as the length parameter to strncmp() (and perhaps consider using memcmp() instead). In OpenBSD I just switched to fgets() for consistency with the rest of the code.
>Release-Note:
>Audit-Trail:
>Unformatted: