NetBSD Problem Report #46279

From Wolfgang.Stukenbrock@nagler-company.com  Thu Mar 29 09:32:29 2012
Return-Path: <Wolfgang.Stukenbrock@nagler-company.com>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id 5C5D663BBEC
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 29 Mar 2012 09:32:29 +0000 (UTC)
Message-Id: <20120329093231.F1C744EA9F8@s012.nagler-company.com>
Date: Thu, 29 Mar 2012 11:32:31 +0200 (CEST)
From: Wolfgang.Stukenbrock@nagler-company.com
Reply-To: Wolfgang.Stukenbrock@nagler-company.com
To: gnats-bugs@gnats.NetBSD.org
Subject: getpwent-routines failes to extract password from adjunct NIS map
X-Send-Pr-Version: 3.95

>Number:         46279
>Category:       lib
>Synopsis:       getpwent-routines failes to extract password from adjunct NIS map
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Mar 29 09:35:00 +0000 2012
>Closed-Date:    Thu Nov 07 20:50:23 +0000 2013
>Last-Modified:  Thu Nov 07 20:50:23 +0000 2013
>Originator:     Dr. W. Stukenbrock
>Release:        NetBSD 6.0-beta
>Organization:
Dr. Nagler & Company GmbH
>Environment:


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
>Description:
	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.
>How-To-Repeat:
	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.
>Fix:
	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 send.

	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];
                        }
                        free(data);

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46279 CVS commit: src/lib/libc/gen
Date: Thu, 29 Mar 2012 10:43:58 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Thu Mar 29 14:43:58 UTC 2012

 Modified Files:
 	src/lib/libc/gen: getpwent.c

 Log Message:
 PR/46279: Dr. W. Stukenbrock: Off-by-one in buffer length check and make sure
 that the password fits in the supplied buffer.


 To generate a diff of this commit:
 cvs rdiff -u -r1.78 -r1.79 src/lib/libc/gen/getpwent.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: PR/46279 CVS commit: src/lib/libc/gen
Date: Sat, 2 Nov 2013 18:14:33 +0000

 On Thu, Mar 29, 2012 at 02:45:02PM +0000, Christos Zoulas wrote:
  >  Log Message:
  >  PR/46279: Dr. W. Stukenbrock: Off-by-one in buffer length check
  >  and make sure
  >  that the password fits in the supplied buffer.

 Are we going to pull this up or should it be closed? Been sitting here
 for >18 months :(

 -- 
 David A. Holland
 dholland@netbsd.org

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, lib-bug-people@netbsd.org, 
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	Wolfgang.Stukenbrock@nagler-company.com
Cc: 
Subject: Re: PR/46279 CVS commit: src/lib/libc/gen
Date: Sat, 2 Nov 2013 16:05:20 -0400

 On Nov 2,  6:15pm, dholland-bugs@netbsd.org (David Holland) wrote:
 -- Subject: Re: PR/46279 CVS commit: src/lib/libc/gen

 |  Are we going to pull this up or should it be closed? Been sitting here
 |  for >18 months :(
 |  -- 

 Just did.

 christos

State-Changed-From-To: open->pending-pullups
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sat, 02 Nov 2013 23:59:39 +0000
State-Changed-Why:
pullup-6 #979
(thanks)


From: "Soren Jacobsen" <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46279 CVS commit: [netbsd-6] src/lib/libc/gen
Date: Thu, 7 Nov 2013 20:38:43 +0000

 Module Name:	src
 Committed By:	snj
 Date:		Thu Nov  7 20:38:43 UTC 2013

 Modified Files:
 	src/lib/libc/gen [netbsd-6]: getpwent.c

 Log Message:
 Pull up following revision(s) (requested by christos in ticket #979):
 	lib/libc/gen/getpwent.c: revision 1.79
 PR/46279: Dr. W. Stukenbrock: Off-by-one in buffer length check and make sure
 that the password fits in the supplied buffer.


 To generate a diff of this commit:
 cvs rdiff -u -r1.77 -r1.77.8.1 src/lib/libc/gen/getpwent.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: snj@NetBSD.org
State-Changed-When: Thu, 07 Nov 2013 20:50:23 +0000
State-Changed-Why:
Pulled up to netbsd-6.


>Unformatted:

Home	PR Database Search