Skip site navigation (1)Skip section navigation (2)
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>