[tz] Bad 32 bit data in 2018f

Paul Eggert eggert at cs.ucla.edu
Thu Oct 25 19:19:34 UTC 2018


On 10/25/18 8:38 AM, Daniel Fischer wrote:
> But if the Qt workaround is disabled, there's a buffer overrun now.

Thanks for catching that; proposed patch attached (and installed into 
GitHub). Please give it a try. The fix is in the patch's last hunk.

>      sed -ibak 's/\(WORK_AROUND_QTBUG_53071 = \)true/\1false/' zic.c
That's awkward. The attached patch surrounds the constant's definition 
with an ifndef so that you don't need to modify the source code to 
disable the Qt bug workaround.

> NB, the allocation of ats as nats * 9 byte at the start of writezone() 
> might not be as intended:

It's intended, though now that you mention it I see that there could be 
an issue if malloc (N) assumes that an odd-aligned pointer can be 
returned when N is odd. The C Standard allows this behavior. The 
attached patch should fix this bug too.

Thanks again for your careful analysis of the code.

-------------- next part --------------
From 72b8edec275a85248b84eb31f37c14e5cda5c607 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert at cs.ucla.edu>
Date: Thu, 25 Oct 2018 10:52:04 -0700
Subject: [PATCH] Fix zic use of uninitialized storage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Daniel Fischer in:
https://mm.icann.org/pipermail/tz/2018-October/027096.html
* zic.c (_Alignof) [__STDC_VERSION__ < 201112]: New macro.
(WORK_AROUND_QTBUG_53071): Don’t define if already
defined, so you can compile with -DWORK_AROUND_QTBUG_53071=0
without having to change the source code.
(align_to): New function.
(writezone): Use it.
Avoid accessing uninitialized storage when timecnt32 == 0.
---
 zic.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/zic.c b/zic.c
index 4c94254..cb1bf28 100644
--- a/zic.c
+++ b/zic.c
@@ -64,6 +64,11 @@ typedef int_fast64_t	zic_t;
 static ptrdiff_t const PTRDIFF_MAX = MAXVAL(ptrdiff_t, TYPE_BIT(ptrdiff_t));
 #endif
 
+/* The minimum alignment of a type, for pre-C11 platforms.  */
+#if __STDC_VERSION__ < 201112
+# define _Alignof(type) offsetof(struct { char a; type b; }, b)
+#endif
+
 /* The type for line numbers.  Use PRIdMAX to format them; formerly
    there was also "#define PRIdLINENO PRIdMAX" and formats used
    PRIdLINENO, but xgettext cannot grok that.  */
@@ -189,7 +194,9 @@ enum { PERCENT_Z_LEN_BOUND = sizeof "+995959" - 1 };
    QTBUG-53071 <https://bugreports.qt.io/browse/QTBUG-53071>.  This
    workaround will no longer be needed when Qt 5.6.1 and earlier are
    obsolete, say in the year 2021.  */
+#ifndef WORK_AROUND_QTBUG_53071
 enum { WORK_AROUND_QTBUG_53071 = true };
+#endif
 
 static int		charcnt;
 static bool		errors;
@@ -431,6 +438,16 @@ size_product(size_t nitems, size_t itemsize)
 	return nitems * itemsize;
 }
 
+static ATTRIBUTE_PURE size_t
+align_to(size_t size, size_t alignment)
+{
+  size_t aligned_size = size + alignment - 1;
+  aligned_size -= aligned_size % alignment;
+  if (aligned_size < size)
+    memory_exhausted(_("alignment overflow"));
+  return aligned_size;
+}
+
 #if !HAVE_STRDUP
 static char *
 strdup(char const *str)
@@ -1740,7 +1757,11 @@ writezone(const char *const name, const char *const string, char version,
 	zic_t one = 1;
 	zic_t y2038_boundary = one << 31;
 	ptrdiff_t nats = timecnt + WORK_AROUND_QTBUG_53071;
-	zic_t *ats = emalloc(size_product(nats, sizeof *ats + 1));
+
+	/* Allocate the ATS and TYPES arrays via a single malloc,
+	   as this is a bit faster.  */
+	zic_t *ats = emalloc(align_to(size_product(nats, sizeof *ats + 1),
+				      _Alignof(zic_t)));
 	void *typesptr = ats + nats;
 	unsigned char *types = typesptr;
 
@@ -1827,17 +1848,17 @@ writezone(const char *const name, const char *const string, char version,
 	leapi32 = 0;
 	while (0 < timecnt32 && INT32_MAX < ats[timecnt32 - 1])
 		--timecnt32;
-	while (0 < timecnt32 && ats[timei32] < INT32_MIN) {
+	while (1 < timecnt32 && ats[timei32] < INT32_MIN
+	       && ats[timei32 + 1] <= INT32_MIN) {
+		/* Discard too-low transitions, except keep any last too-low
+		   transition if no transition is exactly at INT32_MIN.
+		   The kept transition will be output as an INT32_MIN
+		   "transition" appropriate for buggy 32-bit clients that do
+		   not use time type 0 for timestamps before the first
+		   transition; see below.  */
 		--timecnt32;
 		++timei32;
 	}
-	/*
-	** Output an INT32_MIN "transition" if appropriate; see below.
-	*/
-	if (timei32 > 0 && ats[timei32] > INT32_MIN) {
-		--timei32;
-		++timecnt32;
-	}
 	while (0 < leapcnt32 && INT32_MAX < trans[leapcnt32 - 1])
 		--leapcnt32;
 	while (0 < leapcnt32 && trans[leapi32] < INT32_MIN) {
-- 
2.17.2



More information about the tz mailing list