Date: Wed, 5 Feb 1997 14:06:13 -0600 (CST) From: Karl Denninger <karl@Mcs.Net> To: tqbf@enteract.com Cc: karl@Mcs.Net, freebsd-security@freebsd.org, current@freebsd.org Subject: PATCH for *ALL* FreeBSD Setlocale() problems - EVERYONE SHOULD READ THIS MESSAGE Message-ID: <199702052006.OAA11778@Jupiter.Mcs.Net> In-Reply-To: <19970205190333.11804.qmail@char-star.rdist.org> from "tqbf@enteract.com" at Feb 5, 97 07:03:33 pm
next in thread | previous in thread | raw e-mail | index | archive | help
>
> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199702052006.OAA11778>