From owner-freebsd-audit Tue Oct 24 10:25:46 2000 Delivered-To: freebsd-audit@freebsd.org Received: from gvr.gvr.org (gvr.gvr.org [194.151.74.97]) by hub.freebsd.org (Postfix) with ESMTP id BC09337B479 for ; Tue, 24 Oct 2000 10:25:41 -0700 (PDT) Received: by gvr.gvr.org (Postfix, from userid 657) id 227DE5806; Tue, 24 Oct 2000 19:25:40 +0200 (CEST) Date: Tue, 24 Oct 2000 19:25:39 +0200 From: Guido van Rooij To: Jeroen Ruigrok van der Werven Cc: audit@FreeBSD.ORG Subject: Re: printjob.c mktemp() problem Message-ID: <20001024192539.A65599@gvr.gvr.org> References: <20001024140510.G93799@lucifer.bart.nl> <20001024170054.Q93799@lucifer.bart.nl> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="PEIAKu/WMn1b1Hv9" Content-Disposition: inline In-Reply-To: <20001024170054.Q93799@lucifer.bart.nl>; from jruigrok@via-net-works.nl on Tue, Oct 24, 2000 at 05:00:54PM +0200 Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG --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