NetBSD Problem Report #18936

Received: (qmail 4656 invoked by uid 605); 5 Nov 2002 02:32:30 -0000
Message-Id: <20021105033229.47baf3bb.christianbiere@gmx.de>
Date: Tue, 5 Nov 2002 03:32:29 +0100
From: Christian Biere <christianbiere@gmx.de>
Sender: gnats-bugs-owner@netbsd.org
To: gnats-bugs@gnats.netbsd.org
Subject: syslogd should call initgroups()

>Number:         18936
>Category:       bin
>Synopsis:       syslogd should call initgroups()
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Nov 05 02:33:00 +0000 2002
>Closed-Date:    
>Last-Modified:  Sun Nov 30 13:22:00 +0000 2003
>Originator:     Christian Biere
>Release:        NetBSD 1.6J
>Organization:

>Environment:
NetBSD localhost 1.6J NetBSD 1.6J (UNICRON) #5: Mon Oct 28 17:05:26 CET
2002     root@localhost:/usr/src/sys/arch/i386/compile/UNICRON i386

>Description:
/usr/sbin/syslogd should call initgroups() before set{e,}{g,u}gid() like
almost(?) any other daemon to get rid of unnecessary rights.

>How-To-Repeat:

>Fix:
See my patch for a suggestion.


--Multipart_Tue__5_Nov_2002_03:32:29_+0100_08170e00
Content-Type: text/plain;
 name="syslogd.c.udif"
Content-Disposition: attachment;
 filename="syslogd.c.udif"
Content-Transfer-Encoding: 7bit

Index: syslogd.c
===================================================================
RCS file: /cvsroot/basesrc/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.56
diff -u -r1.56 syslogd.c
--- syslogd.c	2002/09/24 13:53:54	1.56
+++ syslogd.c	2002/11/05 02:16:24
@@ -308,7 +308,19 @@
 				die (0);
 			}
 		}
+	} else {
+		if ((pw = getpwuid(getuid())) != NULL) {
+			if ((user = strdup(pw->pw_name)) == NULL) {
+				logerror("Could not allocate user");
+			 	die (0);
+			}
+		} else {
+			errno = 0;  
+			logerror("Cannot find UID `%d'", getuid());
+			die (0);
+		}
 	}
+	

 	if (group != NULL) {
 		if (isdigit((unsigned char)*group)) {
@@ -429,6 +441,12 @@
 	dprintf("Attempt to chroot to `%s'\n", root);  
 	if (chroot(root)) {
 		logerror("Failed to chroot to `%s'", root);
+		die(0);
+	}
+	dprintf("Attempt to initgroups for `%s' with GID `%d'\n", user, gid);  
+	if (initgroups(user, gid)) {
+		logerror("Failed to initgroups for `%s' with GID `%d'", user, 
+			gid);
 		die(0);
 	}
 	dprintf("Attempt to set GID/EGID to `%d'\n", gid);  

--Multipart_Tue__5_Nov_2002_03:32:29_+0100_08170e00--
>Release-Note:
>Audit-Trail:

From: woods@weird.com (Greg A. Woods)
To: Christian Biere <christianbiere@gmx.de>
Cc: gnats-bugs@gnats.netbsd.org (NetBSD GNATS submissions and followups),
  netbsd-bugs@NetBSD.ORG (NetBSD Bugs and PR posting List)
Subject: Re: bin/18936: syslogd should call initgroups()
Date: Tue,  5 Nov 2002 12:23:52 -0500 (EST)

 [ On Tuesday, November 5, 2002 at 03:32:29 (+0100), Christian Biere wrote: ]
 > Subject: bin/18936: syslogd should call initgroups()
 >
 > 
 > /usr/sbin/syslogd should call initgroups() before set{e,}{g,u}gid() like
 > almost(?) any other daemon to get rid of unnecessary rights.

 No, not initgroups(), just setgroups(), and perhaps only if started by
 root (in theory syslogd can be started as non-root):

 	if (uid == 0) {
 		gid_t gid = getgid();

 		setgroups(1, &gid);
 	}

 -- 
 								Greg A. Woods

 +1 416 218-0098;            <g.a.woods@ieee.org>;           <woods@robohack.ca>
 Planix, Inc. <woods@planix.com>; VE3TCP; Secrets of the Weird <woods@weird.com>

From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@gnats.netbsd.org
Cc: netbsd-bugs@NetBSD.ORG, "Greg A. Woods" <woods@weird.com>
Subject: Re: bin/18936: syslogd should call initgroups()
Date: Wed, 19 Nov 2003 23:38:00 +0100

 No, setgroups() would be appropriate if syslogd was started with -g GID. Yes,
 then syslogd() should do "gid_t gid = GID; setgroups(1, &gid);". Otherwise,
 if syslogd() was started without -g, it should call initgroups(). In this
 case you want to control the group permissions by /etc/{passwd,groups} etc.
 and it might be necessary to put syslogd into more than one group.

 BTW, in your example, syslogd would have wheel as GID, right? That's
 probably not desired.

 -- 
 Christian

From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@gnats.netbsd.org
Cc:  
Subject: Re: bin/18936: syslogd should call initgroups()
Date: Sun, 30 Nov 2003 10:05:49 +0100

 I've modified syslogd so that it calls initgroups(). In case you use
 the parameter -g GID, syslogd will use this GID for setgid(). Further, it's
 now possible to run syslogd as a normal user in "secure" mode as long as
 this user can access the log files and devices.[1]
 I've also modified syslogd.c so that control characters will be
 escaped. The code for the function sanitize() was already used by
 printline(). I changed it to not strip the 8th bit and let logmsg()
 call this function.

 [1]: I've created user and group "syslogd", added syslogd to group
 daemon and made /dev/console writable for group syslogd. I use
 only the parameters -s and -n.

From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@gnats.netbsd.org
Cc:  
Subject: Re: bin/18936: syslogd should call initgroups()
Date: Sun, 30 Nov 2003 10:06:56 +0100

 Index: syslogd.c
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 RCS file: /cvsroot/src/usr.sbin/syslogd/syslogd.c,v
 retrieving revision 1.63
 diff -u -r1.63 syslogd.c
 --- syslogd.c	2003/10/17 01:39:25	1.63
 +++ syslogd.c	2003/11/30 08:22:44
 @@ -208,6 +208,7 @@
  int	getmsgbufsize(void);
  int*	socksetup(int);
  void	init(int);
 +char   *sanitize(char *dst, char *src, size_t len);
  void	logerror(const char *, ...);
  void	logmsg(int, char *, char *, int);
  void	printline(char *, char *);
 @@ -229,11 +230,11 @@
  	struct sockaddr_storage frominet;
  	char *p, *line, **pp;
  	struct pollfd *readfds;
 -	uid_t uid =3D 0;
 -	gid_t gid =3D 0;
 +	uid_t uid =3D getuid();
 +	gid_t gid =3D getgid();
  	char *user =3D NULL;
  	char *group =3D NULL;
 -	char *root =3D "/";
 +	char *root =3D NULL;
  	char *endp;
  	struct group   *gr;
  	struct passwd  *pw;
 @@ -255,7 +256,16 @@
  				usage();
  			break;
  		case 'm':		/* mark interval */
 -			MarkInterval =3D atoi(optarg) * 60;
 +			if (!isdigit((u_char)*optarg))
 +				usage();
 +			errno =3D 0;
 +			endp =3D NULL;
 +			l =3D strtoul(optarg, &endp, 10);
 +			if (errno || *endp !=3D '\0' || l > INT_MAX / 60) {
 +				logerror("Invalid value for -m");
 +				die(0);
 +			}
 +			MarkInterval =3D l * 60;
  			break;
  		case 'n':		/* turn off DNS queries */
  			UseNameService =3D 0;
 @@ -315,8 +325,21 @@
  				die (0);
  			}
  		}
 +	} else {
 +		if ((pw =3D getpwuid(getuid())) !=3D NULL) {
 +			logerror("getpwuid failed");
 +			die(0);
 +		}
  	}
 =20
 +	if (getuid() =3D=3D 0) {
 +		if (initgroups(pw->pw_name, pw->pw_gid) =3D=3D -1) {
 +			logerror("initgroups failed for GID `%ld'",
 +				(long) pw->pw_gid);
 +			die(0);
 +		}
 +	}
 +
  	if (group !=3D NULL) {
  		if (isdigit((unsigned char)*group)) {
  			errno =3D 0;
 @@ -342,7 +365,7 @@
  		}
  	}
 =20
 -	if (access (root, F_OK | R_OK)) {
 +	if (root !=3D NULL && access (root, F_OK | R_OK)) {
  		logerror("Cannot access `%s'", root);
  		die (0);
  	}
 @@ -442,10 +465,12 @@
  	/*=20
  	 * All files are open, we can drop privileges and chroot
  	 */
 -	dprintf("Attempt to chroot to `%s'\n", root); =20
 -	if (chroot(root)) {
 -		logerror("Failed to chroot to `%s'", root);
 -		die(0);
 +	if (root !=3D NULL) {
 +		dprintf("Attempt to chroot to `%s'\n", root); =20
 +		if (chroot(root)) {
 +			logerror("Failed to chroot to `%s'", root);
 +			die(0);
 +		}
  	}
  	dprintf("Attempt to set GID/EGID to `%d'\n", gid); =20
  	if (setgid(gid) || setegid(gid)) {
 @@ -456,7 +481,7 @@
  	if (setuid(uid) || seteuid(uid)) {
  		logerror("Failed to set uid to `%d'", uid);
  		die(0);
 -	}
 +	}=09
 =20
  	/*=20
  	 * We cannot detach from the terminal before we are sure we won't=20
 @@ -619,21 +644,58 @@
  }
 =20
  /*
 + * Copies src to dst while escaping control characters. The string in
 + * src will be truncated if the escaped string exceeds len - 2.
 + * Returns src if no escaping was necessary and dst otherwise.
 + */
 +char *
 +sanitize(char *dst, char *src, size_t len)
 +{
 +	char *p, *q;
 +	int c;
 +
 +	for (p =3D src; *p; p++) {
 +		if (iscntrl((u_char)*p))
 +			break;
 +	}
 +	if (*p =3D=3D '\0')
 +		return src;
 +
 +	p =3D src;
 +	q =3D dst;
 +	while ((c =3D (u_char)*p++) !=3D '\0' && q < &dst[len - 2]) {
 +		if (iscntrl(c))
 +			if (c =3D=3D '\n')
 +				*q++ =3D ' ';
 +			else if (c =3D=3D '\t')
 +				*q++ =3D '\t';
 +			else {
 +				*q++ =3D '^';
 +				*q++ =3D c ^ 0100;
 +			}
 +		else
 +			*q++ =3D c;
 +	}
 +	*q =3D '\0';
 +	return dst;
 +}
 +
 +/*
   * Take a raw input line, decode the message, and print the message
   * on the appropriate log files.
   */
  void
  printline(char *hname, char *msg)
  {
 -	int c, pri;
 -	char *p, *q, line[MAXLINE + 1];
 +	int pri;
 +	char *p;
 =20
  	/* test for special codes */
  	pri =3D DEFUPRI;
  	p =3D msg;
  	if (*p =3D=3D '<') {
  		pri =3D 0;
 -		while (isdigit(*++p))
 +		while (isdigit((u_char)*++p))
  			pri =3D 10 * pri + (*p - '0');
  		if (*p =3D=3D '>')
  			++p;
 @@ -644,27 +706,8 @@
  	/* don't allow users to log kernel messages */
  	if (LOG_FAC(pri) =3D=3D LOG_KERN)
  		pri =3D LOG_MAKEPRI(LOG_USER, LOG_PRI(pri));
 -
 -	q =3D line;
 -
 -	while ((c =3D *p++) !=3D '\0' &&
 -	    q < &line[sizeof(line) - 2]) {
 -		c &=3D 0177;
 -		if (iscntrl(c))
 -			if (c =3D=3D '\n')
 -				*q++ =3D ' ';
 -			else if (c =3D=3D '\t')
 -				*q++ =3D '\t';
 -			else {
 -				*q++ =3D '^';
 -				*q++ =3D c ^ 0100;
 -			}
 -		else
 -			*q++ =3D c;
 -	}
 -	*q =3D '\0';
 =20
 -	logmsg(pri, line, hname, 0);
 +	logmsg(pri, msg, hname, 0);
  }
 =20
  /*
 @@ -684,7 +727,7 @@
  		pri =3D DEFSPRI;
  		if (*p =3D=3D '<') {
  			pri =3D 0;
 -			while (isdigit(*++p))
 +			while (isdigit((u_char)*++p))
  				pri =3D 10 * pri + (*p - '0');
  			if (*p =3D=3D '>')
  				++p;
 @@ -710,12 +753,13 @@
   * the priority.
   */
  void
 -logmsg(int pri, char *msg, char *from, int flags)
 +logmsg(int pri, char *rawmsg, char *from, int flags)
  {
  	struct filed *f;
  	int fac, msglen, omask, prilev;
 -	char *timestamp;
 +	char *msg, *timestamp, buf[MAXLINE + 1];
 =20
 +	msg =3D sanitize(buf, rawmsg, sizeof(buf));
  	dprintf("logmsg: pri 0%o, flags 0x%x, from %s, msg %s\n",
  	    pri, flags, from, msg);
 =20
 @@ -1478,12 +1522,19 @@
  	CODE *c;
  	char *p, buf[40];
 =20
 -	if (isdigit(*name))
 -		return (atoi(name));
 +	if (isdigit((u_char)*name)) {
 +		char *endp =3D NULL;
 +		unsigned long l;
 +
 +		errno =3D 0;
 +		l =3D strtoul(name, &endp, 10);
 +		if (errno || *endp !=3D '\0')
 +			return (-1);
 +	}
 =20
  	for (p =3D buf; *name && p < &buf[sizeof(buf) - 1]; p++, name++) {
 -		if (isupper(*name))
 -			*p =3D tolower(*name);
 +		if (isupper((u_char)*name))
 +			*p =3D tolower((u_char)*name);
  		else
  			*p =3D *name;
  	}

From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@gnats.netbsd.org
Cc:  
Subject: Re: bin/18936: syslogd should call initgroups()
Date: Sun, 30 Nov 2003 14:21:25 +0100

 There's a little bug in my patch. Here's the fix:

 --- /tmp/syslogd.c.udif1	2003-11-30 09:22:45.000000000 +0100
 +++ /tmp/syslogd.c.udif	2003-11-30 14:17:03.000000000 +0100
 @@ -4,7 +4,7 @@
  retrieving revision 1.63
  diff -u -r1.63 syslogd.c
  --- syslogd.c	2003/10/17 01:39:25	1.63
 -+++ syslogd.c	2003/11/30 08:22:44
 ++++ syslogd.c	2003/11/30 13:17:03
  @@ -208,6 +208,7 @@
   int	getmsgbufsize(void);
   int*	socksetup(int);
 @@ -190,7 +190,7 @@
  -	*q =3D '\0';
  =20
  -	logmsg(pri, line, hname, 0);
 -+	logmsg(pri, msg, hname, 0);
 ++	logmsg(pri, p, hname, 0);
   }
  =20
   /*
>Unformatted:
 This is a multi-part message in MIME format.

 --Multipart_Tue__5_Nov_2002_03:32:29_+0100_08170e00
 Content-Type: text/plain; charset=US-ASCII
 Content-Transfer-Encoding: 7bit

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.