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
(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.