Date: Fri, 14 Dec 2001 06:19:30 -0500 (EST) From: Robert Watson <rwatson@FreeBSD.org> To: Peter Sanchez <fut0n@linuxforlesbians.org> Cc: freebsd-bugs@FreeBSD.org Subject: Re: bin/32807: which utility replacement in C Message-ID: <Pine.NEB.3.96L.1011214061809.83145A-100000@fledge.watson.org> In-Reply-To: <200112132100.fBDL01r64612@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <fut0n@linuxforlesbians.org>
> To: David Hill <david@visual-network.net>,
> 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 <stdio.h>
> #include <string.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <pwd.h>
>
> #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 <stdio.h>
> > #include <string.h>
> > #include <stdlib.h>
> > #include <unistd.h>
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > #include <pwd.h>
> >
> > #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
>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1011214061809.83145A-100000>