Date: Sun, 19 Aug 2001 15:29:27 -0400 From: Mike Barcroft <mike@FreeBSD.org> To: Kris Kennaway <kris@obsecurity.org> Cc: audit@FreeBSD.org Subject: Re: Checking issetugid() with getenv() in libraries Message-ID: <20010819152927.A49129@coffee.q9media.com> In-Reply-To: <20010818221258.A79194@xor.obsecurity.org>; from kris@obsecurity.org on Sat, Aug 18, 2001 at 10:12:58PM -0700 References: <20010818221258.A79194@xor.obsecurity.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Kris Kennaway <kris@obsecurity.org> writes:
> There were a number of places where library routines blindly use
> getenv() in ways which may be insecure if called from setugid code.
> Please review the following.
>
> I also changed the uthread_info.c to respect TMPDIR if !issetugid()
> instead of dumping to /tmp always.
>
> Kris
>
> Index: libc/db/test/dbtest.c
> ===================================================================
> RCS file: /mnt/ncvs/src/lib/libc/db/test/dbtest.c,v
> retrieving revision 1.4
> diff -u -r1.4 dbtest.c
> --- libc/db/test/dbtest.c 2000/08/04 10:50:21 1.4
> +++ libc/db/test/dbtest.c 2001/08/19 04:25:47
> @@ -155,7 +155,8 @@
> * want it around, and it often screws up tests.
> */
> if (fname == NULL) {
> - p = getenv("TMPDIR");
> + if (issetugid() == 0)
> + p = getenv("TMPDIR");
> if (p == NULL)
> p = "/var/tmp";
> (void)snprintf(buf, sizeof(buf), "%s/__dbtest", p);
[...]
Is p initialized to NULL above this? If not, p could be uninitialized
when issetugid() != 0.
> Index: libc/stdio/tmpfile.c
> ===================================================================
> RCS file: /mnt/ncvs/src/lib/libc/stdio/tmpfile.c,v
> retrieving revision 1.6
> diff -u -r1.6 tmpfile.c
> --- libc/stdio/tmpfile.c 2001/07/07 04:08:32 1.6
> +++ libc/stdio/tmpfile.c 2001/08/19 04:19:53
> @@ -61,7 +61,8 @@
> char *buf;
> const char *tmpdir;
>
> - tmpdir = getenv("TMPDIR");
> + if (issetugid() == 0)
> + tmpdir = getenv("TMPDIR");
> if (tmpdir == NULL)
> tmpdir = _PATH_TMP;
>
[...]
There's a similar problem here.
> Index: libncp/ncpl_rcfile.c
> ===================================================================
> RCS file: /mnt/ncvs/src/lib/libncp/ncpl_rcfile.c,v
> retrieving revision 1.3
> diff -u -r1.3 ncpl_rcfile.c
> --- libncp/ncpl_rcfile.c 2000/05/26 02:00:20 1.3
> +++ libncp/ncpl_rcfile.c 2001/08/19 04:52:39
> @@ -390,7 +390,8 @@
> char *home, *fn;
> int error;
>
> - home = getenv("HOME");
> + if (issetugid() == 0)
> + home = getenv("HOME");
> if (home) {
> fn = malloc(strlen(home) + 20);
> sprintf(fn, "%s/.nwfsrc", home);
[...]
Also here.
The rest of the changes look good.
Best regards,
Mike Barcroft
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?20010819152927.A49129>