From patchwork Tue Jan 20 13:10:23 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 431054 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 10468140277 for ; Wed, 21 Jan 2015 00:10:47 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type; q=dns; s=default; b= rp+UTcwfDopjhj1LBflGtW8lFBLEdRwlBk+38N/QNx1/wmV/8K3n5PYKQPziea+E c7TBwGa2r4WbJlnYRn8CeGTt72KBlz1qwwKN6L2C3V2Z4Nzr2VQt+ivNW2Wh6LDO 1uhISUX/TkxlsygGWyOPJoAsKnZGd9Y0KM/cW4J1vOQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type; s=default; bh=+Y0Z zJwhzKXrci/Bm4t9m8fIGi0=; b=YqfTarY8sXOVW7mInxMCSLpOLwX2ou87MaVZ vUyKplRtPBmi/G9pKqFoUoAbNEgkq1keftaPcx/mjmM/z5HrjQgNgb5leZ4awXP2 b5CLMfbGpoB7mUsg2b3UN2cG4EyJRO5WSM5k2eVS2t8K80tE5sCaX7oB4bJqKYsD 0+uTvyo= Received: (qmail 8746 invoked by alias); 20 Jan 2015 13:10:41 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 8736 invoked by uid 89); 20 Jan 2015 13:10:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Message-ID: <54BE53BF.1080906@redhat.com> Date: Tue, 20 Jan 2015 14:10:23 +0100 From: Florian Weimer User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Joseph Myers CC: GNU C Library Subject: Re: [PATCH] tzset robustness [BZ#17715] References: <54B6E99E.4030109@redhat.com> In-Reply-To: On 01/15/2015 09:25 PM, Joseph Myers wrote: > On Wed, 14 Jan 2015, Florian Weimer wrote: > >> @@ -434,6 +437,10 @@ __tzfile_read (const char *file, size_t extra, char **extrap) >> goto lose; >> >> tzspec_len = st.st_size - off - 1; >> + if (tzspec_len >= 256) >> + /* POSIX time zone specifiers are much shorter than 256 >> + characters. */ >> + goto lose; >> char *tzstr = alloca (tzspec_len); >> if (getc_unlocked (f) != '\n' >> || (__fread_unlocked (tzstr, 1, tzspec_len - 1, f) > > Is it possible to have tzspec_len == 0 here? The code doesn't look safe > if tzspec_len is 0 - it would pass (size_t)-1 to __fread_unlocked. > > This code is for the case where time_t is 4-byte (and so size_t is > 4-byte). tzspec_len is of type size_t. st.st_size is of type off64_t (st > is struct stat64), so 8-byte. If st.st_size < off + 2 we didn't get here, > but if st.st_size is off + 4GB + 1 it seems to me you could then get > tzspec_len being 0. (This file is opened with fopen not fopen64 so the > open should fail if it's a large file at fopen time, but one might suppose > it only becomes a large file between the fopen call and the fstat64 call.) I have verified that the problem you describe actually exists, with another test case and by replacing fopen with fopen64. Thanks. This is probably a real bug on x32, which has a 64-bit off_t. The test case also revealed that the code would read an arbitrary large POSIX time zone specifier on 64-bit architectures. I added another check to catch that, too. Re-tested on x86_64 and i386. From 86c7d40e7d6036075cecce566c37464346ed2ab2 Mon Sep 17 00:00:00 2001 From: Florian Weimer Date: Tue, 20 Jan 2015 13:37:40 +0100 Subject: [PATCH] Make time zone file parser more robust [BZ #17715] 2015-01-20 Florian Weimer [BZ #17715] * time/tzfile.c (__tzfile_read): Check for large values of tzh_ttisstdcnt and tzh_ttisgmtcnt. Reject overlong POSIX time zone specifiers. * time/tzset.c (__tzset_parse_tz): Guard against large time zone specifiers. * timezone/Makefile (tests): Add tst-tzset. (tst-tzset.out): Dependencies on time zone files. (tst-tzset-ENV): Set TZDIR. (testdata/XT%): Copy crafted time zone files. * timezone/README: Mention crafted time zone files. * timezone/testdata/XT1, timezone/testdata/XT2, timezone/testdata/XT3, timezone/testdata/XT4: New time zone test files. * timezone/tst-tzset.c: New test. diff --git a/NEWS b/NEWS index 9086b7e..7a8e654 100644 --- a/NEWS +++ b/NEWS @@ -15,9 +15,9 @@ Version 2.21 17501, 17506, 17508, 17522, 17555, 17570, 17571, 17572, 17573, 17574, 17582, 17583, 17584, 17585, 17589, 17594, 17601, 17608, 17616, 17625, 17630, 17633, 17634, 17635, 17647, 17653, 17657, 17658, 17664, 17665, - 17668, 17682, 17717, 17719, 17722, 17723, 17724, 17725, 17732, 17733, - 17744, 17745, 17746, 17747, 17748, 17775, 17777, 17780, 17781, 17782, - 17791, 17793, 17796, 17797, 17803, 17806, 17834, 17844, 17848 + 17668, 17682, 17715, 17717, 17719, 17722, 17723, 17724, 17725, 17732, + 17733, 17744, 17745, 17746, 17747, 17748, 17775, 17777, 17780, 17781, + 17782, 17791, 17793, 17796, 17797, 17803, 17806, 17834, 17844, 17848 * Port to Altera Nios II has been contributed by Mentor Graphics. @@ -50,6 +50,12 @@ Version 2.21 infinite loop if the DNS response contained a PTR record of an unexpected format. +* The time zone file parser has been made more robust against crafted time + zone files, avoiding heap buffer overflows related to the processing of + the tzh_ttisstdcnt and tzh_ttisgmtcnt fields, and a stack overflow due to + large time zone data files. Overly long time zone specifiers in the TZ + variable no longer result in stack overflows and crashes. + * The minimum GCC version that can be used to build this version of the GNU C Library is GCC 4.6. Older GCC versions, and non-GNU compilers, can still be used to compile programs using the GNU C Library. diff --git a/time/tzfile.c b/time/tzfile.c index bcb408f..c2ddd01 100644 --- a/time/tzfile.c +++ b/time/tzfile.c @@ -200,6 +200,9 @@ __tzfile_read (const char *file, size_t extra, char **extrap) num_isstd = (size_t) decode (tzhead.tzh_ttisstdcnt); num_isgmt = (size_t) decode (tzhead.tzh_ttisgmtcnt); + if (__glibc_unlikely (num_isstd > num_types || num_isgmt > num_types)) + goto lose; + /* For platforms with 64-bit time_t we use the new format if available. */ if (sizeof (time_t) == 8 && trans_width == 4 && tzhead.tzh_version[0] != '\0') @@ -270,7 +273,10 @@ __tzfile_read (const char *file, size_t extra, char **extrap) if (__glibc_unlikely (SIZE_MAX - total_size < tzspec_len)) goto lose; } - if (__glibc_unlikely (SIZE_MAX - total_size - tzspec_len < extra)) + if (__glibc_unlikely (SIZE_MAX - total_size - tzspec_len < extra + || tzspec_len >= 256)) + /* POSIX time zone specifiers are much shorter than 256 + characters. */ goto lose; /* Allocate enough memory including the extra block requested by the @@ -434,6 +440,10 @@ __tzfile_read (const char *file, size_t extra, char **extrap) goto lose; tzspec_len = st.st_size - off - 1; + if (__glibc_unlikely (tzspec_len == 0 || tzspec_len >= 256)) + /* POSIX time zone specifiers are much shorter than 256 + characters. */ + goto lose; char *tzstr = alloca (tzspec_len); if (getc_unlocked (f) != '\n' || (__fread_unlocked (tzstr, 1, tzspec_len - 1, f) diff --git a/time/tzset.c b/time/tzset.c index 8bc7a2e..59fd10d 100644 --- a/time/tzset.c +++ b/time/tzset.c @@ -176,11 +176,14 @@ __tzset_parse_tz (tz) memset (tz_rules, '\0', sizeof tz_rules); tz_rules[0].name = tz_rules[1].name = ""; - /* Get the standard timezone name. */ - char *tzbuf = strdupa (tz); + /* POSIX time zone specifiers are much shorter than 256 characters. */ + char tzbuf[256]; + if (strlen (tz) > sizeof (tzbuf) - 1) + goto out; + /* Get the standard timezone name. */ int consumed; - if (sscanf (tz, "%[A-Za-z]%n", tzbuf, &consumed) != 1) + if (sscanf (tz, "%255[A-Za-z]%n", tzbuf, &consumed) != 1) { /* Check for the quoted version. */ char *wp = tzbuf; @@ -227,7 +230,7 @@ __tzset_parse_tz (tz) /* Get the DST timezone name (if any). */ if (*tz != '\0') { - if (sscanf (tz, "%[A-Za-z]%n", tzbuf, &consumed) != 1) + if (sscanf (tz, "%255[A-Za-z]%n", tzbuf, &consumed) != 1) { /* Check for the quoted version. */ char *wp = tzbuf; diff --git a/timezone/Makefile b/timezone/Makefile index 17424b8..5f18545 100644 --- a/timezone/Makefile +++ b/timezone/Makefile @@ -25,7 +25,7 @@ include ../Makeconfig extra-objs := scheck.o ialloc.o others := zdump zic -tests := test-tz tst-timezone +tests := test-tz tst-timezone tst-tzset # pacificnew doesn't compile; if it is to be used, it should be included in # northamerica. @@ -90,9 +90,11 @@ $(objpfx)tst-timezone.out: $(addprefix $(testdata)/, \ Australia/Melbourne \ America/Sao_Paulo Asia/Tokyo \ Europe/London) +$(objpfx)tst-tzset.out: $(addprefix $(testdata)/XT, 1 2 3 4) test-tz-ENV = TZDIR=$(testdata) tst-timezone-ENV = TZDIR=$(testdata) +tst-tzset-ENV = TZDIR=$(testdata) # Note this must come second in the deps list for $(built-program-cmd) to work. zic-deps = $(objpfx)zic $(leapseconds) yearistype @@ -114,6 +116,8 @@ $(testdata)/America/Sao_Paulo: southamerica $(zic-deps) $(testdata)/Asia/Tokyo: asia $(zic-deps) $(build-testdata) +$(testdata)/XT%: testdata/XT% + cp $< $@ $(objpfx)tzselect: tzselect.ksh $(common-objpfx)config.make sed -e 's|/bin/bash|$(BASH)|' \ diff --git a/timezone/README b/timezone/README index 7a5e31c..2268f8e 100644 --- a/timezone/README +++ b/timezone/README @@ -15,3 +15,6 @@ version of the tzcode and tzdata packages. These packages may be found at ftp://ftp.iana.org/tz/releases/. Commentary should be addressed to tz@iana.org. + +The subdirectory testdata contains manually edited data files for +regression testing purposes. diff --git a/timezone/testdata/XT1 b/timezone/testdata/XT1 new file mode 100644 index 0000000000000000000000000000000000000000..67d7ee0ba59389b4aaea0bdce15ff6bf7056c5ef GIT binary patch literal 127 zcmWHE%1kq2Kn4GS04TzYB+3Y6vq1O}A%;Lk2w{C7Jz#x5AR1vL!~iZJWxxdhA3F~_ literal 0 HcmV?d00001 diff --git a/timezone/testdata/XT2 b/timezone/testdata/XT2 new file mode 100644 index 0000000000000000000000000000000000000000..069189e34998662d526b3645891d515a31fd6495 GIT binary patch literal 127 wcmWHE%1kq2zyQqufdEOA5y)nN@FPM%>O%. */ + +#define _GNU_SOURCE 1 + +#include +#include +#include +#include +#include +#include + +static int do_test (void); +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" + +/* Returns the name of a large TZ file. */ +static char * +create_tz_file (off64_t size) +{ + char *path; + int fd = create_temp_file ("tst-tzset-", &path); + if (fd < 0) + exit (1); + + // Reopen for large-file support. + close (fd); + fd = open64 (path, O_WRONLY); + if (fd < 0) + { + printf ("open64 (%s) failed: %m\n", path); + exit (1); + } + + static const char data[] = { + 0x54, 0x5a, 0x69, 0x66, 0x32, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x58, 0x54, 0x47, 0x00, 0x00, 0x00, + 0x54, 0x5a, 0x69, 0x66, 0x32, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x04, 0xf8, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x58, 0x54, 0x47, 0x00, 0x00, + 0x00, 0x0a, 0x58, 0x54, 0x47, 0x30, 0x0a + }; + ssize_t ret = write (fd, data, sizeof (data)); + if (ret < 0) + { + printf ("write failed: %m\n"); + exit (1); + } + if ((size_t) ret != sizeof (data)) + { + printf ("Short write\n"); + exit (1); + } + if (lseek64 (fd, size, SEEK_CUR) < 0) + { + printf ("lseek failed: %m\n"); + close (fd); + return NULL; + } + if (write (fd, "", 1) != 1) + { + printf ("Single-byte write failed\n"); + close (fd); + return NULL; + } + if (close (fd) != 0) + { + printf ("close failed: %m\n"); + exit (1); + } + return path; +} + +static void +test_tz_file (off64_t size) +{ + char *path = create_tz_file (size); + if (setenv ("TZ", path, 1) < 0) + { + printf ("setenv failed: %m\n"); + exit (1); + } + tzset (); + free (path); +} + +static int +do_test (void) +{ + int errors = 0; + for (int i = 1; i <= 4; ++i) + { + char tz[16]; + snprintf (tz, sizeof (tz), "XT%d", i); + if (setenv ("TZ", tz, 1) < 0) + { + printf ("setenv failed: %m\n"); + return 1; + } + tzset (); + if (strcmp (tzname[0], tz) == 0) + { + printf ("Unexpected success for %s\n", tz); + ++errors; + } + } + + /* Large TZ files. */ + + /* This will succeed on 64-bit architectures, and fail on 32-bit + architectures. It used to crash on 32-bit. */ + test_tz_file (64 * 1024 * 1024); + + /* This will fail on 64-bit and 32-bit architectures. It used to + cause a test timeout on 64-bit and crash on 32-bit if the TZ file + open succeeded for some reason (it does not use O_LARGEFILE in + regular builds). */ + test_tz_file (4LL * 1024 * 1024 * 1024 - 6); + + /* Large TZ variable. */ + { + size_t length = 64 * 1024 * 1024; + char *value = malloc (length + 1); + if (value == NULL) + { + puts ("malloc failed: %m"); + return 1; + } + value[length] = '\0'; + memset (value, ' ', length); + value[0] = 'U'; + value[1] = 'T'; + value[2] = 'C'; + if (setenv ("TZ", value, 1) < 0) + { + printf ("setenv failed: %m\n"); + return 1; + } + tzset (); + } + + return errors > 0; +} -- 2.1.0