Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 8 Oct 2000 23:31:44 -0700
From:      Kris Kennaway <kris@citusc.usc.edu>
To:        audit@freebsd.org
Subject:   make(1) string paranoia part 1 (fwd)
Message-ID:  <20001008233144.A39915@citusc17.usc.edu>

next in thread | raw e-mail | index | archive | help
----- Forwarded message from Will Andrews <will@physics.purdue.edu> -----

Delivered-To: kris@freebsd.org
Date: Mon, 9 Oct 2000 01:23:44 -0500
From: Will Andrews <will@physics.purdue.edu>
To: kris@FreeBSD.org
Subject: make(1) string paranoia part 1
Reply-To: Will Andrews <will@physics.purdue.edu>
User-Agent: Mutt/1.2.5i
X-Operating-System: FreeBSD 4.1-STABLE i386

Here.  The NetBSD make(1) simply converts most of the sprintf() to
snprintf().  Sure, make(1) isn't really much of something that can be
exploited, but nothing wrong with a little string paranoia, IMO.  It
also free()'s the strings properly.

-- 
Will Andrews <will@physics.purdue.edu> - Physics Computer Network wench
The Universal Answer to All Problems - "It has something to do with physics."
	-- Comic on door of Room 240, Physics Building, Purdue University

Index: arch.c
===================================================================
RCS file: /project/cvs/FreeBSD/src/usr.bin/make/arch.c,v
retrieving revision 1.16
diff -u -r1.16 arch.c
--- arch.c	2000/07/09 02:54:53	1.16
+++ arch.c	2000/10/09 06:21:34
@@ -186,7 +186,7 @@
     GNode	    *gn;     	    /* New node */
     char	    *libName;  	    /* Library-part of specification */
     char	    *memName;  	    /* Member-part of specification */
-    char	    nameBuf[MAKE_BSIZE]; /* temporary place for node name */
+    char	    *nameBuf;	    /* temporary place for node name */
     char	    saveChar;  	    /* Ending delimiter of member-name */
     Boolean 	    subLibName;	    /* TRUE if libName should have/had
 				     * variable substitution performed on it */
@@ -299,6 +299,7 @@
 	    char    *buf;
 	    char    *sacrifice;
 	    char    *oldMemName = memName;
+	    size_t   sz;
 
 	    memName = Var_Subst(NULL, memName, ctxt, TRUE);
 
@@ -307,10 +308,12 @@
 	     * variables and multi-word variable values.... The results
 	     * are just placed at the end of the nodeLst we're returning.
 	     */
-	    buf = sacrifice = emalloc(strlen(memName)+strlen(libName)+3);
 
-	    sprintf(buf, "%s(%s)", libName, memName);
+	    sz = strlen(memName) + strlen(libName) + 3;
+	    buf = sacrifice = emalloc(sz);
 
+	    snprintf(buf, sz, "%s(%s)", libName, memName);
+
 	    if (strchr(memName, '$') && strcmp(memName, oldMemName) == 0) {
 		/*
 		 * Must contain dynamic sources, so we can't deal with it now.
@@ -341,15 +344,22 @@
 	} else if (Dir_HasWildcards(memName)) {
 	    Lst	  members = Lst_Init(FALSE);
 	    char  *member;
+	    size_t sz = MAXPATHLEN;
+	    size_t nsz;
+	    nameBuf = emalloc(sz);
 
 	    Dir_Expand(memName, dirSearchPath, members);
 	    while (!Lst_IsEmpty(members)) {
 		member = (char *)Lst_DeQueue(members);
+		nsz = strlen(libName) + strlen(member) + 3;
+		if (sz > nsz)
+			nameBuf = erealloc(nameBuf, sz = nsz * 2);
 
-		sprintf(nameBuf, "%s(%s)", libName, member);
+		snprintf(nameBuf, sz, "%s(%s)", libName, member);
 		free(member);
 		gn = Targ_FindNode (nameBuf, TARG_CREATE);
 		if (gn == NILGNODE) {
+		    free(nameBuf);
 		    return (FAILURE);
 		} else {
 		    /*
@@ -364,9 +374,13 @@
 		}
 	    }
 	    Lst_Destroy(members, NOFREE);
+	    free(nameBuf);
 	} else {
-	    sprintf(nameBuf, "%s(%s)", libName, memName);
+	    size_t sz = strlen(libName) + strlen(memName) + 3;
+	    nameBuf = emalloc(sz);
+	    snprintf(nameBuf, sz, "%s(%s)", libName, memName);
 	    gn = Targ_FindNode (nameBuf, TARG_CREATE);
+	    free(nameBuf);
 	    if (gn == NILGNODE) {
 		return (FAILURE);
 	    } else {
@@ -927,7 +941,7 @@
 			  &arh, "r+");
     efree(p1);
     efree(p2);
-    sprintf(arh.ar_date, "%-12ld", (long) now);
+    snprintf(arh.ar_date, sizeof(arh.ar_date), "%-12ld", (long) now);
 
     if (arch != NULL) {
 	(void)fwrite ((char *)&arh, sizeof (struct ar_hdr), 1, arch);
@@ -960,7 +974,7 @@
     struct utimbuf  times;	/* Times for utime() call */
 
     arch = ArchFindMember (gn->path, RANLIBMAG, &arh, "r+");
-    sprintf(arh.ar_date, "%-12ld", (long) now);
+    snprintf(arh.ar_date, sizeof(arh.ar_date), "%-12ld", (long) now);
 
     if (arch != NULL) {
 	(void)fwrite ((char *)&arh, sizeof (struct ar_hdr), 1, arch);
@@ -1096,9 +1110,11 @@
     Lst	    	    path;	      /* Search path */
 {
     char	    *libName;   /* file name for archive */
+    size_t	    sz;
 
-    libName = (char *)emalloc (strlen (gn->name) + 6 - 2);
-    sprintf(libName, "lib%s.a", &gn->name[2]);
+    libName = (char *)emalloc(sz);
+    sz = strlen(gn->name) + 4;
+    snprintf(libName, sz, "lib%s.a", &gn->name[2]);
 
     gn->path = Dir_FindFile (libName, path);
 


----- End forwarded message -----


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?20001008233144.A39915>