From owner-freebsd-security Wed Feb 5 12:06:30 1997 Return-Path: Received: (from root@localhost) by freefall.freebsd.org (8.8.5/8.8.5) id MAA07915 for security-outgoing; Wed, 5 Feb 1997 12:06:30 -0800 (PST) Received: from Mailbox.mcs.com (Mailbox.mcs.com [192.160.127.87]) by freefall.freebsd.org (8.8.5/8.8.5) with ESMTP id MAA07904; Wed, 5 Feb 1997 12:06:18 -0800 (PST) Received: from Jupiter.Mcs.Net (karl@Jupiter.mcs.net [192.160.127.88]) by Mailbox.mcs.com (8.8.5/8.8.2) with ESMTP id OAA20779; Wed, 5 Feb 1997 14:06:14 -0600 (CST) Received: (from karl@localhost) by Jupiter.Mcs.Net (8.8.5/8.8.2) id OAA11778; Wed, 5 Feb 1997 14:06:13 -0600 (CST) From: Karl Denninger Message-Id: <199702052006.OAA11778@Jupiter.Mcs.Net> Subject: PATCH for *ALL* FreeBSD Setlocale() problems - EVERYONE SHOULD READ THIS MESSAGE To: tqbf@enteract.com Date: Wed, 5 Feb 1997 14:06:13 -0600 (CST) Cc: karl@Mcs.Net, freebsd-security@freebsd.org, current@freebsd.org In-Reply-To: <19970205190333.11804.qmail@char-star.rdist.org> from "tqbf@enteract.com" at Feb 5, 97 07:03:33 pm X-Mailer: ELM [version 2.4 PL24] Content-Type: text Sender: owner-security@freebsd.org X-Loop: FreeBSD.org Precedence: bulk > > In article <199702051742.LAA05872@Jupiter.Mcs.Net>, you wrote: > >The ENTIRE setlocale() code is a HUGE security problem. Among other things, > > locales in general are an issue. FreeBSD's rewritten locale code, which > obviously wasn't written with much thought towards security, is another > issue. The main issue, to my mind, is the caller of an SUID program being > able to control the path to it's locale information. > > To my mind, SUID/SGID programs should be ignoring PATH_LOCALE. I don't > know that the best way to handle this is from euid/uid checks in libc - > that seems like a hack to me. > > >SETLOCALE MUST BE REMOVED FROM USE UNTIL IT CAN BE FIXED. It is FULL of > > ... but, Mr. Denninger is right here. Among other things, the idiom for > calling setlocale() seems to be to do it first, before argument > processing. This means that any program vulnerable to any problem caused > by the locale routings is vulnerable regardless of how it's called. I am > concerned about privileged code calling non-privileged code and becoming > vulnerable. > > >I have already found setlocale() calls in SEVERAL privileged programs. > > They're all over the place in 2.2, as a consequence of it not being > handled automatically anymore. Right. I have the following patches at this point. They are NOT yet tested. I am doing that now and rebuilding. Once the testing is complete, I will advise. NOTE THAT THIS APPLIES TO *ALL* VERSIONS OF FREEBSD WHICH HAVE THE LOCALE DIRECTORY UNDER LIBC IN THE SOURCE TREE. There may be minor differences, but frankly, I expect that these or a slight varient absolutely MUST be applied to close this hole. And unfortunately, you have to rebuild everything once you've done so (or at least, libc.so.x, crt0.o and all static binaries). I will EXPECT that these will show up in the CVS tree within 48 hours unless there are VERY good reasons expressed for them not being included. I WILL be looking for them to appear. This SHOULD fix the vulnerability; if not correctly, then by brute force (by disallowing setlocale() calls from anything running with privileges) This is from a scan for "strcpy"s in the "locale" directory under libc. These patches: 1) Fix all the roaring obvious strcpys and strcats to be the str(n) equivalents. There IS an exception in the final concatenation routine, but the arguments to that routine should have been checked already, so that should be safe. 2) Disallow setlocale() entirely if: 1) You're SUID or SGID 2) OR you are running with EUID = 0 regardless of how Go ahead and poke at this if you'd like - - that's what code reviews are all about - - but I believe this does address the vulnerability. *** collate.c Wed Feb 5 13:28:17 1997 --- collate.c.orig Wed Feb 5 13:26:08 1997 *************** *** 74,83 **** return -1; } /* Range checking already done at upper level caller */ ! (void) strncpy(buf, _PathLocale, (PATH_MAX - 1)); ! (void) strncat(buf, "/", (PATH_MAX - (2 + strlen(buf))); ! (void) strncat(buf, encoding, (PATH_MAX - (1 + strlen(buf))); ! (void) strncat(buf, "/LC_COLLATE", (PATH_MAX - (1 + strlen(buf))); if ((fp = fopen(buf, "r")) == NULL) { __collate_load_error = save_load_error; return -1; --- 74,83 ---- return -1; } /* Range checking already done at upper level caller */ ! (void) strcpy(buf, _PathLocale); ! (void) strcat(buf, "/"); ! (void) strcat(buf, encoding); ! (void) strcat(buf, "/LC_COLLATE"); if ((fp = fopen(buf, "r")) == NULL) { __collate_load_error = save_load_error; return -1; *** setlocale.c Wed Feb 5 13:56:46 1997 --- setlocale.c.orig Wed Feb 5 13:26:12 1997 *************** *** 106,119 **** int i, j, len; char *env, *r; - /* - * KSD - If we're setuid or setgid, or root, ignore this and return - * instantly 2/5/97 - */ - if ((geteuid() != getuid()) || (getegid() != getgid()) || !geteuid()) { - return(NULL); - } - if (category < LC_ALL || category >= _LC_LAST) return (NULL); --- 106,111 ---- *************** *** 124,133 **** /* * Default to the current locale for everything. */ ! for (i = 1; i < _LC_LAST; ++i) { ! (void)strncpy(new_categories[i], current_categories[i], 31); ! new_categories[i][31] = 0; ! } /* * Now go fill up new_categories from the locale argument --- 116,123 ---- /* * Default to the current locale for everything. */ ! for (i = 1; i < _LC_LAST; ++i) ! (void)strcpy(new_categories[i], current_categories[i]); /* * Now go fill up new_categories from the locale argument *************** *** 176,199 **** ++locale; while (*++r && *r != '/'); } while (*locale); ! while (i < _LC_LAST) { ! (void)strncpy(new_categories[i], ! new_categories[i-1], 31); ! new_categories[i][31] = 0; ! } } } if (category) return (loadlocale(category)); for (i = 1; i < _LC_LAST; ++i) { ! (void)strncpy(saved_categories[i], current_categories[i], 31); ! saved_categories[i][31] = 0; if (loadlocale(i) == NULL) { for (j = 1; j < i; j++) { ! (void)strncpy(new_categories[j], ! saved_categories[j], 31); ! new_categories[j][31] = 0; /* XXX can fail too */ (void)loadlocale(j); } --- 166,186 ---- ++locale; while (*++r && *r != '/'); } while (*locale); ! while (i < _LC_LAST) ! (void)strcpy(new_categories[i], ! new_categories[i-1]); } } + if (category) return (loadlocale(category)); for (i = 1; i < _LC_LAST; ++i) { ! (void)strcpy(saved_categories[i], current_categories[i]); if (loadlocale(i) == NULL) { for (j = 1; j < i; j++) { ! (void)strcpy(new_categories[j], ! saved_categories[j]); /* XXX can fail too */ (void)loadlocale(j); } *************** *** 218,226 **** currentlocale() { int i; - /* - * Bounds already checked on current_categories; can't overflow - KSD 2/5/97 - */ (void)strcpy(current_locale_string, current_categories[1]); --- 205,210 ---- *************** *** 228,234 **** if (strcmp(current_categories[1], current_categories[i])) { (void) strcpy(current_locale_string, current_categories[1]); (void) strcat(current_locale_string, "/"); ! (void) strncat(current_locale_string, current_categories[2]); (void) strcat(current_locale_string, "/"); (void) strcat(current_locale_string, current_categories[3]); (void) strcat(current_locale_string, "/"); --- 212,218 ---- if (strcmp(current_categories[1], current_categories[i])) { (void) strcpy(current_locale_string, current_categories[1]); (void) strcat(current_locale_string, "/"); ! (void) strcat(current_locale_string, current_categories[2]); (void) strcat(current_locale_string, "/"); (void) strcat(current_locale_string, current_categories[3]); (void) strcat(current_locale_string, "/"); *** setrunelocale.c Wed Feb 5 13:35:20 1997 --- setrunelocale.c.orig Wed Feb 5 13:26:22 1997 *************** *** 86,96 **** if (!_PathLocale) return(EFAULT); /* Range checking already done at upper level caller */ ! (void) strncpy(name, _PathLocale, (PATH_MAX - 1)); ! name[PATH_MAX - 1] = 0; (void) strcat(name, "/"); ! (void) strncat(name, encoding, (PATH_MAX - (2 + strlen(name))); ! (void) strncat(name, "/LC_CTYPE", (PATH_MAX - (2 + strlen(name))); if ((fp = fopen(name, "r")) == NULL) return(ENOENT); --- 86,95 ---- if (!_PathLocale) return(EFAULT); /* Range checking already done at upper level caller */ ! (void) strcpy(name, _PathLocale); (void) strcat(name, "/"); ! (void) strcat(name, encoding); ! (void) strcat(name, "/LC_CTYPE"); if ((fp = fopen(name, "r")) == NULL) return(ENOENT); -- -- Karl Denninger (karl@MCS.Net)| MCSNet - The Finest Internet Connectivity http://www.mcs.net/~karl | T1's from $600 monthly to FULL DS-3 Service | 99 Analog numbers, 77 ISDN, Web servers $75/mo Voice: [+1 312 803-MCS1 x219]| Email to "info@mcs.net" WWW: http://www.mcs.net/ Fax: [+1 773 248-9865] | 2 FULL DS-3 Internet links; 400Mbps B/W Internal