Date: Fri, 14 Dec 2001 07:00:38 -0700 From: Peter Sanchez <fut0n@linuxforlesbians.org> To: Robert Watson <rwatson@FreeBSD.org> Cc: freebsd-bugs@freebsd.org Subject: Re: bin/32807: which utility replacement in C Message-ID: <20011214070038.B38914@sushi.linuxforlesbians.org> In-Reply-To: <Pine.NEB.3.96L.1011214061809.83145A-100000@fledge.watson.org>; from rwatson@FreeBSD.org on Fri, Dec 14, 2001 at 06:19:30AM -0500 References: <200112132100.fBDL01r64612@freefall.freebsd.org> <Pine.NEB.3.96L.1011214061809.83145A-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <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;
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 <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
> >
--
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011214070038.B38914>