Date: Mon, 3 Sep 2001 10:59:34 +0300 From: Ruslan Ermilov <ru@FreeBSD.ORG> To: Kris Kennaway <kris@obsecurity.org> Cc: audit@FreeBSD.ORG Subject: Re: issetugid checks revisited Message-ID: <20010903105934.C49997@sunbay.com> In-Reply-To: <20010902225445.A27902@xor.obsecurity.org>; from kris@obsecurity.org on Sun, Sep 02, 2001 at 10:54:45PM -0700 References: <20010902225445.A27902@xor.obsecurity.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Sep 02, 2001 at 10:54:45PM -0700, Kris Kennaway wrote:
> I posted a broken version of this a few weeks ago. I think this
> updated version fixes all of the bugs..reviews, please?
>
issetugid() is boolean, no need for ``!= 0''.
> Index: lib/libc/rpc/getnetpath.c
>
Missing ``include <unistd.h>''.
> Index: lib/libc_r/uthread/uthread_info.c
>
[...]
> +#include <errno.h>
> +#include <fcntl.h>
> #include <stdio.h>
> #include <stdlib.h>
> -#include <fcntl.h>
> #include <string.h>
> -#include <unistd.h>
> +#include <paths.h>
> #include <pthread.h>
> -#include <errno.h>
> +#include <unistd.h>
> #include "pthread_private.h"
>
Sort includes in a separate commit?
[...]
> - char tmpfile[128];
> + char *tmpdir;
> + char tmpfile[PATH_MAX];
>
Missing `const' qualifier.
> Index: lib/libcompat/4.3/rexec.c
>
[...]
> @@ -144,8 +145,15 @@
> char myname[MAXHOSTNAMELEN], *mydomain;
> int t, i, c, usedefault = 0;
> struct stat stb;
> + struct passwd *pwd;
>
> - hdir = getenv("HOME");
> + if (issetugid() != 0 || (hdir = getenv("HOME")) == NULL) {
> + pwd = getpwuid(getuid());
> + if (pwd == NULL)
> + return (0);
> + hdir = pwd->pw_dir;
> + }
> +
> if (hdir == NULL)
> hdir = ".";
> if (strlen(hdir) + 8 > sizeof(buf))
>
Hmm, you are changing the semantics of the ruserpass() function,
even if it's not setugid, and the HOME variable isn't set. Is
this intentional?
> Index: gnu/lib/libdialog/rc.c
> ===================================================================
> - unsigned char str[MAX_LEN+1], *var, *value, *tempptr;
> - FILE *rc_file;
> + unsigned char str[MAX_LEN+1], *var, *value, *tempptr = NULL;
> + FILE *rc_file = NULL;
>
> /*
> *
> @@ -103,12 +103,12 @@
> *
> */
>
> - if ((tempptr = getenv("DIALOGRC")) != NULL)
> + if (issetugid() == 0 && (tempptr = getenv("DIALOGRC")) != NULL)
> rc_file = fopen(tempptr, "rt");
>
How about:
else
tempptr = rc_file = NULL;
instead on initializations above?
Cheers,
--
Ruslan Ermilov Oracle Developer/DBA,
ru@sunbay.com Sunbay Software AG,
ru@FreeBSD.org FreeBSD committer,
+380.652.512.251 Simferopol, Ukraine
http://www.FreeBSD.org The Power To Serve
http://www.oracle.com Enabling The Information Age
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?20010903105934.C49997>