From owner-freebsd-bugs Fri Sep 22 13:30:06 1995 Return-Path: owner-bugs Received: (from root@localhost) by freefall.freebsd.org (8.6.12/8.6.6) id NAA17664 for bugs-outgoing; Fri, 22 Sep 1995 13:30:06 -0700 Received: (from gnats@localhost) by freefall.freebsd.org (8.6.12/8.6.6) id NAA17658 ; Fri, 22 Sep 1995 13:30:02 -0700 Resent-Date: Fri, 22 Sep 1995 13:30:02 -0700 Resent-Message-Id: <199509222030.NAA17658@freefall.freebsd.org> Resent-From: gnats (GNATS Management) Resent-To: freebsd-bugs Resent-Reply-To: FreeBSD-gnats@freefall.FreeBSD.org, fenner@parc.xerox.com Received: from alpha.xerox.com (alpha.Xerox.COM [13.1.64.93]) by freefall.freebsd.org (8.6.12/8.6.6) with SMTP id NAA17586 for ; Fri, 22 Sep 1995 13:26:21 -0700 Received: from baobab.parc.xerox.com ([13.2.116.113]) by alpha.xerox.com with SMTP id <14490(1)>; Fri, 22 Sep 1995 13:25:36 PDT Received: (from fenner@localhost) by baobab.parc.xerox.com (8.6.11/8.6.9) id NAA14886; Fri, 22 Sep 1995 13:27:47 -0700 Message-Id: <199509222027.NAA14886@baobab.parc.xerox.com> Date: Fri, 22 Sep 1995 13:27:47 PDT From: Bill Fenner Reply-To: fenner@parc.xerox.com To: FreeBSD-gnats-submit@freebsd.org X-Send-Pr-Version: 3.2 Subject: bin/732: getpwent() dumps core if NIS password file is malformed Sender: owner-bugs@freebsd.org Precedence: bulk >Number: 732 >Category: bin >Synopsis: getpwent() dumps core if NIS password file is malformed >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Fri Sep 22 13:30:01 PDT 1995 >Last-Modified: >Originator: Bill Fenner >Organization: Xerox PARC >Release: FreeBSD 2.0-BUILT-19950628 i386 >Environment: FreeBSD 2.1.0-950726-SNAP >Description: "finger" dumps core after enabling NIS passwords with +::::::::: in /etc/master.passwd It turns out that there is an entry in our NIS map with a 13-character username, which overflows a static buffer. It also turns out that this static buffer is one character too small anyway, as 8-character usernames can overflow it with the trailing NUL. (but probably won't cause a core dump...) >How-To-Repeat: Create an entry with a 13-character username, and then try "finger ". finger will core dump because of a buffer overrun in _getyppass() >Fix: I rewrote the code to not require the static buffer, and instead to use the malloc()'d buffer that yp_first(), yp_next(), or yp_match() allocated for us. I was sure that I sent these diffs in previously, but I can't find the email message so I am sending them as a PR now that gnats is fixed. I should note that it seems like a real shame to have two functions whose bodies are so similar, but I wasn't prepared to address the higher level problem, I just wanted it to stop dumping core. --- getpwent.c.orig Tue Sep 5 17:04:47 1995 +++ getpwent.c Tue Sep 5 18:30:56 1995 @@ -586,13 +586,11 @@ _getyppass(struct passwd *pw, const char *name, const char *map) { char *result, *s; - static char resultbuf[1024]; int resultlen; char mastermap[1024]; int gotmaster = 0; struct _pw_cache *m, *p; struct _namelist *n; - char user[UT_NAMESIZE]; if(!_pw_yp_domain) { if(yp_get_default_domain(&_pw_yp_domain)) @@ -613,18 +611,23 @@ return 0; s = strchr(result, '\n'); - if(s) *s = '\0'; + if (s) *s = '\0'; - if(resultlen >= sizeof resultbuf) return 0; - strcpy(resultbuf, result); - sprintf (user, "%.*s", (strchr(result, ':') - result), result); + s = strchr(result, ':'); + if (s) { + *s = '\0'; + } else { + /* malformed, no colon */ + free(result); + return 0; + } _pw_passwd.pw_fields = -1; /* Impossible value */ if (_minuscnt && _minushead) { m = _minushead; while (m) { n = m->namelist; while (n) { - if (!strcmp(n->name,user) || *n->name == '\0') { + if (!strcmp(n->name,result) || *n->name == '\0') { free(result); return (0); } @@ -638,7 +641,7 @@ while (p) { n = p->namelist; while (n) { - if (!strcmp(n->name, user) || *n->name == '\0') + if (!strcmp(n->name, result) || *n->name == '\0') bcopy((char *)&p->pw_entry, (char *)&_pw_passwd, sizeof(p->pw_entry)); n = n->next; @@ -646,12 +649,14 @@ p = p->next; } } - free(result); /* No hits in the plus or minus lists: Bzzt! reject. */ - if (_pw_passwd.pw_fields == -1) + if (_pw_passwd.pw_fields == -1) { + free(result); return(0); - result = resultbuf; - _pw_breakout_yp(pw, resultbuf, gotmaster); + } + *s = ':'; /* Put colon back */ + _pw_breakout_yp(pw, result, gotmaster); + free(result); return 1; } @@ -662,14 +667,13 @@ static char *key; static int keylen; char *lastkey, *result; - static char resultbuf[1024]; int resultlen; int rv; char *map = "passwd.byname"; int gotmaster = 0; struct _pw_cache *m, *p; struct _namelist *n; - char user[UT_NAMESIZE]; + char *s; if(!_pw_yp_domain) { if(yp_get_default_domain(&_pw_yp_domain)) @@ -704,20 +708,21 @@ return 0; } - if(resultlen > sizeof(resultbuf)) { + s = strchr(result, ':'); + if (s) { + *s = '\0'; + } else { + /* malformed, no colon */ free(result); goto tryagain; } - - strcpy(resultbuf, result); - sprintf(user, "%.*s", (strchr(result, ':') - result), result); _pw_passwd.pw_fields = -1; /* Impossible value */ if (_minuscnt && _minushead) { m = _minushead; while (m) { n = m->namelist; while (n) { - if (!strcmp(n->name, user) || *n->name == '\0') { + if (!strcmp(n->name, result) || *n->name == '\0') { free(result); goto tryagain; } @@ -731,7 +736,7 @@ while (p) { n = p->namelist; while (n) { - if (!strcmp(n->name, user) || *n->name == '\0') + if (!strcmp(n->name, result) || *n->name == '\0') bcopy((char *)&p->pw_entry, (char*)&_pw_passwd, sizeof(p->pw_entry)); n = n->next; @@ -739,12 +744,15 @@ p = p->next; } } - free(result); /* No plus or minus hits: Bzzzt! reject. */ - if (_pw_passwd.pw_fields == -1) + if (_pw_passwd.pw_fields == -1) { + free(result); goto tryagain; - if(result = strchr(resultbuf, '\n')) *result = '\0'; - _pw_breakout_yp(pw, resultbuf, gotmaster); + } + *s = ':'; /* Put colon back */ + if(s = strchr(result, '\n')) *s = '\0'; + _pw_breakout_yp(pw, result, gotmaster); + free(result); } return 1; } >Audit-Trail: >Unformatted: