From owner-freebsd-bugs@FreeBSD.ORG Tue Jul 31 13:00:26 2012 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 789D3106566B for ; Tue, 31 Jul 2012 13:00:26 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 453078FC16 for ; Tue, 31 Jul 2012 13:00:26 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q6VD0Q0M025172 for ; Tue, 31 Jul 2012 13:00:26 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q6VD0Qcs025171; Tue, 31 Jul 2012 13:00:26 GMT (envelope-from gnats) Resent-Date: Tue, 31 Jul 2012 13:00:26 GMT Resent-Message-Id: <201207311300.q6VD0Qcs025171@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Andrey Simonenko Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 7DB39106564A for ; Tue, 31 Jul 2012 12:54:04 +0000 (UTC) (envelope-from simon@comsys.ntu-kpi.kiev.ua) Received: from comsys.kpi.ua (comsys.kpi.ua [77.47.192.42]) by mx1.freebsd.org (Postfix) with ESMTP id CCCF18FC0A for ; Tue, 31 Jul 2012 12:54:03 +0000 (UTC) Received: from pm513-1.comsys.kpi.ua ([10.18.52.101] helo=pm513-1.comsys.ntu-kpi.kiev.ua) by comsys.kpi.ua with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1SwBxa-00033r-Dl for FreeBSD-gnats-submit@freebsd.org; Tue, 31 Jul 2012 15:54:02 +0300 Received: by pm513-1.comsys.ntu-kpi.kiev.ua (Postfix, from userid 1001) id 30CF71CC1E; Tue, 31 Jul 2012 15:54:01 +0300 (EEST) Message-Id: <20120731125400.GA12154@pm513-1.comsys.ntu-kpi.kiev.ua> Date: Tue, 31 Jul 2012 15:54:01 +0300 From: Andrey Simonenko To: FreeBSD-gnats-submit@FreeBSD.org Cc: Subject: bin/170295: mountd: correct credentials parsing in -mapall and -maproot options X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 Jul 2012 13:00:26 -0000 >Number: 170295 >Category: bin >Synopsis: mountd: correct credentials parsing in -mapall and -maproot options >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: update >Submitter-Id: current-users >Arrival-Date: Tue Jul 31 13:00:25 UTC 2012 >Closed-Date: >Last-Modified: >Originator: Andrey Simonenko >Release: FreeBSD 10.0-CURRENT amd64 >Organization: >Environment: >Description: The usr.sbin/mountd/mountd.c:parsecred() function has the following mistakes: 1. It has buffer overflow if number of GIDs of some user is greater than the XU_NGROUPS value, incorrect usage of getgrouplist(3). 2. It incorrectly gets group lists for a user given without groups: it forgets about a single group of a user or forgets about the first supplementary group of a user. 3. If a user is unknown it silently uses -2:-2 credentials and this does not correspond to exports(5) rules. 4. If a group is unknown, then it ignores this group and this does not correspond to exports(5) rules. 5. It uses atoi(3) function to parse UID and GID, and does not check any mistakes in numbers. >How-To-Repeat: >Fix: --- mountd.c.orig 2012-01-20 13:19:39.000000000 +0200 +++ mountd.c 2012-07-31 15:31:39.000000000 +0300 @@ -199,7 +199,7 @@ int makemask(struct sockaddr_storage *ss void mntsrv(struct svc_req *, SVCXPRT *); void nextfield(char **, char **); void out_of_mem(void); -void parsecred(char *, struct xucred *); +static int parsecred(char *, struct xucred *); int parsesec(char *, struct exportlist *); int put_exlist(struct dirlist *, XDR *, struct dirlist *, int *, int); void *sa_rawaddr(struct sockaddr *sa, int *nbytes); @@ -2140,7 +2140,8 @@ do_opt(char **cpp, char **endcpp, struct !(allflag = strcmp(cpopt, "mapall")) || !strcmp(cpopt, "root") || !strcmp(cpopt, "r"))) { usedarg++; - parsecred(cpoptarg, cr); + if (parsecred(cpoptarg, cr)) + return (1); if (allflag == 0) { *exflagsp |= MNT_EXPORTANON; opt_flags |= OP_MAPALL; @@ -2760,81 +2761,100 @@ get_line(void) /* * Parse a description of a credential. */ -void +static int parsecred(char *namelist, struct xucred *cr) { - char *name; - int cnt; - char *names; - struct passwd *pw; - struct group *gr; - gid_t groups[XU_NGROUPS + 1]; + const struct group *gr; + const struct passwd *pw; + const char *errstr, *username; + char *name, *names; + uid_t uid; int ngroups; - cr->cr_version = XUCRED_VERSION; - /* - * Set up the unprivileged user. - */ - cr->cr_uid = -2; - cr->cr_groups[0] = -2; - cr->cr_ngroups = 1; /* * Get the user's password table entry. */ names = strsep(&namelist, " \t\n"); - name = strsep(&names, ":"); - if (isdigit(*name) || *name == '-') - pw = getpwuid(atoi(name)); - else - pw = getpwnam(name); - /* - * Credentials specified as those of a user. - */ - if (names == NULL) { - if (pw == NULL) { - syslog(LOG_ERR, "unknown user: %s", name); - return; + username = name = strsep(&names, ":"); + errno = 0; + pw = getpwnam(name); + if (pw == NULL) { + if (errno != 0) { + syslog(LOG_ERR, "getpwnam: %m"); + return (1); + } + uid = (uid_t)strtonum(name, 0, UID_MAX, &errstr); + if (errstr != NULL) { + if (errno == ERANGE) + syslog(LOG_ERR, "UID %s is %s", name, errstr); + else + syslog(LOG_ERR, "unknown user: %s", name); + return (1); + } + if (names == NULL) { + errno = 0; + pw = getpwuid(uid); + if (pw == NULL) { + if (errno != 0) + syslog(LOG_ERR, "getpwuid: %m"); + else + syslog(LOG_ERR, "unknown user: %s", + name); + return (1); + } } - cr->cr_uid = pw->pw_uid; - ngroups = XU_NGROUPS + 1; - if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups)) - syslog(LOG_ERR, "too many groups"); + } else + uid = pw->pw_uid; + cr->cr_version = XUCRED_VERSION; + cr->cr_uid = uid; + if (names == NULL) { /* - * Compress out duplicate. + * Credentials specified as those of a user. */ - cr->cr_ngroups = ngroups - 1; - cr->cr_groups[0] = groups[0]; - for (cnt = 2; cnt < ngroups; cnt++) - cr->cr_groups[cnt - 1] = groups[cnt]; - return; - } - /* - * Explicit credential specified as a colon separated list: - * uid:gid:gid:... - */ - if (pw != NULL) - cr->cr_uid = pw->pw_uid; - else if (isdigit(*name) || *name == '-') - cr->cr_uid = atoi(name); - else { - syslog(LOG_ERR, "unknown user: %s", name); - return; - } - cr->cr_ngroups = 0; - while (names != NULL && *names != '\0' && cr->cr_ngroups < XU_NGROUPS) { - name = strsep(&names, ":"); - if (isdigit(*name) || *name == '-') { - cr->cr_groups[cr->cr_ngroups++] = atoi(name); - } else { - if ((gr = getgrnam(name)) == NULL) { - syslog(LOG_ERR, "unknown group: %s", name); - continue; + ngroups = XU_NGROUPS; + if (getgrouplist(pw->pw_name, pw->pw_gid, cr->cr_groups, + &ngroups) < 0) { + syslog(LOG_ERR, "user %s: too many groups", + pw->pw_name); + cr->cr_ngroups = XU_NGROUPS; + } else + cr->cr_ngroups = ngroups; + } else { + /* + * Explicit credential specified as a colon separated list: + * uid:gid:gid:... + */ + cr->cr_ngroups = 0; + for (; names != NULL && *names != '\0'; cr->cr_ngroups++) { + if (cr->cr_ngroups == XU_NGROUPS) { + syslog(LOG_ERR, "user %s: too many groups", + username); + break; } - cr->cr_groups[cr->cr_ngroups++] = gr->gr_gid; + name = strsep(&names, ":"); + errno = 0; + gr = getgrnam(name); + if (gr == NULL) { + if (errno != 0) { + syslog(LOG_ERR, "getgrnam: %m"); + return (1); + } + cr->cr_groups[cr->cr_ngroups] = + (gid_t)strtonum(name, 0, GID_MAX, &errstr); + if (errstr != NULL) { + if (errno == ERANGE) + syslog(LOG_ERR, "GID %s " + "is %s", name, errstr); + else + syslog(LOG_ERR, "unknown " + "group: %s", name); + return (1); + } + } else + cr->cr_groups[cr->cr_ngroups] = gr->gr_gid; } } - if (names != NULL && *names != '\0' && cr->cr_ngroups == XU_NGROUPS) - syslog(LOG_ERR, "too many groups"); + return (0); } #define STRSIZ (MNTNAMLEN+MNTPATHLEN+50) >Release-Note: >Audit-Trail: >Unformatted: