Subject: bin/24070: moduser uses fgetln() incorrectly
To: None <>
From: None <>
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
>Originator:     Todd Miller
>Release:        -current
I don't run NetBSD...
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).
inspect the code...
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.