NetBSD Problem Report #46500

From www@NetBSD.org  Wed May 30 10:47:29 2012
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id ED76363B88D
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 30 May 2012 10:47:28 +0000 (UTC)
Message-Id: <20120530104728.28CE563B882@www.NetBSD.org>
Date: Wed, 30 May 2012 10:47:28 +0000 (UTC)
From: henning.petersen@t-online.de
Reply-To: henning.petersen@t-online.de
To: gnats-bugs@NetBSD.org
Subject: Permission of created files in lpr.c wrong.
X-Send-Pr-Version: www-1.0

>Number:         46500
>Category:       bin
>Synopsis:       Permission of created files in lpr.c wrong.
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed May 30 10:50:00 +0000 2012
>Closed-Date:    Sun Oct 06 12:37:20 +0000 2013
>Last-Modified:  Sun Oct 06 12:37:20 +0000 2013
>Originator:     Henning Petersen
>Release:        NetBSD-current
>Organization:
>Environment:
>Description:
Permission of created files */.seq is wrong.
>How-To-Repeat:

>Fix:
diff -u -p -r1.45 lpr.c
--- usr.sbin/lpr/lpr/lpr.c	30 Aug 2011 19:27:37 -0000	1.45
+++ usr.sbin/lpr/lpr/lpr.c	30 May 2012 09:07:55 -0000
@@ -698,7 +698,7 @@ mktemps(void)

 	(void)snprintf(buf, sizeof(buf), "%s/.seq", SD);
 	seteuid(euid);
-	if ((fd = open(buf, O_RDWR|O_CREAT, 0661)) < 0)
+	if ((fd = open(buf, O_RDWR|O_CREAT, 0664)) < 0)
 		err(1, "cannot create %s", buf);
 	if (flock(fd, LOCK_EX))
 		err(1, "cannot lock %s", buf);

>Release-Note:

>Audit-Trail:
From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: re: bin/46500: Permission of created files in lpr.c wrong.
Date: Thu, 31 May 2012 03:03:33 +1000

 > >Number:         46500
 > >Category:       bin
 > >Synopsis:       Permission of created files in lpr.c wrong.
 > >Confidential:   no
 > >Severity:       serious
 > >Priority:       medium
 > >Responsible:    bin-bug-people
 > >State:          open
 > >Class:          sw-bug
 > >Submitter-Id:   net
 > >Arrival-Date:   Wed May 30 10:50:00 +0000 2012
 > >Originator:     Henning Petersen
 > >Release:        NetBSD-current
 > >Organization:
 > >Environment:
 > >Description:
 > Permission of created files */.seq is wrong.
 > >How-To-Repeat:
 > 
 > >Fix:
 > diff -u -p -r1.45 lpr.c
 > --- usr.sbin/lpr/lpr/lpr.c	30 Aug 2011 19:27:37 -0000	1.45
 > +++ usr.sbin/lpr/lpr/lpr.c	30 May 2012 09:07:55 -0000
 > @@ -698,7 +698,7 @@ mktemps(void)
 >  
 >  	(void)snprintf(buf, sizeof(buf), "%s/.seq", SD);
 >  	seteuid(euid);
 > -	if ((fd = open(buf, O_RDWR|O_CREAT, 0661)) < 0)
 > +	if ((fd = open(buf, O_RDWR|O_CREAT, 0664)) < 0)
 >  		err(1, "cannot create %s", buf);
 >  	if (flock(fd, LOCK_EX))
 >  		err(1, "cannot lock %s", buf);

 what's wrong with this?  your change makes the temp files world
 readable which seems like a security issue to me.


 .mrg.

From: matthew green <mrg@eterna.com.au>
To: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
    gnats-bugs@NetBSD.org
Cc: 
Subject: re: bin/46500: Permission of created files in lpr.c wrong.
Date: Thu, 31 May 2012 03:30:24 +1000

 > > >Description:
 > > Permission of created files */.seq is wrong.
 > > >How-To-Repeat:
 > > 
 > > >Fix:
 > > diff -u -p -r1.45 lpr.c
 > > --- usr.sbin/lpr/lpr/lpr.c	30 Aug 2011 19:27:37 -0000	1.45
 > > +++ usr.sbin/lpr/lpr/lpr.c	30 May 2012 09:07:55 -0000
 > > @@ -698,7 +698,7 @@ mktemps(void)
 > >  
 > >  	(void)snprintf(buf, sizeof(buf), "%s/.seq", SD);
 > >  	seteuid(euid);
 > > -	if ((fd = open(buf, O_RDWR|O_CREAT, 0661)) < 0)
 > > +	if ((fd = open(buf, O_RDWR|O_CREAT, 0664)) < 0)
 > >  		err(1, "cannot create %s", buf);
 > >  	if (flock(fd, LOCK_EX))
 > >  		err(1, "cannot lock %s", buf);
 > 
 > what's wrong with this?  your change makes the temp files world
 > readable which seems like a security issue to me.

 additionally, this will break lpd as it uses these execute bits
 modes specific meanings.


 .mrg.

From: "John Nemeth" <jnemeth@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46500 CVS commit: src/usr.sbin/lpr/lpr
Date: Sat, 2 Jun 2012 03:32:53 +0000

 Module Name:	src
 Committed By:	jnemeth
 Date:		Sat Jun  2 03:32:53 UTC 2012

 Modified Files:
 	src/usr.sbin/lpr/lpr: lpr.c

 Log Message:
 PR/46500 - Henning Petersen -- wrong permissions on create .seq files


 To generate a diff of this commit:
 cvs rdiff -u -r1.45 -r1.46 src/usr.sbin/lpr/lpr/lpr.c

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

From: Bernd Ernesti <netbsd@lists.veego.de>
To: John Nemeth <jnemeth@netbsd.org>
Cc: gnats-bugs@NetBSD.org
Subject: Re: PR/46500 CVS commit: src/usr.sbin/lpr/lpr
Date: Sat, 2 Jun 2012 07:51:02 +0200

 On Sat, Jun 02, 2012 at 03:35:02AM +0000, John Nemeth wrote:
 > The following reply was made to PR bin/46500; it has been noted by GNATS.
 > 
 > From: "John Nemeth" <jnemeth@netbsd.org>
 > To: gnats-bugs@gnats.NetBSD.org
 > Cc: 
 > Subject: PR/46500 CVS commit: src/usr.sbin/lpr/lpr
 > Date: Sat, 2 Jun 2012 03:32:53 +0000
 > 
 >  Module Name:	src
 >  Committed By:	jnemeth
 >  Date:		Sat Jun  2 03:32:53 UTC 2012
 >  
 >  Modified Files:
 >  	src/usr.sbin/lpr/lpr: lpr.c
 >  
 >  Log Message:
 >  PR/46500 -Reply Henning Petersen -- wrong permissions on create .seq files

 What about the concerns by Matthew Green to not apply this change?

 Bernd

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	henning.petersen@t-online.de
Cc: 
Subject: Re: PR/46500 CVS commit: src/usr.sbin/lpr/lpr
Date: Sat, 2 Jun 2012 09:03:28 -0400

 On Jun 2,  6:00am, netbsd@lists.veego.de (Bernd Ernesti) wrote:
 -- Subject: Re: PR/46500 CVS commit: src/usr.sbin/lpr/lpr

 | The following reply was made to PR bin/46500; it has been noted by GNATS.
 | 
 | From: Bernd Ernesti <netbsd@lists.veego.de>
 | To: John Nemeth <jnemeth@netbsd.org>
 | Cc: gnats-bugs@NetBSD.org
 | Subject: Re: PR/46500 CVS commit: src/usr.sbin/lpr/lpr
 | Date: Sat, 2 Jun 2012 07:51:02 +0200
 | 
 |  On Sat, Jun 02, 2012 at 03:35:02AM +0000, John Nemeth wrote:
 |  > The following reply was made to PR bin/46500; it has been noted by GNATS.
 |  > 
 |  > From: "John Nemeth" <jnemeth@netbsd.org>
 |  > To: gnats-bugs@gnats.NetBSD.org
 |  > Cc: 
 |  > Subject: PR/46500 CVS commit: src/usr.sbin/lpr/lpr
 |  > Date: Sat, 2 Jun 2012 03:32:53 +0000
 |  > 
 |  >  Module Name:	src
 |  >  Committed By:	jnemeth
 |  >  Date:		Sat Jun  2 03:32:53 UTC 2012
 |  >  
 |  >  Modified Files:
 |  >  	src/usr.sbin/lpr/lpr: lpr.c
 |  >  
 |  >  Log Message:
 |  >  PR/46500 -Reply Henning Petersen -- wrong permissions on create .seq files
 |  
 |  What about the concerns by Matthew Green to not apply this change?

 The permissions are set/checked for the lock file not the seq file.

 christos

From: jnemeth@victoria.tc.ca (John Nemeth)
To: matthew green <mrg@eterna.com.au>, gnats-admin@NetBSD.org,
        netbsd-bugs@NetBSD.org, gnats-bugs@NetBSD.org
Cc: 
Subject: re: bin/46500: Permission of created files in lpr.c wrong.
Date: Sat, 2 Jun 2012 12:24:47 -0700

 On Oct 20, 10:06pm, matthew green wrote:
 } 
 } > > >Description:
 } > > Permission of created files */.seq is wrong.
 } > > >How-To-Repeat:
 } > > 
 } > > >Fix:
 } > > diff -u -p -r1.45 lpr.c
 } > > --- usr.sbin/lpr/lpr/lpr.c	30 Aug 2011 19:27:37 -0000	1.45
 } > > +++ usr.sbin/lpr/lpr/lpr.c	30 May 2012 09:07:55 -0000
 } > > @@ -698,7 +698,7 @@ mktemps(void)
 } > >  
 } > >  	(void)snprintf(buf, sizeof(buf), "%s/.seq", SD);
 } > >  	seteuid(euid);
 } > > -	if ((fd = open(buf, O_RDWR|O_CREAT, 0661)) < 0)
 } > > +	if ((fd = open(buf, O_RDWR|O_CREAT, 0664)) < 0)
 } > >  		err(1, "cannot create %s", buf);
 } > >  	if (flock(fd, LOCK_EX))
 } > >  		err(1, "cannot lock %s", buf);
 } > 
 } > what's wrong with this?  your change makes the temp files world
 } > readable which seems like a security issue to me.

      All it contains is a three digit sequence number for the next
 queue entry.  That doesn't seem to be particularly security critical,
 but if you like, it could be changed to 660.  Certainly execute is
 wrong.

 } additionally, this will break lpd as it uses these execute bits
 } modes specific meanings.

      Uh, read the code.  The .seq file is opened, locked, read,
 written, and closed (implicitly unlocking) all within the same function
 and the fd is never passed to any functions other then the libc
 functions required to perform the above operations.  Also, the
 containing function is only called from with one place in lpr.c.  In
 other words, lpd doesn't even look at the file.

 }-- End of excerpt from matthew green

State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 06 Oct 2013 12:37:20 +0000
State-Changed-Why:
fixed last year


>Unformatted:

Home	PR Database Search