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: