Date: Sat, 11 Nov 2000 20:43:43 +0000 From: Josef Karthauser <joe@pavilion.net> To: Chris Faulhaber <jedgar@fxp.org> Cc: freebsd-audit@FreeBSD.ORG Subject: Re: crunchgen(1) patch Message-ID: <20001111204343.B17219@pavilion.net> In-Reply-To: <20001111085645.A77992@earth.causticlabs.com>; from jedgar@fxp.org on Sat, Nov 11, 2000 at 08:56:45AM -0500 References: <20001111085645.A77992@earth.causticlabs.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Cool. I'll review and commit this as time permits. (Seeing as I'm hacking crunchgen at the moment.) [But don't let that stop anyone else if they have a burning inclination]. Joe On Sat, Nov 11, 2000 at 08:56:45AM -0500, Chris Faulhaber wrote: > Attached is a patch to src/usr.sbin/crunch/crunchgen/crunchgen.c to > do the following: > > 1) replace mktemp() usage with mkstemp() > 2) allocate [MAXPATHLEN + 1] to ensure MAXPATHLEN and '\0' > 3) strcpy() -> strlcpy() > 4) sprintf() -> snprintf() > > My orignal intention was to fix mktemp(3) usage; however, I could not > resist fixing the strcpy(3)'s and sprintf(3)'s also. Note that the > 'sprintf(line, ...' lines were probably ok (unless MAXPATHLEN approaches > MAXLINELEN), but I figured better safe than otherwise. > > -- > Chris D. Faulhaber - jedgar@fxp.org - jedgar@FreeBSD.org > -------------------------------------------------------- > FreeBSD: The Power To Serve - http://www.FreeBSD.org > Index: crunchgen.c > =================================================================== > RCS file: /home/ncvs/src/usr.sbin/crunch/crunchgen/crunchgen.c,v > retrieving revision 1.15 > diff -u -r1.15 crunchgen.c > --- crunchgen.c 2000/11/10 15:21:37 1.15 > +++ crunchgen.c 2000/11/11 13:32:52 > @@ -83,10 +83,12 @@ > > char line[MAXLINELEN]; > > -char confname[MAXPATHLEN], infilename[MAXPATHLEN]; > -char outmkname[MAXPATHLEN], outcfname[MAXPATHLEN], execfname[MAXPATHLEN]; > -char tempfname[MAXPATHLEN], cachename[MAXPATHLEN], curfilename[MAXPATHLEN]; > -char outhdrname[MAXPATHLEN] ; /* user-supplied header for *.mk */ > +char confname[MAXPATHLEN + 1], infilename[MAXPATHLEN + 1]; > +char outmkname[MAXPATHLEN + 1], outcfname[MAXPATHLEN + 1]; > +char execfname[MAXPATHLEN + 1]; > +char tempfname[MAXPATHLEN + 1], cachename[MAXPATHLEN + 1]; > +char curfilename[MAXPATHLEN + 1]; > +char outhdrname[MAXPATHLEN + 1] ; /* user-supplied header for *.mk */ > int linenum = -1; > int goterror = 0; > > @@ -126,10 +128,26 @@ > case 'o': makeobj = 1; break; > case 'q': verbose = 0; break; > > - case 'm': strcpy(outmkname, optarg); break; > - case 'h': strcpy(outhdrname, optarg); break; > - case 'c': strcpy(outcfname, optarg); break; > - case 'e': strcpy(execfname, optarg); break; > + case 'm': > + if (strlcpy(outmkname, optarg, sizeof(outmkname)) >= > + sizeof(outmkname)) > + usage(); > + break; > + case 'h': > + if (strlcpy(outhdrname, optarg, sizeof(outhdrname)) >= > + sizeof(outmkname)) > + usage(); > + break; > + case 'c': > + if (strlcpy(outcfname, optarg, sizeof(outcfname)) >= > + sizeof(outmkname)) > + usage(); > + break; > + case 'e': > + if (strlcpy(execfname, optarg, sizeof(execfname)) >= > + sizeof(outmkname)) > + usage(); > + break; > case 'l': list_mode++; verbose = 0; break; > > case '?': > @@ -146,24 +164,21 @@ > * generate filenames > */ > > - strcpy(infilename, argv[0]); > + if (strlcpy(infilename, argv[0], sizeof(infilename)) >= sizeof(outmkname)) > + usage(); > > /* confname = `basename infilename .conf` */ > > - if((p=strrchr(infilename, '/')) != NULL) strcpy(confname, p+1); > - else strcpy(confname, infilename); > + if((p=strrchr(infilename, '/')) != NULL) strlcpy(confname, p+1, sizeof(confname)); > + else strlcpy(confname, infilename, sizeof(confname)); > if((p=strrchr(confname, '.')) != NULL && !strcmp(p, ".conf")) *p = '\0'; > > - if(!*outmkname) sprintf(outmkname, "%s.mk", confname); > - if(!*outcfname) sprintf(outcfname, "%s.c", confname); > - if(!*execfname) sprintf(execfname, "%s", confname); > - > - sprintf(cachename, "%s.cache", confname); > - sprintf(tempfname, ".tmp_%sXXXXXX", confname); > - if(mktemp(tempfname) == NULL) { > - perror(tempfname); > - exit(1); > - } > + if(!*outmkname) snprintf(outmkname, sizeof(outmkname), "%s.mk", confname); > + if(!*outcfname) snprintf(outcfname, sizeof(outcfname), "%s.c", confname); > + if(!*execfname) snprintf(execfname, sizeof(execfname), "%s", confname); > + > + snprintf(cachename, sizeof(cachename), "%s.cache", confname); > + snprintf(tempfname, sizeof(tempfname), ".tmp_%sXXXXXX", confname); > > parse_conf_file(); > if (list_mode) > @@ -223,9 +238,9 @@ > void (*f)(int c, char **v); > FILE *cf; > > - sprintf(line, "reading %s", filename); > + snprintf(line, sizeof(line), "reading %s", filename); > status(line); > - strcpy(curfilename, filename); > + strlcpy(curfilename, filename, sizeof(curfilename)); > > if((cf = fopen(curfilename, "r")) == NULL) { > warn("%s", curfilename); > @@ -492,11 +507,11 @@ > */ > void fillin_program(prog_t *p) > { > - char path[MAXPATHLEN]; > + char path[MAXPATHLEN + 1]; > char *srcparent; > strlst_t *s; > > - sprintf(line, "filling in parms for %s", p->name); > + snprintf(line, sizeof(line), "filling in parms for %s", p->name); > status(line); > > if(!p->ident) > @@ -504,14 +519,14 @@ > if(!p->srcdir) { > srcparent = dir_search(p->name); > if(srcparent) > - sprintf(path, "%s/%s", srcparent, p->name); > + snprintf(path, sizeof(path), "%s/%s", srcparent, p->name); > if(is_dir(path)) > p->srcdir = strdup(path); > } > if(!p->objdir && p->srcdir) { > FILE *f; > > - sprintf(path, "cd %s && echo -n /usr/obj`/bin/pwd`", p->srcdir); > + snprintf(path, sizeof(path), "cd %s && echo -n /usr/obj`/bin/pwd`", p->srcdir); > p->objdir = p->srcdir; > f = popen(path,"r"); > if (f) { > @@ -526,18 +541,18 @@ > * XXX look for a Makefile.{name} in local directory first. > * This lets us override the original Makefile. > */ > - sprintf(path, "Makefile.%s", p->name); > + snprintf(path, sizeof(path), "Makefile.%s", p->name); > if (is_nonempty_file(path)) { > - sprintf(line, "Using %s for %s", path, p->name); > + snprintf(line, sizeof(path), "Using %s for %s", path, p->name); > status(line); > } else > - if(p->srcdir) sprintf(path, "%s/Makefile", p->srcdir); > + if(p->srcdir) snprintf(path, sizeof(path), "%s/Makefile", p->srcdir); > if(!p->objs && p->srcdir && is_nonempty_file(path)) > fillin_program_objs(p, path); > > if(!p->objpaths && p->objdir && p->objs) > for(s = p->objs; s != NULL; s = s->next) { > - sprintf(line, "%s/%s", p->objdir, s->str); > + snprintf(line, sizeof(line), "%s/%s", p->objdir, s->str); > add_string(&p->objpaths, line); > } > > @@ -558,14 +573,18 @@ > void fillin_program_objs(prog_t *p, char *path) > { > char *obj, *cp; > - int rc; > + int fd, rc; > FILE *f; > char *objvar="OBJS"; > strlst_t *s; > > /* discover the objs from the srcdir Makefile */ > > - if((f = fopen(tempfname, "w")) == NULL) { > + if((fd = mkstemp(tempfname)) == -1) { > + perror(tempfname); > + exit(1); > + } > + if((f = fdopen(fd, "w")) == NULL) { > warn("%s", tempfname); > goterror = 1; > return; > @@ -592,7 +611,7 @@ > > fclose(f); > > - sprintf(line, "make -f %s crunchgen_objs 2>&1", tempfname); > + snprintf(line, sizeof(line), "make -f %s crunchgen_objs 2>&1", tempfname); > if((f = popen(line, "r")) == NULL) { > warn("submake pipe"); > goterror = 1; > @@ -646,7 +665,7 @@ > FILE *cachef; > prog_t *p; > > - sprintf(line, "generating %s", cachename); > + snprintf(line, sizeof(line), "generating %s", cachename); > status(line); > > if((cachef = fopen(cachename, "w")) == NULL) { > @@ -680,7 +699,7 @@ > prog_t *p; > FILE *outmk; > > - sprintf(line, "generating %s", outmkname); > + snprintf(line, sizeof(line), "generating %s", outmkname); > status(line); > > if((outmk = fopen(outmkname, "w")) == NULL) { > @@ -712,7 +731,7 @@ > prog_t *p; > strlst_t *s; > > - sprintf(line, "generating %s", outcfname); > + snprintf(line, sizeof(line), "generating %s", outcfname); > status(line); > > if((outcf = fopen(outcfname, "w")) == NULL) { > @@ -770,11 +789,11 @@ > > char *dir_search(char *progname) > { > - char path[MAXPATHLEN]; > + char path[MAXPATHLEN + 1]; > strlst_t *dir; > > for(dir=srcdirs; dir != NULL; dir=dir->next) { > - sprintf(path, "%s/%s", dir->str, progname); > + snprintf(path, sizeof(path), "%s/%s", dir->str, progname); > if(is_dir(path)) return dir->str; > } > return NULL; -- Josef Karthauser FreeBSD: How many times have you booted today? Technical Manager Viagra for your server (http://www.uk.freebsd.org) Pavilion Internet plc. [joe@pavilion.net, joe@uk.freebsd.org, joe@tao.org.uk] 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?20001111204343.B17219>