This is something of a security bug report, but the security problem is not necessarily a bug in the timezone code. However, I do think it's a bug that our code (which is not the very latest tzcode) will crash if the TZ variable is set to point to a non-TZ file. I can think of two ways to combat this: 1) Disallow selection of files containing `.' or starting with a leading `/' unless the name is the same as TZDEFAULT. 2) Use some of the tzh_reserved[] bytes for a magic number. (2) is more flexible, but still allows for a malicious user to cause a core dump. I prefer (1) and will probably implement that unless someone can provide a good reason not to. ------- start of forwarded message (RFC 934 encapsulation) ------- From: J Wunsch <j@ida.interface-business.de> To: wollman@freebsd.org Subject: hey (fwd) Date: Fri, 10 Jan 1997 12:28:15 +0100 (MET) Hi Garrett, can you deal in time with this problem? Otherwise, i'd look into it myself. - ----- Forwarded message from Adam Kubicki ----- Message-Id: <199701092155.WAA12271@innocence.interface-business.de> From: Adam Kubicki <mikee@solozzo.tele.pw.edu.pl> Subject: hey To: joerg_wunsch@interface-business.de Date: Thu, 09 Jan 1997 23:02:44 MET In-Reply-To: <199612171142.MAA11937@ida.interface-business.de>; from "J Wunsch" at Dec 17, 96 12:42 (noon) X-Mailer: Elm [revision: 112.2] hi, there is a bug in tzset() function. setting TZ environment variable to some file, you can cause program to dump core - variables read from this file are used as various offsets in settzname() thus you get sigsegv. Because suid programs dont dump core, its not so dangerous, but you can export TZ in telnet and force login to dump core. gettimeoftheday() in login.c is called after loging in, but before setuid(uid) so you will get login.core in you home directory. this core file will follow symlink allowing you to overwrite any file on system. And, setting TZ to /etc/master.passwd you will find whole master.passwd in core file (touch login.core first to fool default umask/owner core flags). I'be be glad to get a smart patch from you, as quick fix i disabled TZ in telnetd. - -adam - ----- End of forwarded message from Adam Kubicki ----- - -- J"org Wunsch Unix support engineer joerg_wunsch@interface-business.de http://www.interface-business.de/~j ------- end -------
I can think of a third way to combat that problem: make the code that reads the data files robust, so it checks for invalid data and makes sure it can never crash due to bogus data. I just added code to the GNU libc tz implementation to do this. It took me about five minutes, and the added run-time overhead is not much given what's already going on to internalize the data file. It is a matter of security policy how you want to restrict the setting of the TZ variable. I wouldn't want it restricted at all. Why shouldn't I use my own timezone data files? It's certainly the easiest way to test new data files before installing them to point TZ at my own data files and run `date'. I prefer policies based on content rather than origin. It is a matter of local administrative quality whether the installed data files are more or less likely to contain valid or correct data than ones a user maintains himself. If the intent is to avoid crashing programs, robust reading code is protected against both mistakes and malice, whether installed system-wide or self-inflicted by the individual user. If the intent is to administratively restrict the time zones users can ask for display in, then all you have succeeded in doing is forcing people to use the cryptic 1003.1 TZ syntax instead of writing their own wonderful zoneinfo files through the joys of zic.
Date: Fri, 10 Jan 1997 14:19:50 -0500 From: Roland McGrath <roland@frob.com> I can think of a third way to combat that problem: make the code that reads the data files robust, so it checks for invalid data and makes sure it can never crash due to bogus data. That is the best way, and in fact the tz code is supposed to do that already. I just took a quick look and I couldn't see any holes in its checking, though I didn't look that carefully. The original bug report is too sketchy to see what the problem might be. I could not reproduce it with the elsie localtime.c under Solaris 2.5.1. Perhaps the problem is in the FreeBSD edition rather than the elsie code? I just compared their localtime.c implementations (using FreeBSD-current), and the only differences that I see are: * The FreeBSD edition has mutexes to support reentrant variants like localtime_r. * The FreeBSD edition refuses to read a timezone file if it is not a regular file. (I don't know why this restriction is needed.) * The FreeBSD edition is missing a fix for the Posix case. Perhaps the bug is in the FreeBSD mutex handling. For example, gmtload is sometimes protected by gmt_mutex, and sometimes by lcl_mutex; this sounds odd to me, but I don't understand FreeBSD mutexes so I could just be misunderstanding things. Perhaps the elsie version should add support for localtime_r and friends; this might help avoid future bugs in this area. localtime_r is now officially part of Posix, after all. From: Adam Kubicki <mikee@solozzo.tele.pw.edu.pl> Date: Thu, 09 Jan 1997 23:02:44 MET And, setting TZ to /etc/master.passwd you will find whole master.passwd in core file (touch login.core first to fool default umask/owner core flags). I presume that /etc/master.password is not supposed to be readable to ordinary users. That's odd. In both elsie tz and FreeBSD-current, tzload uses access() as well as open() to check whether the file is readable. There is of course a window of vulnerability here, but it doesn't sound like Kubicki is trying to exploit it. Perhaps he's using some other implementation of localtime?
<<On Fri, 10 Jan 1997 15:38:45 -0800, Paul Eggert <eggert@twinsun.com> said:
report is too sketchy to see what the problem might be. I could not reproduce it with the elsie localtime.c under Solaris 2.5.1.
Here is a typescript which demonstrates it... root@khavrinen(31)# TZ=/etc/master.passwd gdb obj/date GDB is free software and you are welcome to distribute copies of it under certain conditions; type "show copying" to see the conditions. There is absolutely no warranty for GDB; type "show warranty" for details. GDB 4.16 (i386-unknown-freebsd2.2), Copyright 1996 Free Software Foundation, Inc... (gdb) br main Breakpoint 1 at 0x119c: file /usr/wd1/src/bin/date/date.c, line 83. (gdb) run Starting program: /usr/wd1/src/bin/date/obj/date Breakpoint 1, main (argc=1, argv=0xefbfd8a8) at /usr/wd1/src/bin/date/date.c:83 83 (void) setlocale(LC_TIME, ""); [deletia] (gdb) n 144 (void)strftime(buf, sizeof(buf), format, localtime(&tval)); (gdb) br localtime Breakpoint 2 at 0x139ab: file /usr/wd1/src/lib/libc/stdtime/localtime.c, line 1125. (gdb) cont Continuing. Breakpoint 2, localtime (timep=0x28534) at /usr/wd1/src/lib/libc/stdtime/localtime.c:1125 1125 tzset(); (gdb) n Program received signal SIGSEGV, Segmentation fault. 0x1293d in settzname () at /usr/wd1/src/lib/libc/stdtime/localtime.c:241 241 tzname[ttisp->tt_isdst] = (gdb) bt #0 0x1293d in settzname () at /usr/wd1/src/lib/libc/stdtime/localtime.c:241 #1 0x138e1 in tzset () at /usr/wd1/src/lib/libc/stdtime/localtime.c:1010 #2 0x139b0 in localtime (timep=0x28534) at /usr/wd1/src/lib/libc/stdtime/localtime.c:1125 #3 0x139a in main (argc=1, argv=0xefbfd8a8) at /usr/wd1/src/bin/date/date.c:144 (gdb) `ttisp' appears to point off into nowhere land... `lclmem' contains the following data: $2 = {leapcnt = 0x726f6f74, timecnt = 0x206f6e20, typecnt = 0x6b686176, charcnt = 0x72696e65, ats = {0x0 <repeats 370 times>}, types = { 0x0 <repeats 370 times>}, ttis = {{tt_gmtoff = 0x0, tt_isdst = 0x0, tt_abbrind = 0x0, tt_ttisstd = 0x0, tt_ttisgmt = 0x0} <repeats 256 times>}, chars = { 0x0 <repeats 512 times>}, lsis = {{ls_trans = 0x0, ls_corr = 0x0} <repeats 50 times>}} ...which manifestly makes no sense whatsoever.
* The FreeBSD edition refuses to read a timezone file if it is not a regular file. (I don't know why this restriction is needed.)
As I look in the problem-report database, it is clear that this change results from a previous attempt at dealing with the same bug. The description from our PR#1740 is:
Description: Setting TZ to unusual values like "Europe" crashes all applications using the localtime routine. This is because the tzload routine happily opens any file, even directories, as valid timezone files. "Europe" happens to be a directory under /usr/share/zoneinfo. How-To-Repeat: TZ=Europe date
Perhaps the bug is in the FreeBSD mutex handling.
Unlikely, since neither the reporters in question nor I am using the thread-safe version of libc.
From: Adam Kubicki <mikee@solozzo.tele.pw.edu.pl> Date: Thu, 09 Jan 1997 23:02:44 MET
And, setting TZ to /etc/master.passwd you will find whole master.passwd in core file (touch login.core first to fool default umask/owner core flags).
I presume that /etc/master.password is not supposed to be readable to ordinary users. That's odd. In both elsie tz and FreeBSD-current, tzload uses access() as well as open() to check whether the file is readable. There is of course a window of vulnerability here, but it doesn't sound like Kubicki is trying to exploit it.
Recall that the `login' program prints out the time of last login. Because it has previously opened the password database, that data may still be floating around its memory image, and thus would be stored in the core file even though it is created after login has switched to the user's ID. I don't think that this bug is still present; writecore() should refuse to operate if the process's P_SUGID flag is set, which is supposed to happen any time setuid(2) or setgid(2) is successful. It is possible to use the TELNET Environment Variable option to pass a bogus TZ variable to the login process on the other end. -GAWollman -- Garrett A. Wollman | O Siem / We are all family / O Siem / We're all the same wollman@lcs.mit.edu | O Siem / The fires of freedom Opinions not those of| Dance in the burning flame MIT, LCS, ANA, or NSA| - Susan Aglukark and Chad Irschick
<<On Fri, 10 Jan 1997 15:38:45 -0800, Paul Eggert <eggert@twinsun.com> said:
That is the best way, and in fact the tz code is supposed to do that already. I just took a quick look and I couldn't see any holes in its checking, though I didn't look that carefully.
It turns out that it is doing the proper checking, but it is not doing a good job of cleaning up the mess. Here are more segments of my debugging typescript: gmtload (sp=0x24124) at /usr/wd1/src/lib/libc/stdtime/localtime.c:921 921 (void) tzparse(gmt, sp, TRUE); (gdb) s tzparse (name=0x128d0 "GMT", sp=0x24124, lastditch=1) at /usr/wd1/src/lib/libc/stdtime/localtime.c:720 720 INITIALIZE(dstname); (gdb) 721 stdname = name; (gdb) 722 if (lastditch) { (gdb) 723 stdlen = strlen(name); /* length of standard zone name */ (gdb) 724 name += stdlen; (gdb) 725 if (stdlen >= sizeof sp->chars) (gdb) 727 } else { (gdb) 733 if (*name == '\0') (gdb) 734 return -1; /* was "stdoffset = 0;" */ (gdb) p *sp $13 = {leapcnt = 1919905652, timecnt = 544173600, typecnt = 1802002806, charcnt = 1919512165, ats = {0 <repeats 370 times>}, types = '\000' <repeats 369 times>, ttis = {{tt_gmtoff = 0, tt_isdst = 0, tt_abbrind = 0, tt_ttisstd = 0, tt_ttisgmt = 0} <repeats 256 times>}, chars = '\000' <repeats 511 times>, lsis = {{ls_trans = 0, ls_corr = 0} <repeats 50 times>}} (gdb) n 914 } (gdb) gmtload (sp=0x24124) at /usr/wd1/src/lib/libc/stdtime/localtime.c:922 922 } (gdb) tzset () at /usr/wd1/src/lib/libc/stdtime/localtime.c:1010 1010 settzname(); (gdb) (and of course, settzname() is where it already blows up...) Line 734 above appears to be the error; it doesn't take into account the possibility that this is a `lasatditch' attempt. The following patch appears to fix the problem: --- localtime.c.ORIG Mon Jan 13 12:00:35 1997 +++ localtime.c Mon Jan 13 12:01:20 1997 @@ -724,18 +724,19 @@ name += stdlen; if (stdlen >= sizeof sp->chars) stdlen = (sizeof sp->chars) - 1; + stdoffset = 0; } else { name = getzname(name); stdlen = name - stdname; if (stdlen < 3) return -1; - } - if (*name == '\0') - return -1; /* was "stdoffset = 0;" */ - else { - name = getoffset(name, &stdoffset); - if (name == NULL) - return -1; + if (*name == '\0') + return -1; /* was "stdoffset = 0;" */ + else { + name = getoffset(name, &stdoffset); + if (name == NULL) + return -1; + } } load_result = tzload(TZDEFRULES, sp); if (load_result != 0) -GAWollman -- Garrett A. Wollman | O Siem / We are all family / O Siem / We're all the same wollman@lcs.mit.edu | O Siem / The fires of freedom Opinions not those of| Dance in the burning flame MIT, LCS, ANA, or NSA| - Susan Aglukark and Chad Irschick
participants (3)
-
Garrett Wollman -
Paul Eggert -
Roland McGrath