From owner-freebsd-audit Mon Sep 3 9:25:14 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 0A68137B408; Mon, 3 Sep 2001 09:24:56 -0700 (PDT) Received: (from ru@localhost) by whale.sunbay.crimea.ua (8.11.2/8.11.2) id f83GOnn36329; Mon, 3 Sep 2001 19:24:49 +0300 (EEST) (envelope-from ru) Date: Mon, 3 Sep 2001 19:24:49 +0300 From: Ruslan Ermilov To: Warner Losh , Bruce Evans , Kris Kennaway , Mark Murray Cc: audit@FreeBSD.org Subject: wall -g is broken Message-ID: <20010903192449.B29616@sunbay.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="/9DWx/yDrRhgMJTb" Content-Disposition: inline User-Agent: Mutt/1.2.5i Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG --/9DWx/yDrRhgMJTb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi! As the subject line says, ``wall -g'' appears to be broken. I feel somewhat confused, as the original list of reviewers looks quite amazing: imp, bde, kris, markm, audit@. The use of the getgroups(3) function is unproven since: 1) Its first argument should specify the array size, and is of type `int', not `gid_t'. 2) The code gives false matches and does not produce the required matches. Instead of checking the membership of each line's owner in the -g list of groups, the code gives a match if at least one of the -g groups matches those of the process's groups, as returned by getgroups(). Thus, wall -g `id -gn` will match the entire ttys(5). The attached patch fixes this. Please _REALLY_ review this now! This bug was obtained from OpenBSD, but without mentioning this in the commit log's ``Obtained from: '' field. The bug is still present in OpenBSD. Thanks, -- 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 --/9DWx/yDrRhgMJTb Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=p Index: wall.c =================================================================== RCS file: /home/ncvs/src/usr.bin/wall/wall.c,v retrieving revision 1.19 diff -u -p -r1.19 wall.c --- wall.c 2001/05/08 11:11:42 1.19 +++ wall.c 2001/09/03 16:19:54 @@ -87,18 +87,16 @@ main(int argc, char *argv[]) { struct iovec iov; struct utmp utmp; - gid_t grps[NGROUPS_MAX]; int ch; - int ingroup, ngrps, i; + int ingroup; FILE *fp; struct wallgroup *g; struct group *grp; - char *p; + char *p, **np; struct passwd *pw; char line[sizeof(utmp.ut_line) + 1]; char username[sizeof(utmp.ut_name) + 1]; - ingroup = 0; (void)setlocale(LC_CTYPE, ""); while ((ch = getopt(argc, argv, "g:n")) != -1) @@ -144,19 +142,24 @@ main(int argc, char *argv[]) !strncmp(utmp.ut_name, IGNOREUSER, sizeof(utmp.ut_name))) continue; if (grouplist) { + ingroup = 0; strlcpy(username, utmp.ut_name, sizeof(utmp.ut_name)); pw = getpwnam(username); if (!pw) continue; - ngrps = getgroups(pw->pw_gid, grps); for (g = grouplist; g && ingroup == 0; g = g->next) { if (g->gid == -1) continue; if (g->gid == pw->pw_gid) ingroup = 1; - for (i = 0; i < ngrps && ingroup == 0; i++) - if (g->gid == grps[i]) - ingroup = 1; + else if ((grp = getgrgid(g->gid)) != NULL) { + for (np = grp->gr_mem; *np; np++) { + if (strcmp(*np, username) == 0) { + ingroup = 1; + break; + } + } + } } if (ingroup == 0) continue; --/9DWx/yDrRhgMJTb-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message