NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

lib/46279: getpwent-routines failes to extract password from adjunct NIS map

>Number:         46279
>Category:       lib
>Synopsis:       getpwent-routines failes to extract password from adjunct NIS 
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Mar 29 09:35:00 +0000 2012
>Originator:     Dr. W. Stukenbrock
>Release:        NetBSD 6.0-beta
Dr. Nagler & Company GmbH
System: NetBSD s012 4.0 NetBSD 4.0 (NSW-S012) #11: Fri Mar 26 15:01:49 CET 2010 
root@s012:/usr/src/sys/arch/amd64/compile/NSW-S012 amd64
Architecture: x86_64
Machine: amd64
        The nis-parse routine _nis_parse() in src/lib/libc/gen/getpwent.c 
extracts the
        password from the passwd.adjunct map, if the YP-server does not support 
a master.passwd
        but an adjunct map - as Solaris systems may do.
        Accedently the extracted password is copied to a wrong location in the 
buffer, so
        the termnating '\0' of the shell from the main entry gets overwritten. 
This ends up in a
        corrupted shell in the passwd entry returned.
        The file getpwent.c has not changed for 2 years in the CVS-repository, 
any supported netbsd
        version (4.0, .... 6.0-beta) is affected.
        Setup a YP-server with an passwd.adjunct map and try to authenticate 
against it.
        getpwnam() will return a corrupted shell entry as described above.
        The following patch to getpwent.c will fix the problem.

        Perhaps the still pending patch 40728 should also been applied together 
with this fix.
        The problem was found while testing the last update for PR40728 just 

        The patch will also fix a problem with truncatetd password from the 
adjunct map
        without error indication to the caller, if the buffer is to small to 
hold the password
        from the adjunct map after the initialy copied entry.
        The style used to check this is inspired by the way the _pw_parse() 
routine does it.
        Incrementing elen already prior first len check is OK, because the 
check was wrong
        before too, but this problem was detected _pw_parse() and the bad check 
was harmless.

--- getpwent.c  2012/03/28 14:54:50     1.3
+++ getpwent.c  2012/03/29 09:15:30
@@ -1204,7 +1204,7 @@
        _DIAGASSERT(state != NULL);
        elen = strlen(entry);
-       if (elen >= buflen)
+       if (++elen >= buflen)  /* remark: we need the ++ for the adjunct cast 
below */
                return 0;
        if (! _pw_parse(entry, pw, buf, buflen,
            !(state->maptype == NISMAP_MASTER)))
@@ -1221,10 +1221,13 @@
                        char    *bp, *ep;
                                                /* skip name to get password */
                        ep = data;
-                       if ((bp = strsep(&ep, ":")) != NULL &&
+                       if (      strsep(&ep, ":")  != NULL &&
                            (bp = strsep(&ep, ":")) != NULL) {
                                        /* store new pw_passwd after entry */
-                               strlcpy(buf + elen, bp, buflen - elen);
+                               if (strlcpy(buf + elen, bp, buflen - elen) >= 
buflen - elen) {
+                                         free(data);
+                                         return 0;
+                               }
                                pw->pw_passwd = &buf[elen];


Home | Main Index | Thread Index | Old Index