[12/59] Check TZif files more carefully

Message ID 20250105055750.1668721-13-eggert@cs.ucla.edu (mailing list archive)
State New
Headers
Series time: sync mktime from Gnulib |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Paul Eggert Jan. 5, 2025, 5:56 a.m. UTC
  This tests the contents of TZif files more carefully for
conformance to RFC 9636, in areas where non-conformance
might plausibly mess up glibc or application behavior.
It also removes some nonportable assumptions
about alignment and arithmetic.
* time/tzfile.c: Include stdalign.h, stdckdint.h.
(tzidx): New type.  Prefer it to size_t for TZif-related counts
that must fit in 32-bit unsigned integer.  This is mostly for clarity.
(tzidx_decode): New function, for obtaining tzidx values.
All uses of decode changed, when tzidx values are wanted.
This fixes some potential misuse of values with the top bit set.
(decode): Reimplement using tzidx_decode.
(decode64): Don’t assume sizeof (int64_t) == 8; admittedly
this is theoretical.  Simplify the usual case.
(__tzfile_read): Use unions to guarantee proper alignment.
Check that there is at least one time type, since we always use at
least the zeroth entry.  Check for overflow in size and offset
computations.  Check that the TZif's file TZ string is either
empty or at least 4 bytes long (not counting newlines).
Avoid aliasing issues when unpacking transitions.
Fix off-by-one bug when checking for bogus index.
Check that all time zone abbreviations are null-terminated.
Check that leap seconds are in ascending order, that they are all
either insertions or deletions of a single second, and that they
are at least 2419199 seconds apart.
(__tzfile_default): Defend against negating INT_MIN.
(__tzfile_compute): Args are now int * and bool *, not
long int * and int *.  All uses changed.  Defend against
unlikely integer overflows leading to misbehavior.
Do not worry about adjacent leap seconds, as it cannot happen
even though a bug in the original C standard said it could;
this is why we can now use bool * not int * for leap hits.
---
 time/tzfile.c | 369 ++++++++++++++++++++++++++++++--------------------
 time/tzset.c  |  21 ++-
 time/tzset.h  |   3 +-
 3 files changed, 241 insertions(+), 152 deletions(-)
  

Patch

diff --git a/time/tzfile.c b/time/tzfile.c
index 15d1890892..e60c3c0b9c 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -17,6 +17,8 @@ 
 
 #include <assert.h>
 #include <limits.h>
+#include <stdalign.h>
+#include <stdckdint.h>
 #include <stdio.h>
 #include <stdio_ext.h>
 #include <stdlib.h>
@@ -48,17 +50,23 @@  struct ttinfo
 struct leap
   {
     __time64_t transition;	/* Time the transition takes effect.  */
-    long int change;		/* Seconds of correction to apply.  */
+    int change;			/* Seconds of correction to apply.  */
   };
 
-static size_t num_transitions;
+/* Internal index derived from a count taken from a TZif file.
+   These counts are at most 2**32 - 1; see Internet RFC 9636.
+   Although this type is currently unsigned to save time and/or space,
+   code ordinarily should not assume it is unsigned.  */
+typedef unsigned int tzidx;
+
+static tzidx num_transitions;
 static __time64_t *transitions;
 static unsigned char *type_idxs;
-static size_t num_types;
+static tzidx num_types;
 static struct ttinfo *types;
 static char *zone_names;
 static long int rule_stdoff;
-static size_t num_leaps;
+static tzidx num_leaps;
 static struct leap *leaps;
 static char *tzspec;
 
@@ -69,19 +77,25 @@  static int daylight_saved;
 #include <endian.h>
 #include <byteswap.h>
 
-/* Decode the four bytes at PTR as a signed integer in network byte order.  */
-static inline int
+/* Decode the four bytes at PTR as a tzidx in network byte order.  */
+static inline tzidx
 __attribute ((always_inline))
-decode (const void *ptr)
+tzidx_decode (const void *ptr)
 {
-  if (BYTE_ORDER == BIG_ENDIAN && sizeof (int) == 4)
-    return *(const int *) ptr;
-  if (sizeof (int) == 4)
-    return bswap_32 (*(const int *) ptr);
-
-  const unsigned char *p = ptr;
-  int result = *p & (1 << (CHAR_BIT - 1)) ? ~0 : 0;
+  if (sizeof (tzidx) == 4
+      && offsetof (struct tzhead, tzh_timecnt) % alignof (tzidx) == 0
+      && offsetof (struct tzhead, tzh_typecnt) % alignof (tzidx) == 0
+      && offsetof (struct tzhead, tzh_charcnt) % alignof (tzidx) == 0
+      && offsetof (struct tzhead, tzh_leapcnt) % alignof (tzidx) == 0
+      && offsetof (struct tzhead, tzh_ttisstdcnt) % alignof (tzidx) == 0
+      && offsetof (struct tzhead, tzh_ttisutcnt) % alignof (tzidx) == 0)
+    {
+      tzidx const *uptr = ptr, i = *uptr;
+      return BYTE_ORDER == BIG_ENDIAN ? i : bswap_32 (i);
+    }
 
+  unsigned char const *p = ptr;
+  unsigned long result = 0;
   result = (result << 8) | *p++;
   result = (result << 8) | *p++;
   result = (result << 8) | *p++;
@@ -90,15 +104,37 @@  decode (const void *ptr)
   return result;
 }
 
+/* Decode the four bytes at PTR as a signed int in network byte order.  */
+static inline int
+__attribute ((always_inline))
+decode (const void *ptr)
+{
+  tzidx i = tzidx_decode (ptr);
+  if ((UINT_MAX >> 31) == 1 && INT_MAX == UINT_MAX / 2 && (int) UINT_MAX == -1)
+    return i;
+
+  /* An unusual platform.  Explicitly extend the sign.  */
+  if (i <= INT_MAX)
+    return i;
+  int r = ~i;
+  return ~r;
+}
 
 static inline int64_t
 __attribute ((always_inline))
 decode64 (const void *ptr)
 {
-  if ((BYTE_ORDER == BIG_ENDIAN))
-    return *(const int64_t *) ptr;
+  if (sizeof (int64_t) == 8)
+    {
+      int64_t const *p = ptr, i = *p;
+      return BYTE_ORDER == BIG_ENDIAN ? i : bswap_64 (i);
+    }
 
-  return bswap_64 (*(const int64_t *) ptr);
+  unsigned char const *p = ptr;
+  uint64_t result = 0; /* uint64_t avoids undefined behavior with <<.  */
+  for (int i = 0; i < 8; i++)
+    result = (result << 8) | p[i];
+  return result;
 }
 
 
@@ -106,11 +142,11 @@  void
 __tzfile_read (const char *file)
 {
   static const char default_tzdir[] = TZDIR;
-  size_t num_isstd, num_isgmt;
+  tzidx num_isstd, num_isgmt;
   FILE *f;
-  struct tzhead tzhead;
-  size_t chars;
-  size_t i;
+  union { struct tzhead tzhead; tzidx tzidx_aligned; } u;
+  tzidx chars;
+  tzidx i;
   int was_using_tzfile = __use_tzfile;
   int trans_width = 4;
   char *new = NULL;
@@ -174,7 +210,7 @@  __tzfile_read (const char *file)
   if (__fstat64_time64 (__fileno (f), &st) != 0)
     goto lose;
 
-  free ((void *) transitions);
+  free (transitions);
   transitions = NULL;
 
   /* Remember the inode and device number and modification time.  */
@@ -186,34 +222,41 @@  __tzfile_read (const char *file)
   __fsetlocking (f, FSETLOCKING_BYCALLER);
 
  read_again:
-  if (__builtin_expect (__fread_unlocked ((void *) &tzhead, sizeof (tzhead),
-					  1, f) != 1, 0)
-      || memcmp (tzhead.tzh_magic, TZ_MAGIC, sizeof (tzhead.tzh_magic)) != 0)
+  if (__builtin_expect (__fread_unlocked (&u, sizeof u.tzhead, 1, f) != 1, 0)
+      || memcmp (u.tzhead.tzh_magic, TZ_MAGIC, sizeof u.tzhead.tzh_magic) != 0)
     goto lose;
+  f_offset += sizeof u.tzhead;
 
-  f_offset += sizeof tzhead;
-  num_transitions = (size_t) decode (tzhead.tzh_timecnt);
-  num_types = (size_t) decode (tzhead.tzh_typecnt);
-  chars = (size_t) decode (tzhead.tzh_charcnt);
-  num_leaps = (size_t) decode (tzhead.tzh_leapcnt);
-  num_isstd = (size_t) decode (tzhead.tzh_ttisstdcnt);
-  num_isgmt = (size_t) decode (tzhead.tzh_ttisutcnt);
+  num_transitions = tzidx_decode (u.tzhead.tzh_timecnt);
+  num_types = tzidx_decode (u.tzhead.tzh_typecnt);
+  chars = tzidx_decode (u.tzhead.tzh_charcnt);
+  num_leaps = tzidx_decode (u.tzhead.tzh_leapcnt);
+  num_isstd = tzidx_decode (u.tzhead.tzh_ttisstdcnt);
+  num_isgmt = tzidx_decode (u.tzhead.tzh_ttisutcnt);
 
-  if (__glibc_unlikely (num_isstd > num_types || num_isgmt > num_types))
+  if (__glibc_unlikely (num_isstd > num_types || num_isgmt > num_types
+			|| !num_types))
     goto lose;
 
-  if (trans_width == 4 && tzhead.tzh_version[0] != '\0')
+  if (trans_width == 4 && u.tzhead.tzh_version[0] != '\0')
     {
       /* We use the 8-byte format.  */
       trans_width = 8;
 
       /* Position the stream before the second header.  */
-      size_t to_skip = (num_transitions * (4 + 1)
-			+ num_types * 6
-			+ chars
-			+ num_leaps * 8
-			+ num_isstd
-			+ num_isgmt);
+      off_t to_skip, product;
+      bool v = false;
+      v |= ckd_mul (&to_skip, num_transitions, 4 + 1);
+      v |= ckd_mul (&product, num_types, 6);
+      v |= ckd_add (&to_skip, to_skip, product);
+      v |= ckd_add (&to_skip, to_skip, chars);
+      v |= ckd_mul (&product, num_leaps, 8);
+      v |= ckd_add (&to_skip, to_skip, product);
+      v |= ckd_add (&to_skip, to_skip, num_isstd);
+      v |= ckd_add (&to_skip, to_skip, num_isgmt);
+      if (v)
+	goto lose;
+
       if (fseek (f, to_skip, SEEK_CUR) != 0)
 	goto lose;
 
@@ -222,34 +265,29 @@  __tzfile_read (const char *file)
     }
 
   /* Compute the size of the POSIX time zone specification in the
-     file.  */
-  size_t tzspec_len;
+     file.  This includes the trailing but not the leading newline.  */
+  size_t tzspec_size;
   if (trans_width == 8)
     {
-      off_t rem = st.st_size - f_offset;
-      if (__builtin_expect (rem < 0
-			    || (size_t) rem < (num_transitions * (8 + 1)
-					       + num_types * 6
-					       + chars), 0))
-	goto lose;
-      tzspec_len = (size_t) rem - (num_transitions * (8 + 1)
-				   + num_types * 6
-				   + chars);
-      if (__builtin_expect (num_leaps > SIZE_MAX / 12
-			    || tzspec_len < num_leaps * 12, 0))
-	goto lose;
-      tzspec_len -= num_leaps * 12;
-      if (__glibc_unlikely (tzspec_len < num_isstd))
-	goto lose;
-      tzspec_len -= num_isstd;
-      if (__glibc_unlikely (tzspec_len == 0 || tzspec_len - 1 < num_isgmt))
-	goto lose;
-      tzspec_len -= num_isgmt + 1;
-      if (tzspec_len == 0)
+      off_t rem = st.st_size - f_offset, product;
+      bool v = false;
+      v |= ckd_mul (&product, num_transitions, 8 + 1);
+      v |= ckd_sub (&rem, rem, product);
+      v |= ckd_mul (&product, num_types, 6);
+      v |= ckd_sub (&rem, rem, product);
+      v |= ckd_sub (&rem, rem, chars);
+      v |= ckd_mul (&product, num_leaps, 12);
+      v |= ckd_sub (&rem, rem, product);
+      v |= ckd_sub (&rem, rem, num_isstd);
+      v |= ckd_sub (&rem, rem, num_isgmt);
+      v |= rem <= 1;
+      v |= 3 <= rem && rem < sizeof "XYZ0\n";
+      if (v)
 	goto lose;
+      tzspec_size = rem - 1;
     }
   else
-    tzspec_len = 0;
+    tzspec_size = 0;
 
   /* The file is parsed into a single heap allocation, comprising of
      the following arrays:
@@ -259,7 +297,7 @@  __tzfile_read (const char *file)
      struct ttinfo types[num_types];
      unsigned char type_idxs[num_types];
      char zone_names[chars];
-     char tzspec[tzspec_len];
+     char tzspec[tzspec_size];
 
      The piece-wise allocations from buf below verify that no
      overflow/wraparound occurred in these computations.
@@ -274,12 +312,18 @@  __tzfile_read (const char *file)
 		  "alignment of struct leap");
   struct alloc_buffer buf;
   {
-    size_t total_size = (num_transitions * sizeof (__time64_t)
-			 + num_leaps * sizeof (struct leap)
-			 + num_types * sizeof (struct ttinfo)
-			 + num_transitions /* type_idxs */
-			 + chars /* zone_names */
-			 + tzspec_len);
+    size_t total_size, product;
+    bool v = false;
+    v |= ckd_mul (&total_size, num_transitions, sizeof (__time64_t));
+    v |= ckd_mul (&product, num_leaps, sizeof (struct leap));
+    v |= ckd_add (&total_size, total_size, product);
+    v |= ckd_mul (&product, num_types, sizeof (struct ttinfo));
+    v |= ckd_add (&total_size, total_size, product);
+    v |= ckd_add (&total_size, total_size, num_transitions); /* type_idxs */
+    v |= ckd_add (&total_size, total_size, chars); /* zone_names */
+    v |= ckd_add (&total_size, total_size, tzspec_size);
+    if (v)
+      goto lose;
     transitions = malloc (total_size);
     if (transitions == NULL)
       goto lose;
@@ -294,18 +338,53 @@  __tzfile_read (const char *file)
   type_idxs = alloc_buffer_alloc_array (&buf, unsigned char, num_transitions);
   zone_names = alloc_buffer_alloc_array (&buf, char, chars);
   if (trans_width == 8)
-    tzspec = alloc_buffer_alloc_array (&buf, char, tzspec_len);
+    tzspec = alloc_buffer_alloc_array (&buf, char, tzspec_size);
   else
     tzspec = NULL;
   if (alloc_buffer_has_failed (&buf))
     goto lose;
 
-  if (__glibc_unlikely (__fread_unlocked (transitions, trans_width,
-					  num_transitions, f)
-			!= num_transitions)
-      || __glibc_unlikely (__fread_unlocked (type_idxs, 1, num_transitions, f)
-			   != num_transitions))
+  if (trans_width == 4)
+    {
+      for (i = 0; i < num_transitions; i++)
+	{
+	  union { unsigned char c[4]; tzidx tzidx_aligned; } x;
+	  if (__fread_unlocked (&x, 1, 4, f) != 4)
+	    goto lose;
+	  transitions[i] = decode (&x);
+	  if (i && transitions[i] <= transitions[i - 1])
+	    goto lose;
+	}
+    }
+  else if (sizeof (int64_t) == 8)
+    {
+      if (__fread_unlocked (transitions, 8, num_transitions, f)
+	  != num_transitions)
 	goto lose;
+      for (i = 0; i < num_transitions; i++)
+	{
+	  if (BYTE_ORDER != BIG_ENDIAN)
+	    transitions[i] = decode64 (&transitions[i]);
+	  if (i && transitions[i] <= transitions[i - 1])
+	    goto lose;
+	}
+    }
+  else
+    {
+      for (i = 0; i < num_transitions; i++)
+	{
+	  union { unsigned char c[8]; int64_t int64_t_aligned; } x;
+	  if (__fread_unlocked (&x, 1, 8, f) != 8)
+	    goto lose;
+	  transitions[i] = decode64 (&x);
+	  if (i && transitions[i] <= transitions[i - 1])
+	    goto lose;
+	}
+    }
+
+  if (__glibc_unlikely (__fread_unlocked (type_idxs, 1, num_transitions, f)
+			!= num_transitions))
+    goto lose;
 
   /* Check for bogus indices in the data file, so we can hereafter
      safely use type_idxs[T] as indices into `types' and never crash.  */
@@ -313,61 +392,67 @@  __tzfile_read (const char *file)
     if (__glibc_unlikely (type_idxs[i] >= num_types))
       goto lose;
 
-  if (trans_width == 4)
-    {
-      /* Decode the transition times, stored as 4-byte integers in
-	 network (big-endian) byte order.  We work from the end of the
-	 array so as not to clobber the next element to be
-	 processed.  */
-      i = num_transitions;
-      while (i-- > 0)
-	transitions[i] = decode ((char *) transitions + i * 4);
-    }
-  else if (BYTE_ORDER != BIG_ENDIAN)
-    {
-      /* Decode the transition times, stored as 8-byte integers in
-	 network (big-endian) byte order.  */
-      for (i = 0; i < num_transitions; ++i)
-	transitions[i] = decode64 ((char *) transitions + i * 8);
-    }
-
+  unsigned char max_idx = 0;
   for (i = 0; i < num_types; ++i)
     {
-      unsigned char x[4];
+      union { unsigned char c[4]; tzidx tzidx_aligned; } x;
       int c;
-      if (__builtin_expect (__fread_unlocked (x, 1,
-					      sizeof (x), f) != sizeof (x),
-			    0))
+      if (__builtin_expect (__fread_unlocked (&x, 1, 4, f) != 4, 0))
 	goto lose;
       c = __getc_unlocked (f);
-      if (__glibc_unlikely ((unsigned int) c > 1u))
+      if (__glibc_unlikely (! (0 <= c && c <= 1)))
 	goto lose;
       types[i].isdst = c;
       c = __getc_unlocked (f);
-      if (__glibc_unlikely ((size_t) c > chars))
+      if (__glibc_unlikely (! (0 <= c && c < chars)))
 	/* Bogus index in data file.  */
 	goto lose;
+      if (max_idx < c)
+	max_idx = c;
       types[i].idx = c;
-      types[i].offset = decode (x);
+      types[i].offset = decode (&x);
+
+      /* If long int is only 32 bits, reject offsets that cannot be negated.
+         RFC 9636 section 3.2 allows this.  */
+      long int negated_offset;
+      if (ckd_sub (&negated_offset, 0, types[i].offset))
+	goto lose;
     }
 
   if (__glibc_unlikely (__fread_unlocked (zone_names, 1, chars, f) != chars))
     goto lose;
+  if (__strnlen (zone_names + max_idx, chars - max_idx) == chars - max_idx)
+    goto lose;
 
+  int minimum_leap_gap = 2419199; /* See RFC 9636 section 3.2.  */
+  __time64_t prevtransition = -minimum_leap_gap;
+  int prevchange = 0;
   for (i = 0; i < num_leaps; ++i)
     {
-      unsigned char x[8];
-      if (__builtin_expect (__fread_unlocked (x, 1, trans_width, f)
-			    != trans_width, 0))
-	goto lose;
-      if (trans_width == 4)
-	leaps[i].transition = decode (x);
-      else
-	leaps[i].transition = decode64 (x);
-
-      if (__glibc_unlikely (__fread_unlocked (x, 1, 4, f) != 4))
-	goto lose;
-      leaps[i].change = (long int) decode (x);
+      {
+	union { char c[8]; tzidx tzidx_aligned; int64_t int64_t_aligned; } x;
+	if (__builtin_expect ((__fread_unlocked (&x, 1, trans_width, f)
+			       != trans_width),
+			      0))
+	  goto lose;
+	leaps[i].transition = trans_width == 4 ? decode (&x) : decode64 (&x);
+	if (leaps[i].transition < 0
+	    || leaps[i].transition - minimum_leap_gap < prevtransition)
+	  goto lose;
+	prevtransition = leaps[i].transition;
+      }
+
+      {
+	union { unsigned char c[4]; tzidx tzidx_aligned; } x;
+	if (__glibc_unlikely (__fread_unlocked (&x, 1, 4, f) != 4))
+	  goto lose;
+	leaps[i].change = decode (&x);
+	int delta_change;
+	if (ckd_sub (&delta_change, leaps[i].change, prevchange)
+	    || ! (delta_change == 1 || delta_change == -1))
+	  goto lose;
+	prevchange = leaps[i].change;
+      }
     }
 
   for (i = 0; i < num_isstd; ++i)
@@ -393,14 +478,16 @@  __tzfile_read (const char *file)
   /* Read the POSIX TZ-style information if possible.  */
   if (tzspec != NULL)
     {
-      assert (tzspec_len > 0);
-      /* Skip over the newline first.  */
-      if (__getc_unlocked (f) != '\n'
-	  || (__fread_unlocked (tzspec, 1, tzspec_len - 1, f)
-	      != tzspec_len - 1))
-	tzspec = NULL;
+      char *nl;
+      assert (tzspec_size > 0);
+      /* Skip the leading newline, then grab everything up to the next
+	 newline; ignore everything after that.  */
+      if (__getc_unlocked (f) == '\n'
+	  && __fread_unlocked (tzspec, 1, tzspec_size, f) == tzspec_size
+	  && (nl = memchr (tzspec, '\n', tzspec_size)) != NULL)
+	*nl = '\0';
       else
-	tzspec[tzspec_len - 1] = '\0';
+	tzspec = NULL;
     }
 
   /* Don't use an empty TZ string.  */
@@ -484,16 +571,16 @@  __tzfile_read (const char *file)
   fclose (f);
  ret_free_transitions:
   free (new);
-  free ((void *) transitions);
+  free (transitions);
   transitions = NULL;
 }
 
 void
 __tzfile_compute (__time64_t timer, int use_localtime,
-		  long int *leap_correct, int *leap_hit,
+		  int *leap_correct, bool *leap_hit,
 		  struct tm *tp)
 {
-  size_t i;
+  tzidx i;
 
   if (use_localtime)
     {
@@ -519,7 +606,7 @@  __tzfile_compute (__time64_t timer, int use_localtime,
 	  __tzname[0] = __tzstring (&zone_names[types[i].idx]);
 	  if (__tzname[1] == NULL)
 	    {
-	      size_t j = i;
+	      tzidx j = i;
 	      while (j < num_types)
 		if (types[j].isdst)
 		  {
@@ -556,15 +643,16 @@  __tzfile_compute (__time64_t timer, int use_localtime,
 	{
 	  /* Find the first transition after TIMER, and
 	     then pick the type of the transition before it.  */
-	  size_t lo = 0;
-	  size_t hi = num_transitions - 1;
+	  tzidx lo = 0;
+	  tzidx hi = num_transitions - 1;
 	  /* Assume that DST is changing twice a year and guess
 	     initial search spot from it.  Half of a gregorian year
 	     has on average 365.2425 * 86400 / 2 = 15778476 seconds.
-	     The value i can be truncated if size_t is smaller than
-	     __time64_t, but this is harmless because it is just
-	     a guess.  */
-	  i = (transitions[num_transitions - 1] - timer) / 15778476;
+	     Although i's value can be wrong if overflow occurs,
+	     this is harmless because it is just a guess.  */
+	  __time64_t tdiff;
+	  ckd_sub (&tdiff, transitions[num_transitions - 1], timer);
+	  ckd_add (&i, tdiff / 15778476, 0);
 	  if (i < num_transitions)
 	    {
 	      i = num_transitions - 1 - i;
@@ -581,7 +669,7 @@  __tzfile_compute (__time64_t timer, int use_localtime,
 		}
 	      else
 		{
-		  if (i + 10 >= num_transitions || timer < transitions[i + 10])
+		  if (num_transitions - i <= 10 || timer < transitions[i + 10])
 		    {
 		      /* Linear search.  */
 		      while (timer >= transitions[i])
@@ -596,7 +684,7 @@  __tzfile_compute (__time64_t timer, int use_localtime,
 	  /* assert (timer >= transitions[lo] && timer < transitions[hi]); */
 	  while (lo + 1 < hi)
 	    {
-	      i = (lo + hi) / 2;
+	      i = (lo >> 1) + (hi >> 1) + (lo & hi & 1);
 	      if (timer < transitions[i])
 		hi = i;
 	      else
@@ -609,7 +697,7 @@  __tzfile_compute (__time64_t timer, int use_localtime,
 	     && (i == num_transitions || timer < transitions[i])); */
 	  __tzname[types[type_idxs[i - 1]].isdst]
 	    = __tzstring (&zone_names[types[type_idxs[i - 1]].idx]);
-	  size_t j = i;
+	  tzidx j = i;
 	  while (j < num_transitions)
 	    {
 	      int type = type_idxs[j];
@@ -654,8 +742,8 @@  __tzfile_compute (__time64_t timer, int use_localtime,
     }
 
  leap:
-  *leap_correct = 0L;
-  *leap_hit = 0;
+  *leap_correct = 0;
+  *leap_hit = false;
 
   /* Find the last leap second correction transition time before TIMER.  */
   i = num_leaps;
@@ -669,16 +757,7 @@  __tzfile_compute (__time64_t timer, int use_localtime,
 
   if (timer == leaps[i].transition /* Exactly at the transition time.  */
       && (leaps[i].change > (i == 0 ? 0 : leaps[i - 1].change)))
-    {
-      *leap_hit = 1;
-      while (i > 0
-	     && leaps[i].transition == leaps[i - 1].transition + 1
-	     && leaps[i].change == leaps[i - 1].change + 1)
-	{
-	  ++*leap_hit;
-	  --i;
-	}
-    }
+    *leap_hit = true;
 }
 
 weak_alias (transitions, __libc_tzfile_freemem_ptr)
diff --git a/time/tzset.c b/time/tzset.c
index 9fb3a127f7..68e68f6cb9 100644
--- a/time/tzset.c
+++ b/time/tzset.c
@@ -557,8 +557,8 @@  weak_alias (__tzset, tzset)
 struct tm *
 __tz_convert (__time64_t timer, int use_localtime, struct tm *tp)
 {
-  long int leap_correction;
-  int leap_extra_secs;
+  int leap_correction;
+  bool leap_extra_sec;
 
   /* Update internal database according to current TZ setting.
      POSIX.1 8.3.7.2 says that localtime_r is not required to set tzname.
@@ -567,15 +567,15 @@  __tz_convert (__time64_t timer, int use_localtime, struct tm *tp)
 
   if (__use_tzfile)
     __tzfile_compute (timer, use_localtime, &leap_correction,
-		      &leap_extra_secs, tp);
+		      &leap_extra_sec, tp);
   else
     {
       if (! __offtime (timer, 0, tp))
 	tp = NULL;
       else
 	__tz_compute (timer, tp, use_localtime);
-      leap_correction = 0L;
-      leap_extra_secs = 0;
+      leap_correction = 0;
+      leap_extra_sec = false;
     }
 
   if (tp)
@@ -588,7 +588,16 @@  __tz_convert (__time64_t timer, int use_localtime, struct tm *tp)
 	}
 
       if (__offtime (timer, tp->tm_gmtoff - leap_correction, tp))
-        tp->tm_sec += leap_extra_secs;
+	{
+	  /* This assumes leap seconds can occur only when the local
+	     time offset from UTC is a multiple of 60 seconds,
+	     so the leap second occurs at HH:MM:60 local time.
+	     Historically this has always been true, as the last
+	     timezone to use some other UTC offset was Africa/Monrovia
+	     in January 1972, whereas the first leap second did not
+	     occur until almost six months later.  */
+	  tp->tm_sec += leap_extra_sec;
+	}
       else
 	tp = NULL;
     }
diff --git a/time/tzset.h b/time/tzset.h
index fb7984563e..d2d5853f0b 100644
--- a/time/tzset.h
+++ b/time/tzset.h
@@ -3,6 +3,7 @@ 
 
 #include <time.h>
 #include <libc-lock.h>
+#include <stdbool.h>
 
 /* Defined in tzset.c.  */
 extern char *__tzstring (const char *string) attribute_hidden;
@@ -11,7 +12,7 @@  extern int __use_tzfile attribute_hidden;
 
 extern void __tzfile_read (const char *file) attribute_hidden;
 extern void __tzfile_compute (__time64_t timer, int use_localtime,
-			      long int *leap_correct, int *leap_hit,
+			      int *leap_correct, bool *leap_hit,
 			      struct tm *tp) attribute_hidden;
 extern void __tzset_parse_tz (const char *tz) attribute_hidden;
 extern void __tz_compute (__time64_t timer, struct tm *tm, int use_localtime)