Date: Tue, 31 Jul 2012 15:54:01 +0300 From: Andrey Simonenko <simon@comsys.ntu-kpi.kiev.ua> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/170295: mountd: correct credentials parsing in -mapall and -maproot options Message-ID: <20120731125400.GA12154@pm513-1.comsys.ntu-kpi.kiev.ua> Resent-Message-ID: <201207311300.q6VD0Qcs025171@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120731125400.GA12154>