From owner-freebsd-audit  Mon Sep  3  1: 0: 5 2001
Delivered-To: freebsd-audit@freebsd.org
Received: from whale.sunbay.crimea.ua (whale.sunbay.crimea.ua [212.110.138.65])
	by hub.freebsd.org (Postfix) with ESMTP id D38EC37B406
	for <audit@FreeBSD.ORG>; Mon,  3 Sep 2001 00:59:55 -0700 (PDT)
Received: (from ru@localhost)
	by whale.sunbay.crimea.ua (8.11.2/8.11.2) id f837xYg57495;
	Mon, 3 Sep 2001 10:59:34 +0300 (EEST)
	(envelope-from ru)
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>
References: <20010902225445.A27902@xor.obsecurity.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
In-Reply-To: <20010902225445.A27902@xor.obsecurity.org>; from kris@obsecurity.org on Sun, Sep 02, 2001 at 10:54:45PM -0700
Sender: owner-freebsd-audit@FreeBSD.ORG
Precedence: bulk
List-ID: <freebsd-audit.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20freebsd-audit>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20freebsd-audit>
X-Loop: FreeBSD.ORG

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