From 363e11939739d08c4dc3a0304a6ff5b28eb7dee6 Mon Sep 17 00:00:00 2001 From: christos Date: Sat, 24 Jun 1995 16:21:33 +0000 Subject: [PATCH] - Don't use system(3) to fork processes. It is a big security hole. - Encode the filenames in the scan files using strvis(3), so filenames that contain newlines or other weird characters don't break the scanner. --- usr.sbin/sup/source/run.c | 71 ++++++++++++++++++++++++++++++ usr.sbin/sup/source/scan.c | 33 +++++++++----- usr.sbin/sup/source/supcmeat.c | 16 +++++-- usr.sbin/sup/source/supfilesrv.c | 74 +++++++++++++++++++++----------- 4 files changed, 156 insertions(+), 38 deletions(-) diff --git a/usr.sbin/sup/source/run.c b/usr.sbin/sup/source/run.c index d1ffa52dc938..97dcd1dcf4a8 100644 --- a/usr.sbin/sup/source/run.c +++ b/usr.sbin/sup/source/run.c @@ -29,6 +29,7 @@ * i = runv (file, arglist); * i = runp (file, arg1, arg2, ..., argn, 0); * i = runvp (file, arglist); + * i = runio (argv, in, out, err); * * Run, runv, runp and runvp have argument lists exactly like the * corresponding routines, execl, execv, execlp, execvp. The run @@ -47,6 +48,11 @@ ********************************************************************** * HISTORY * $Log: run.c,v $ + * Revision 1.2 1995/06/24 16:21:33 christos + * - Don't use system(3) to fork processes. It is a big security hole. + * - Encode the filenames in the scan files using strvis(3), so filenames + * that contain newlines or other weird characters don't break the scanner. + * * Revision 1.1.1.1 1993/05/21 14:52:17 cgd * initial import of CMU's SUP to NetBSD * @@ -90,9 +96,16 @@ #include #include +#include #include #include +#ifndef __STDC__ +#ifndef const +#define const +#endif +#endif + static int dorun(); int run (name,va_alist) @@ -176,3 +189,61 @@ int usepath; return (status.w_retcode); } + +/* + * Like system(3), but with an argument list and explicit redirections + * that does not use the shell + */ +int +runio(argv, infile, outfile, errfile) + char *const argv[]; + const char *infile; + const char *outfile; + const char *errfile; +{ + int fd; + pid_t pid; + int status; + + switch ((pid = fork())) { + case -1: + return -1; + + case 0: + if (infile) { + (void) close(0); + if ((fd = open(infile, O_RDONLY)) == -1) + exit(1); + if (fd != 0) + (void) dup2(fd, 0); + } + + if (outfile) { + (void) close(1); + if ((fd = open(outfile, O_RDWR|O_CREAT|O_TRUNC, + 0666)) == -1) + exit(1); + if (fd != 1) + (void) dup2(fd, 1); + } + + if (errfile) { + (void) close(2); + if ((fd = open(errfile, O_RDWR|O_CREAT|O_TRUNC, + 0666)) == -1) + exit(1); + if (fd != 2) + (void) dup2(fd, 2); + } + + execvp(argv[0], argv); + exit(1); + /*NOTREACHED*/ + return 0; + + default: + if (waitpid(pid, &status, 0) == -1) + return -1; + return status; + } +} diff --git a/usr.sbin/sup/source/scan.c b/usr.sbin/sup/source/scan.c index f484e88e7ef7..ff4392067661 100644 --- a/usr.sbin/sup/source/scan.c +++ b/usr.sbin/sup/source/scan.c @@ -28,6 +28,11 @@ ********************************************************************** * HISTORY * $Log: scan.c,v $ + * Revision 1.3 1995/06/24 16:21:42 christos + * - Don't use system(3) to fork processes. It is a big security hole. + * - Encode the filenames in the scan files using strvis(3), so filenames + * that contain newlines or other weird characters don't break the scanner. + * * Revision 1.2 1994/01/03 14:47:25 brezak * Change to * @@ -90,7 +95,9 @@ #include #include +#include #include +#include #include #include #ifdef HAS_POSIX_DIR @@ -291,7 +298,7 @@ char *release; if (f != NULL) { rewound = TRUE; for (;;) { - p = fgets (buf,STRINGLENGTH,f); + p = fgets (buf,sizeof(buf),f); if (p == NULL) { if (rewound) break; @@ -343,7 +350,7 @@ makescanlists () (void) sprintf (buf,FILERELEASES,collname); f = fopen (buf,"r"); if (f != NULL) { - while (p = fgets (buf,STRINGLENGTH,f)) { + while (p = fgets (buf,sizeof(buf),f)) { q = index (p,'\n'); if (q) *q = 0; if (index ("#;:",*p)) continue; @@ -459,7 +466,7 @@ static readlistfile (fname) char *fname; { - char buf[STRINGLENGTH],*p; + char buf[STRINGLENGTH+MAXPATHLEN*4+1],*p; register char *q,*r; register FILE *f; register int ltn,n,i,flags; @@ -470,7 +477,7 @@ char *fname; f = fopen (fname,"r"); if (f == NULL) goaway ("Can't read list file %s",fname); cdprefix (prefix); - while (p = fgets (buf,STRINGLENGTH,f)) { + while (p = fgets (buf,sizeof(buf),f)) { if (q = index (p,'\n')) *q = '\0'; if (index ("#;:",*p)) continue; q = nxtarg (&p," \t"); @@ -811,6 +818,7 @@ int getscanfile (scanfile) char *scanfile; { char buf[STRINGLENGTH]; + char fname[MAXPATHLEN]; struct stat sbuf; register FILE *f; TREE ts; @@ -826,7 +834,7 @@ char *scanfile; return (FALSE); if ((f = fopen (buf,"r")) == NULL) return (FALSE); - if ((p = fgets (buf,STRINGLENGTH,f)) == NULL) { + if ((p = fgets (buf,sizeof(buf),f)) == NULL) { (void) fclose (f); return (FALSE); } @@ -846,7 +854,7 @@ char *scanfile; return (TRUE); } notwanted = FALSE; - while (p = fgets (buf,STRINGLENGTH,f)) { + while (p = fgets (buf,sizeof(buf),f)) { q = index (p,'\n'); if (q) *q = 0; ts.Tflags = 0; @@ -880,20 +888,21 @@ char *scanfile; goaway ("scanfile format inconsistant"); *q++ = 0; ts.Tmtime = atoi (p); + (void) strunvis(fname, q); if (ts.Tctime > lasttime) ts.Tflags |= FNEW; else if (newonly) { for (tl = listTL; tl != NULL; tl = tl->TLnext) - if (tmp = Tsearch (tl->TLtree,q)) + if (tmp = Tsearch (tl->TLtree,fname)) tmp->Tflags &= ~FNEW; notwanted = TRUE; continue; } - if (Tlookup (refuseT,q)) { + if (Tlookup (refuseT,fname)) { notwanted = TRUE; continue; } - t = Tinsert (&listT,q,TRUE); + t = Tinsert (&listT,fname,TRUE); t->Tmode = ts.Tmode; t->Tflags = ts.Tflags; t->Tctime = ts.Tctime; @@ -957,9 +966,11 @@ TREE *t; FILE **scanF; { int recordexec (); + char fname[MAXPATHLEN*4+1]; if (t->Tflags&FBACKUP) fprintf (*scanF,"B"); if (t->Tflags&FNOACCT) fprintf (*scanF,"N"); + strvis(fname, t->Tname, VIS_WHITE); fprintf (*scanF,"%o %d %d %s\n", t->Tmode,t->Tctime,t->Tmtime,t->Tname); (void) Tprocess (t->Texec,recordexec,*scanF); @@ -971,7 +982,9 @@ recordexec (t,scanF) TREE *t; FILE **scanF; { - fprintf(*scanF,"X%s\n",t->Tname); + char fname[MAXPATHLEN*4+1]; + strvis(fname, t->Tname, VIS_WHITE); + fprintf(*scanF,"X%s\n",fname); return (SCMOK); } diff --git a/usr.sbin/sup/source/supcmeat.c b/usr.sbin/sup/source/supcmeat.c index 4d4e3eb3e5a0..ced02966349a 100644 --- a/usr.sbin/sup/source/supcmeat.c +++ b/usr.sbin/sup/source/supcmeat.c @@ -32,6 +32,11 @@ * across the network to save BandWidth * * $Log: supcmeat.c,v $ + * Revision 1.5 1995/06/24 16:21:48 christos + * - Don't use system(3) to fork processes. It is a big security hole. + * - Encode the filenames in the scan files using strvis(3), so filenames + * that contain newlines or other weird characters don't break the scanner. + * * Revision 1.4 1995/06/03 21:21:56 christos * Changes to write ascii timestamps in the when files. * Looked into making it 64 bit clean, but it is hopeless. @@ -1081,7 +1086,6 @@ char *from; /* 0 if reading from network */ register int fromf,tof,istemp,x; char dpart[STRINGLENGTH],fpart[STRINGLENGTH]; char tname[STRINGLENGTH]; - char sys_com[STRINGLENGTH]; struct stat sbuf; static int thispid = 0; /* process id # */ @@ -1217,9 +1221,13 @@ char *from; /* 0 if reading from network */ } /* uncompress it first */ if (docompress) { - sprintf(sys_com, "gunzip < %s > %s\n", tname, to); - /* Uncompress it onto the destination */ - if (system(sys_com) < 0) { + char *av[4]; + int ac = 0; + av[ac++] = "gzip"; + av[ac++] = "-d"; + av[ac++] = NULL; + if (runio(av, tname, to, NULL) < 0) { + /* Uncompress it onto the destination */ notify ("SUP: Error in uncompressing file %s\n", to); (void) unlink (tname); diff --git a/usr.sbin/sup/source/supfilesrv.c b/usr.sbin/sup/source/supfilesrv.c index d3243d20a1d1..423c902d7183 100644 --- a/usr.sbin/sup/source/supfilesrv.c +++ b/usr.sbin/sup/source/supfilesrv.c @@ -42,6 +42,11 @@ * across the network to save BandWidth * * $Log: supfilesrv.c,v $ + * Revision 1.7 1995/06/24 16:21:55 christos + * - Don't use system(3) to fork processes. It is a big security hole. + * - Encode the filenames in the scan files using strvis(3), so filenames + * that contain newlines or other weird characters don't break the scanner. + * * Revision 1.6 1995/06/03 21:22:00 christos * Changes to write ascii timestamps in the when files. * Looked into making it 64 bit clean, but it is hopeless. @@ -1141,18 +1146,19 @@ TREE *t; { register int x,fd; register int fdtmp; - char sys_com[STRINGLENGTH], temp_file[STRINGLENGTH], rcs_file[STRINGLENGTH]; + char temp_file[STRINGLENGTH], rcs_file[STRINGLENGTH]; union wait status; char *uconvert(),*gconvert(); int sendfile (); + int ac; + char *av[50]; /* More than enough */ if ((t->Tflags&FNEEDED) == 0) /* only send needed files */ return (SCMOK); if ((t->Tmode&S_IFMT) == S_IFDIR) /* send no directories this pass */ return (SCMOK); x = msgsend (); - if (x != SCMOK) goaway ("Error reading receive file request from clien -t"); + if (x != SCMOK) goaway ("Error reading receive file request from client"); upgradeT = t; /* upgrade file pointer */ fd = -1; /* no open file */ if ((t->Tmode&S_IFMT) == S_IFREG) { @@ -1164,42 +1170,60 @@ t"); tmpnam(rcs_file); if (strcmp(&t->Tname[strlen(t->Tname)-2], ",v") == 0) { t->Tname[strlen(t->Tname)-2] = '\0'; - if (rcs_branch != NULL) + ac = 0; #ifdef CVS - sprintf(rcs_release, "-r %s", rcs_branch); + av[ac++] = "cvs"; + av[ac++] = "-d"; + av[ac++] = cvs_root; + av[ac++] = "-r"; + av[ac++] = "-l"; + av[ac++] = "-Q"; + av[ac++] = "co"; + av[ac++] = "-p"; + if (rcs_branch != NULL) { + av[ac++] = "-r"; + av[ac++] = rcs_branch; + } #else - sprintf(rcs_release, "-r%s", rcs_branch); + av[ac++] = "co"; + av[ac++] = "-q"; + av[ac++] = "-p"; + if (rcs_branch != NULL) { + sprintf(rcs_release, "-r%s", + rcs_branch); + av[ac++] = rcs_release; + } #endif - else - rcs_release[0] = '\0'; -#ifdef CVS - sprintf(sys_com, "cvs -d %s -r -l -Q co -p %s %s > %s\n", cvs_root, rcs_release, t->Tname, rcs_file); -#else - sprintf(sys_com, "co -q -p %s %s > %s 2> /dev/null\n", rcs_release, t->Tname, rcs_file); -#endif - /*loginfo("using rcs mode \"%s\"\n", sys_com);*/ - status.w_status = system(sys_com); + av[ac++] = t->Tname; + av[ac++] = NULL; + status.w_status = runio(av, + NULL, + rcs_file, + "/dev/null"); + /*loginfo("using rcs mode \n");*/ if (status.w_status < 0 || status.w_retcode) { /* Just in case */ unlink(rcs_file); if (status.w_status < 0) { - goaway ("We died trying to \"%s\"", sys_com); + goaway ("We died trying to run cvs or rcs"); t->Tmode = 0; } else { - /*logerr("rcs command failed \"%s\" = %d\n", - sys_com, status.w_retcode);*/ + /*logerr("rcs command failed = %d\n", + status.w_retcode);*/ t->Tflags |= FUPDATE; } } else if (docompress) { tmpnam(temp_file); - sprintf(sys_com, "gzip -c < %s > %s\n", rcs_file, temp_file); - if (system(sys_com) < 0) { + av[0] = "gzip"; + av[1] = "-c"; + av[2] = NULL; + if (runio(av, rcs_file, temp_file, NULL) < 0) { /* Just in case */ unlink(temp_file); unlink(rcs_file); - goaway ("We died trying to \"%s\"", sys_com); + goaway ("We died trying to gzip a file"); t->Tmode = 0; } fd = open (temp_file,O_RDONLY,0); @@ -1212,11 +1236,13 @@ t"); if (fd == -1) { if (docompress) { tmpnam(temp_file); - sprintf(sys_com, "gzip -c < %s > %s\n", t->Tname, temp_file); - if (system(sys_com) < 0) { + av[0] = "gzip"; + av[1] = "-c"; + av[2] = NULL; + if (runio(av, t->Tname, temp_file, NULL) < 0) { /* Just in case */ unlink(temp_file); - goaway ("We died trying to \"%s\"", sys_com); + goaway ("We died trying to run gzip"); t->Tmode = 0; } fd = open (temp_file,O_RDONLY,0);