Date: Tue, 24 Oct 2000 19:25:39 +0200 From: Guido van Rooij <guido@gvr.org> To: Jeroen Ruigrok van der Werven <jruigrok@via-net-works.nl> Cc: audit@FreeBSD.ORG Subject: Re: printjob.c mktemp() problem Message-ID: <20001024192539.A65599@gvr.gvr.org> In-Reply-To: <20001024170054.Q93799@lucifer.bart.nl>; from jruigrok@via-net-works.nl on Tue, Oct 24, 2000 at 05:00:54PM %2B0200 References: <20001024140510.G93799@lucifer.bart.nl> <20001024170054.Q93799@lucifer.bart.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
--PEIAKu/WMn1b1Hv9 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Oct 24, 2000 at 05:00:54PM +0200, Jeroen Ruigrok van der Werven wrote: > > The patch might not be totally accurate yet, but that due to the code > throwing me off at one point, namely the dup2(n, 2); and the > unlink(tempfile); The dup2 is on the already opened tempfile so should not cause any problems. You should remove the addition of int i = -1 since it isn't used. Furthermore, I would exit after the syslog in stead of the return -1 as it seems all errors result in an exit. Did I mention that this code is awfull? Btw: there might be another problem. In line 956, another open() is done with tempfile which you did not cover. (the one you covered is within printit(). This one is reached via sendit(). I guess it is best to also replace this one with mkstemp(). Attached my attempt (which also fixes: printjob.c:1339: warning: int format, long int arg (arg 3) -Guido --PEIAKu/WMn1b1Hv9 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=pp Index: lpd/printjob.c =================================================================== RCS file: /scratch/cvsup/freebsd/CVS/src/usr.sbin/lpr/lpd/printjob.c,v retrieving revision 1.23 diff -u -r1.23 printjob.c --- lpd/printjob.c 2000/05/24 11:38:41 1.23 +++ lpd/printjob.c 2000/10/24 17:24:06 @@ -168,8 +168,6 @@ signal(SIGQUIT, abortpr); signal(SIGTERM, abortpr); - (void) mktemp(tempfile); - /* * uses short form file names */ @@ -733,7 +731,11 @@ if ((child = dofork(pp, DORETURN)) == 0) { /* child */ dup2(fi, 0); dup2(fo, 1); - n = open(tempfile, O_WRONLY|O_CREAT|O_TRUNC, 0664); + if ((n = mkstemp(tempfile)) == -1) { + syslog(LOG_WARNING, "%s: %s: %m", pp->printer, tempfile); + exit(-1); + } + fchmod(n, 0664); if (n >= 0) dup2(n, 2); closelog(); @@ -953,8 +955,12 @@ if ((ifilter = dofork(pp, DORETURN)) == 0) { /* child */ dup2(f, 0); dup2(tfd, 1); - n = open(tempfile, O_WRONLY|O_CREAT|O_TRUNC, - TEMP_FILE_MODE); + if ((n = mkstemp(tempfile)) == -1) { + syslog(LOG_WARNING, + "%s: %s: %m", pp->printer, tempfile); + exit(-1); + } + fchmod(n, 0664); if (n >= 0) dup2(n, 2); closelog(); @@ -1329,7 +1335,7 @@ */ if (pid == 0) { if ((pwd = getpwuid(pp->daemon_user)) == NULL) { - syslog(LOG_ERR, "Can't lookup default daemon uid (%d) in password file", + syslog(LOG_ERR, "Can't lookup default daemon uid (%ld) in password file", pp->daemon_user); break; } --PEIAKu/WMn1b1Hv9-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20001024192539.A65599>