re: bin/46500: Permission of created files in lpr.c wrong.

[jnemeth%victoria.tc.ca@localhost (John Nemeth)]

The following reply was made to PR bin/46500; it has been noted by GNATS.

From: jnemeth%victoria.tc.ca@localhost (John Nemeth)
To: matthew green <mrg%eterna.com.au@localhost>, 
gnats-admin%NetBSD.org@localhost,
        netbsd-bugs%NetBSD.org@localhost, gnats-bugs%NetBSD.org@localhost
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