From owner-freebsd-bugs Fri Dec 14 5:59:52 2001 Delivered-To: freebsd-bugs@freebsd.org Received: from sushi.linuxforlesbians.org (cc265612-b.albqrq1.nm.home.com [67.165.160.99]) by hub.freebsd.org (Postfix) with ESMTP id DDEF637B41B; Fri, 14 Dec 2001 05:58:52 -0800 (PST) Received: by sushi.linuxforlesbians.org (Postfix, from userid 1001) id BD8EB1939; Fri, 14 Dec 2001 07:00:38 -0700 (MST) Date: Fri, 14 Dec 2001 07:00:38 -0700 From: Peter Sanchez To: Robert Watson Cc: freebsd-bugs@freebsd.org Subject: Re: bin/32807: which utility replacement in C Message-ID: <20011214070038.B38914@sushi.linuxforlesbians.org> References: <200112132100.fBDL01r64612@freefall.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: ; from rwatson@FreeBSD.org on Fri, Dec 14, 2001 at 06:19:30AM -0500 Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org Robert, Thanks for clearing that up. I cleaned up the code, and got rid of all the assigned buffers and added access(). Here is the updated source. Peter #include #include #include #include #include #include #include #define NGROUPS 15 struct pathinfo { char *path; struct pathinfo *next; }; struct pathinfo *all = NULL; int found = 1; void printusage(bin) char *bin; { fprintf(stderr,"usage: %s [-a] [-s] program ...\n",bin); return; } int file_exists(file) char *file; { struct stat info; int check, i; check = stat(file,&info); if(check == -1) return check; /* file doesnt exist */ if(info.st_mode & S_IFREG && access(file,X_OK) == 0) return check; else return -1; } void losepath(void) { struct pathinfo *tmp; while(all != NULL) { tmp = all->next; free(all); all = tmp; } return; } void findpath(void) { struct pathinfo *cur = NULL; char *userpath = getenv("PATH"); int i, x; if(userpath == NULL) exit(1); all = (struct pathinfo *)malloc((unsigned)sizeof(struct pathinfo)); if(all == NULL) { fprintf(stderr,"Out of memory, malloc() failed!\n"); exit(1); } cur = all; while((cur->path = strsep(&userpath,":")) != NULL) { cur->next = (struct pathinfo *)malloc((unsigned)sizeof(struct pathinfo)); if(cur->next == NULL) { losepath(); fprintf(stderr,"Out of memory, malloc() failed!\n"); exit(1); } cur = cur->next; } cur->next = NULL; cur = all; return; } void findprog(prog, aflag, sflag) char *prog; int aflag; int sflag; { struct pathinfo *tmp; char *tmpbuf; tmp = all; while(all != NULL) { if(all->path == NULL) break; if(strchr(prog,'/')) tmpbuf = strdup(prog); else { tmpbuf = malloc(sizeof(char) * (strlen(all->path) + strlen(prog) + 1)); if(tmpbuf == NULL) { losepath(); fprintf(stderr,"Out of memory, malloc() failed!\n"); exit(1); } sprintf(tmpbuf,"%s/%s",all->path,prog); } if(!file_exists(tmpbuf)) { found = 0; if(sflag && aflag) ; else if(sflag && !aflag) { all = tmp; free(tmpbuf); return; } else if(aflag && !sflag) printf("%s\n",tmpbuf); else { printf("%s\n",tmpbuf); all = tmp; free(tmpbuf); return; } } all = all->next; free(tmpbuf); } all = tmp; return; } int main(argc, argv) int argc; char *argv[]; { char ch; int aflag, sflag, i; aflag = sflag = 0; if(argc < 2) return 1; if(!strncmp(argv[1],"-h",2) || !strncmp(argv[1],"--h",3) || !strcmp(argv[1],"?")) { printusage(argv[0]); return 1; } while((ch = getopt(argc,argv,"as")) != -1) { switch(ch) { case 'a': aflag = 1; break; case 's': sflag = 1; break; default: printusage(argv[0]); return 1; } } argc -= optind; argv += optind; findpath(); for(i = 0; i < argc; i++) findprog(argv[i],aflag,sflag); losepath(); return sflag ? found : 0; } On Fri, Dec 14, 2001 at 06:19:30AM -0500, Robert Watson wrote: > Use eaccess() instead of rolling your own. The comment in access(2) is > incredibly mis-leading, and sometime I'll get around to fixing it. I may > already have in -CURRENT. > > The security risk comes from two things: > > (1) Many consumers don't realize it uses ruid/rgid instead of euid/egid. > (2) Improper use can lend this call to race conditions. Your use is not > improper. > > I should really MFC the eaccess() code, and may get to it tomorrow. > > Robert N M Watson FreeBSD Core Team, TrustedBSD Project > robert@fledge.watson.org NAI Labs, Safeport Network Services > > On Thu, 13 Dec 2001, Peter Sanchez wrote: > > > The following reply was made to PR bin/32807; it has been noted by GNATS. > > > > From: Peter Sanchez > > To: David Hill , > > freebsd-gnats-submit@FreeBSD.org > > Cc: > > Subject: Re: bin/32807: which utility replacement in C > > Date: Thu, 13 Dec 2001 13:56:33 -0700 > > > > Now I feel like an idiot. I knew about that bug and simple forgot about > > it. Here is the updated source. Sorry about that. > > > > Peter > > > > ----------------------------------------------------------------------- > > > > #include > > #include > > #include > > #include > > #include > > #include > > #include > > > > #define NGROUPS 15 > > > > struct pathinfo > > { > > char *path; > > struct pathinfo *next; > > }; > > > > struct pathinfo *all = NULL; > > struct passwd *pw; > > uid_t uid; > > int found = 1, gfail = 0; > > int groups[NGROUPS + 1], ngroups = (NGROUPS + 1); > > > > void > > printusage(bin) > > char *bin; > > { > > fprintf(stderr,"usage: %s [-a] [-s] program ...\n",bin); > > return; > > } > > > > int > > file_exists(file) > > char *file; > > { > > struct stat info; > > int check, i; > > > > check = stat(file,&info); > > if(check == -1) > > return check; /* file doesnt exist */ > > > > if(S_ISDIR(info.st_mode)) > > return -1; /* file is a directory */ > > > > /* > > * I did not use access() here cause of man page warnings that > > * it is a potential security risk and shoule NEVER be used. Not > > * quit sure on how it can be a security risk, but I worked around > > * it anyways. > > */ > > if(info.st_uid == uid && info.st_mode & S_IFREG && info.st_mode & S_IXUSR) > > return check; /* user executable */ > > > > if(gfail) > > { > > if(info.st_gid == pw->pw_gid && info.st_mode & S_IFREG && > > info.st_mode & S_IXGRP) > > return check; /* group executable */ > > } > > else > > { > > for(i = 0; i < ngroups; i++) > > { > > if(info.st_gid == groups[i] && info.st_mode & S_IFREG && > > info.st_mode & S_IXGRP) > > return check; /* group executable */ > > } > > } > > > > if(info.st_mode & S_IFREG && info.st_mode & S_IXOTH) > > return check; /* other executable */ > > else > > return -1; > > } > > > > void > > losepath(void) > > { > > struct pathinfo *tmp; > > > > while(all != NULL) > > { > > tmp = all->next; > > free(all); > > all = tmp; > > } > > return; > > } > > > > void > > findpath(void) > > { > > struct pathinfo *cur = NULL; > > char *userpath = getenv("PATH"); > > int i, x; > > > > if(userpath == NULL) > > exit(1); > > > > all = (struct pathinfo *)malloc((unsigned)sizeof(struct pathinfo)); > > if(all == NULL) > > { > > fprintf(stderr,"Out of memory, malloc() failed!\n"); > > exit(1); > > } > > cur = all; > > > > while((cur->path = strsep(&userpath,":")) != NULL) > > { > > cur->next = (struct pathinfo *)malloc((unsigned)sizeof(struct pathinfo)); > > if(cur->next == NULL) > > { > > losepath(); > > fprintf(stderr,"Out of memory, malloc() failed!\n"); > > exit(1); > > } > > cur = cur->next; > > } > > cur->next = NULL; > > cur = all; > > return; > > } > > > > void > > findprog(prog, aflag, sflag) > > char *prog; > > int aflag; > > int sflag; > > { > > struct pathinfo *tmp; > > char tmpbuf[2048]; > > > > tmp = all; > > while(all != NULL) > > { > > if(strchr(prog,'/')) > > strncpy(tmpbuf,prog,2048); > > else > > snprintf(tmpbuf,2048,"%s/%s",all->path,prog); > > > > if(!file_exists(tmpbuf)) > > { > > found = 0; > > if(sflag && aflag) ; > > else if(sflag && !aflag) > > { > > all = tmp; > > return; > > } > > else if(aflag && !sflag) > > printf("%s\n",tmpbuf); > > else > > { > > printf("%s\n",tmpbuf); > > all = tmp; > > return; > > } > > } > > all = all->next; > > } > > all = tmp; > > return; > > } > > > > int > > main(argc, argv) > > int argc; > > char *argv[]; > > { > > char buf[1024]; > > int aflag, sflag, pass, i; > > > > aflag = sflag = pass = 0; > > if(argc < 2) > > return 1; > > > > if(!strncmp(argv[1],"-h",2) || !strncmp(argv[1],"--h",3) || > > !strcmp(argv[1],"?")) > > { > > printusage(argv[0]); > > return 1; > > } > > > > uid = getuid(); > > pw = getpwuid(uid); > > if(getgrouplist(pw->pw_name,pw->pw_gid,groups,&ngroups) == -1) > > gfail = 1; > > > > findpath(); > > for(i = 1; i < argc; i++) > > { > > if(!pass && *argv[i] == '-') > > { > > if(!strcmp(argv[i],"-a")) > > aflag = 1; > > else if(!strcmp(argv[i],"-s")) > > sflag = 1; > > else > > { > > printusage(argv[0]); > > return 1; > > } > > continue; > > } > > > > pass = 1; > > strncpy(buf,argv[i],1024); > > findprog(buf,aflag,sflag); > > } > > > > losepath(); > > return sflag ? found : 0; > > } > > > > > > On Thu, Dec 13, 2001 at 01:03:45PM -0500, David Hill wrote: > > > > > > > > > -----Original Message----- > > > From: owner-freebsd-bugs@FreeBSD.ORG > > > [mailto:owner-freebsd-bugs@FreeBSD.ORG]On Behalf Of Peter Sanchez > > > Sent: Thursday, December 13, 2001 12:39 PM > > > To: freebsd-gnats-submit@FreeBSD.org > > > Subject: bin/32807: which utility replacement in C > > > > > > > > > > > > >Number: 32807 > > > >Category: bin > > > >Synopsis: which utility replacement in C > > > >Confidential: no > > > >Severity: non-critical > > > >Priority: low > > > >Responsible: freebsd-bugs > > > >State: open > > > >Quarter: > > > >Keywords: > > > >Date-Required: > > > >Class: change-request > > > >Submitter-Id: current-users > > > >Arrival-Date: Thu Dec 13 09:40:00 PST 2001 > > > >Closed-Date: > > > >Last-Modified: > > > >Originator: Peter Sanchez > > > >Release: 4.4-STABLE > > > >Organization: > > > >Environment: > > > FreeBSD char.linuxforlesbians.org 4.4-STABLE FreeBSD 4.4-STABLE #0: Thu > > > Nov 22 14:19:15 MST 2001 > > > root@char.linuxforlesbians.org:/usr/src/sys/compile/CHAR i386 > > > >Description: > > > Just a simple C program to replace the perl which utility that comes > > > default at /usr/bin/which (/usr/src/usr.bin/which). Its slighly faster > > > than the perl version. > > > >How-To-Repeat: > > > N/A > > > >Fix: > > > #include > > > #include > > > #include > > > #include > > > #include > > > #include > > > #include > > > > > > #define NGROUPS 15 > > > > > > struct pathinfo > > > { > > > char path[1024]; > > > struct pathinfo *next; > > > }; > > > > > > struct pathinfo *all = NULL; > > > struct passwd *pw; > > > uid_t uid; > > > int found = 1, gfail = 0; > > > int groups[NGROUPS + 1], ngroups = (NGROUPS + 1); > > > > > > void > > > printusage(bin) > > > char *bin; > > > { > > > fprintf(stderr,"usage: %s [-a] [-s] program ...\n",bin); > > > return; > > > } > > > > > > int > > > file_exists(file) > > > char *file; > > > { > > > struct stat info; > > > int check, i; > > > > > > check = stat(file,&info); > > > if(check == -1) > > > return check; /* file doesnt exist */ > > > > > > if(S_ISDIR(info.st_mode)) > > > return -1; /* file is a directory */ > > > > > > /* > > > * I did not use access() here cause of man page warnings that > > > * it is a potential security risk and shoule NEVER be used. Not > > > * quit sure on how it can be a security risk, but I worked around > > > * it anyways. > > > */ > > > if(info.st_uid == uid && info.st_mode & S_IFREG && info.st_mode & > > > S_IXUSR) > > > return check; /* user executable */ > > > > > > if(gfail) > > > { > > > if(info.st_gid == pw->pw_gid && info.st_mode & S_IFREG && > > > info.st_mode & S_IXGRP) > > > return check; /* group executable */ > > > } > > > else > > > { > > > for(i = 0; i < ngroups; i++) > > > { > > > if(info.st_gid == groups[i] && info.st_mode & S_IFREG && > > > info.st_mode & S_IXGRP) > > > return check; /* group executable */ > > > } > > > } > > > > > > if(info.st_mode & S_IFREG && info.st_mode & S_IXOTH) > > > return check; /* other executable */ > > > else > > > return -1; > > > } > > > > > > void > > > losepath(void) > > > { > > > struct pathinfo *tmp; > > > > > > while(all != NULL) > > > { > > > tmp = all->next; > > > free(all); > > > all = tmp; > > > } > > > return; > > > } > > > > > > void > > > findpath(void) > > > { > > > struct pathinfo *cur = NULL; > > > char *userpath = getenv("PATH"); > > > int i, x; > > > > > > if(userpath == NULL) > > > exit(1); > > > > > > all = (struct pathinfo *)malloc((unsigned)sizeof(struct pathinfo)); > > > if(all == NULL) > > > { > > > fprintf(stderr,"Out of memory, malloc() failed!\n"); > > > exit(1); > > > } > > > cur = all; > > > > > > for(i = 0, x = 0; i < strlen(userpath); i++) > > > { > > > if(userpath[i] == ':') > > > { > > > cur->path[x] = '\0'; > > > x = 0; > > > cur->next = (struct pathinfo > > > *)malloc((unsigned)sizeof(struct pathinfo)); > > > if(cur->next == NULL) > > > { > > > losepath(); > > > fprintf(stderr,"Out of memory, malloc() failed!\n"); > > > exit(1); > > > } > > > cur = cur->next; > > > } > > > else > > > cur->path[x++] = userpath[i]; > > > } > > > cur->path[x] = '\0'; > > > cur->next = NULL; > > > cur = all; > > > return; > > > } > > > > > > void > > > findprog(prog, aflag, sflag) > > > char *prog; > > > int aflag; > > > int sflag; > > > { > > > struct pathinfo *tmp; > > > char tmpbuf[2048]; > > > > > > tmp = all; > > > while(all != NULL) > > > { > > > if(strchr(prog,'/')) > > > strncpy(tmpbuf,prog,2048); > > > else > > > snprintf(tmpbuf,2048,"%s/%s",all->path,prog); > > > > > > if(!file_exists(tmpbuf)) > > > { > > > found = 0; > > > if(sflag && aflag) ; > > > else if(sflag && !aflag) > > > { > > > all = tmp; > > > return; > > > } > > > else if(aflag && !sflag) > > > printf("%s\n",tmpbuf); > > > else > > > { > > > printf("%s\n",tmpbuf); > > > all = tmp; > > > return; > > > } > > > } > > > all = all->next; > > > } > > > all = tmp; > > > return; > > > } > > > > > > int > > > main(argc, argv) > > > int argc; > > > char *argv[]; > > > { > > > char buf[1024]; > > > int aflag, sflag, pass, i; > > > > > > aflag = sflag = pass = 0; > > > if(argc < 2) > > > return 1; > > > > > > if(!strncmp(argv[1],"-h",2) || !strncmp(argv[1],"--h",3) || > > > !strcmp(argv[1],"?")) > > > { > > > printusage(argv[0]); > > > return 1; > > > } > > > > > > uid = getuid(); > > > pw = getpwuid(uid); > > > if(getgrouplist(pw->pw_name,pw->pw_gid,groups,&ngroups) == -1) > > > gfail = 1; > > > > > > findpath(); > > > for(i = 1; i < argc; i++) > > > { > > > if(!pass && *argv[i] == '-') > > > { > > > if(!strcmp(argv[i],"-a")) > > > aflag = 1; > > > else if(!strcmp(argv[i],"-s")) > > > sflag = 1; > > > else > > > { > > > printusage(argv[0]); > > > return 1; > > > } > > > continue; > > > } > > > > > > pass = 1; > > > strncpy(buf,argv[i],1024); > > > findprog(buf,aflag,sflag); > > > } > > > > > > losepath(); > > > return sflag ? found : 0; > > > } > > > >Release-Note: > > > >Audit-Trail: > > > >Unformatted: > > > > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > > > with "unsubscribe freebsd-bugs" in the body of the message > > > > > > --- > > > > > > Needs a little work. If my PATH env variable contains a "path" larger > > > than 1024, this app seg faults. > > > > > > $ export PATH=$PATH:`perl -e "print '/bin/','x' x 4048"` > > > $ ./which ls > > > Segmentation fault (core dumped) > > > > > > - David > > > > > > --- > > > Outgoing mail is certified Virus Free. > > > Checked by AVG anti-virus system (http://www.grisoft.com). > > > Version: 6.0.303 / Virus Database: 164 - Release Date: 11/24/2001 > > > > > > > -- > > Peter Sanchez, aka fut0n | "The ability to read is what > > - fut0n@linuxforlesbians.org | distinguishes Unix users from > > - www.linuxforlesbians.org | those of more popular platforms." > > - FreeBSD or DIE | - John Lasser > > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > > with "unsubscribe freebsd-bugs" in the body of the message > > -- Peter Sanchez, aka fut0n | "The ability to read is what - fut0n@linuxforlesbians.org | distinguishes Unix users from - www.linuxforlesbians.org | those of more popular platforms." - FreeBSD or DIE | - John Lasser To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message