Date: Mon, 7 Jun 1999 10:30:05 -0700 (PDT) From: Dag-Erling Smorgrav <des@flood.ping.uio.no> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/12052: sh type builtin appends first path component to absolute result Message-ID: <199906071730.KAA34575@freefall.freebsd.org>
index | next in thread | raw e-mail
The following reply was made to PR bin/12052; it has been noted by GNATS.
From: Dag-Erling Smorgrav <des@flood.ping.uio.no>
To: matt@zigg.com
Cc: FreeBSD-gnats-submit@FreeBSD.ORG
Subject: Re: bin/12052: sh type builtin appends first path component to absolute result
Date: 07 Jun 1999 19:28:36 +0200
matt@zigg.com writes:
> >Fix:
No fix yet, but an analysis: since the argument to 'type' (typecmd()
in src/bin/sh/exec.c) is neither a builtin nor an alias, typecmd()
calls find_command() (also in src/bin/sh/exec.c). The first thing
find_command() does is check if the command contains a slash, in which
case it sets the command type to CMDNORMAL and the path index to 0:
/* If name contains a slash, don't use the hash table */
if (strchr(name, '/') != NULL) {
entry->cmdtype = CMDNORMAL;
entry->u.index = 0;
return;
}
Here's the hitch - the caller has no way of distinguishing "a normal
command which resides in the first directory in the path" from "a
normal command which contains slashes". It tries to construct a full
command name by calling padvance() entry->u.index times:
case CMDNORMAL: {
int j = entry.u.index;
char *path = pathval(), *name;
do {
name = padvance(&path, argv[i]);
stunalloc(name);
out1fmt("%s\n", name);
} while (--j >= 0);
out1fmt(" is%s %s\n",
cmdp ? " a tracked alias for" : "", name);
break;
}
This seems to me an extremely awkward way of concatenating two
strings... but never mind.
To summarize, there are (at least) two bugs in the way find_command()
treats command names with slashes in them:
* it sets entry->u.index to 0, thus telling the caller that the
command is in the first directory in the path
* it does not check that the command actually exists and is
executable by the current user, which results in the following:
des@des ~% sh -c 'type this/file/does/not/exist'
this/file/does/not/exist is /usr/home/des/bin/this/file/does/not/exist
It is my humble opinion that the root of this problem lies with the
use of padvance(), which is a horrible hackery. I would modify
padvance() to take a numeric argument indicating which path component
to use, rather than have it modify the path pointer, and to interpret
-1 to mean "do not prepend anything at all". I would also modify it so
it does not use a global variable (pathopt).
DES
--
Dag-Erling Smorgrav - des@flood.ping.uio.no
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199906071730.KAA34575>