Date: Mon, 17 Jan 2000 13:03:25 +1100 From: Peter Jeremy <peter.jeremy@alcatel.com.au> To: Kris Kennaway <kris@hub.freebsd.org> Cc: audit@FreeBSD.ORG Subject: Re: awk tempfile handling Message-ID: <00Jan17.130325est.40324@border.alcanet.com.au> In-Reply-To: <Pine.BSF.4.21.0001161707440.18027-100000@hub.freebsd.org>; from kris@hub.freebsd.org on Mon, Jan 17, 2000 at 12:01:00PM %2B1100 References: <Pine.BSF.4.21.0001161707440.18027-100000@hub.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2000-Jan-17 12:01:00 +1100, Kris Kennaway <kris@hub.freebsd.org> wrote:
>+ char *name = "/tmp/pipXXXXXXXXXX";
No good. mkstemp() updates the passed string in place (see the notes
in mktemp(3). The above defines a read-only string. This leads to
segmentation violations at runtime.
> static char cmdbuf[256];
Into which you're sprintf()ing an unbounded string. Not a good idea.
>- if ((name = tempnam(".", "pip")) == NULL)
>+ if ((current = mkstemp(name)) == INVALID_HANDLE)
> return NULL;
tempnam() returns a malloc'd buffer, mkstemp() reuses the passed name.
This will cause problems if `name' is later free()d.
> sprintf(cmdbuf, "%s > %s", cmd, name);
> system(cmdbuf);
system(3) is one of those system calls which is virtually impossible to
use safely. There's also a race condition here - there's nothing stopping
another process doing something nasty between the mkstemp() and the
I/O redirection done by the shell within system().
The following is an outline of what needs to be done:
{
static char name[] = "/tmp/pipXXXXXXXXXX";
int current; /* FD for stdout from command */
pid_t pid;
int status;
if ((current = mkstemps(name, 10)) == INVALID_HANDLE)
return NULL;
if ((pid = fork()) < 0)
return NULL;
else if (pid == 0) {
if (current != 1) {
if (dup2(current, 1) < 0)
_exit(1);
close(current);
}
/*XXX close other random FD's and/or set close-on-exec flags */
/*XXX If security is an issue, don't call sh at all */
execl("/bin/sh", "sh", "-c", cmd, NULL);
_exit(1);
} else {
while (waitpid(pid, &status, 0) < 0 && errno == EINTR)
;
}
pipes[current].name = strdup(name);
pipes[current].command = strdup(cmd);
rp->iop = iop_alloc(current, name, NULL);
...
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?00Jan17.130325est.40324>