]> Savannah Git Hosting - gnulib.git/commitdiff
time_rz: fix heap buffer overflow vulnerability
authorPádraig Brady <P@draigBrady.com>
Mon, 24 Apr 2017 08:43:36 +0000 (01:43 -0700)
committerPádraig Brady <P@draigBrady.com>
Thu, 27 Apr 2017 03:32:12 +0000 (20:32 -0700)
This issue has been assigned CVE-2017-7476 and was
detected with American Fuzzy Lop 2.41b run on the
coreutils date(1) program with ASAN enabled.

  ERROR: AddressSanitizer: heap-buffer-overflow on address 0x...
  WRITE of size 8 at 0x60d00000cff8 thread T0
  #1 0x443020 in extend_abbrs lib/time_rz.c:88
  #2 0x443356 in save_abbr lib/time_rz.c:155
  #3 0x44393f in localtime_rz lib/time_rz.c:290
  #4 0x41e4fe in parse_datetime2 lib/parse-datetime.y:1798

A minimized reproducer is the following 120 byte TZ value,
which goes beyond the value of ABBR_SIZE_MIN (119) on x86_64.
Extend the aa...b portion to overwrite more of the heap.

  date -d $(printf 'TZ="aaa%020daaaaaab%089d"')

localtime_rz and mktime_z were affected since commit 4bc76593.
parse_datetime was affected since commit 4e6e16b3f.

* lib/time_rz.c (save_abbr): Rearrange the calculation determining
whether there is enough buffer space available.  The rearrangement
ensures we're only dealing with positive numbers, thus avoiding
the problematic promotion of signed to unsigned causing an invalid
comparison when zone_copy is more than ABBR_SIZE_MIN bytes beyond
the start of the buffer.
* tests/test-parse-datetime.c (main): Add a test case written by
Paul Eggert, which overwrites enough of the heap so that
standard glibc will fail with "free(): invalid pointer"
without the patch applied.
Reported and analyzed at https://bugzilla.redhat.com/1444774

ChangeLog
lib/time_rz.c
tests/test-parse-datetime.c

index fd5ec8936e892d0fe76a0941658be77bf8f1fea6..314f7d97a9ff3bb1a2c90ec5a38598bd2d7dfbd5 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2017-04-26  Pádraig Brady  <P@draigBrady.com>
+       and Paul Eggert  <eggert@cs.ucla.edu>
+
+       time_rz: fix heap buffer overflow vulnerability
+       Reported and analyzed at https://bugzilla.redhat.com/CVE-2017-7476
+       * lib/time_rz.c (save_abbr): Rearrange the calculation determining
+       whether there is enough buffer space available, thus avoiding
+       the problematic promotion of signed to unsigned causing an invalid
+       comparison when zone_copy is more than ABBR_SIZE_MIN bytes beyond
+       the start of the buffer.
+       * tests/test-parse-datetime.c (main): Add a test case written by
+       Paul Eggert, which overwrites enough of the heap so that
+       standard glibc will fail with "free(): invalid pointer"
+       without the patch applied.
+
 2017-04-26  Paul Eggert  <eggert@cs.ucla.edu>
 
        xalloc: add missing integer overflow check
index 1b878702c01d34c9c54d4effe71f7dc42be3f7a0..82f3f3f19dc5beae04ac1c567daaebaa666bc6c8 100644 (file)
@@ -27,6 +27,7 @@
 #include <time.h>
 
 #include <errno.h>
+#include <limits.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdlib.h>
 #include "flexmember.h"
 #include "time-internal.h"
 
+#ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+#endif
+
 #if !HAVE_TZSET
 static void tzset (void) { }
 #endif
@@ -43,7 +48,7 @@ static void tzset (void) { }
    the largest "small" request for the GNU C library malloc.  */
 enum { DEFAULT_MXFAST = 64 * sizeof (size_t) / 4 };
 
-/* Minimum size of the ABBRS member of struct abbr.  ABBRS is larger
+/* Minimum size of the ABBRS member of struct tm_zone.  ABBRS is larger
    only in the unlikely case where an abbreviation longer than this is
    used.  */
 enum { ABBR_SIZE_MIN = DEFAULT_MXFAST - offsetof (struct tm_zone, abbrs) };
@@ -150,7 +155,13 @@ save_abbr (timezone_t tz, struct tm *tm)
           if (! (*zone_copy || (zone_copy == tz->abbrs && tz->tz_is_set)))
             {
               size_t zone_size = strlen (zone) + 1;
-              if (zone_size < tz->abbrs + ABBR_SIZE_MIN - zone_copy)
+              size_t zone_used = zone_copy - tz->abbrs;
+              if (SIZE_MAX - zone_used < zone_size)
+                {
+                  errno = ENOMEM;
+                  return false;
+                }
+              if (zone_used + zone_size < ABBR_SIZE_MIN)
                 extend_abbrs (zone_copy, zone, zone_size);
               else
                 {
index b42a51c4f56b7ec54577108205555d94231e04e1..b6fe457cc13da414911285ff3505cca13249ff38 100644 (file)
@@ -432,5 +432,21 @@ main (int argc _GL_UNUSED, char **argv)
   ASSERT (   parse_datetime (&result, "TZ=\"\\\\\"", &now));
   ASSERT (   parse_datetime (&result, "TZ=\"\\\"\"", &now));
 
+  /* Outlandishly-long time zone abbreviations should not cause problems.  */
+  {
+    static char const bufprefix[] = "TZ=\"";
+    enum { tzname_len = 2000 };
+    static char const bufsuffix[] = "0\" 1970-01-01 01:02:03.123456789";
+    enum { bufsize = sizeof bufprefix - 1 + tzname_len + sizeof bufsuffix };
+    char buf[bufsize];
+    memcpy (buf, bufprefix, sizeof bufprefix - 1);
+    memset (buf + sizeof bufprefix - 1, 'X', tzname_len);
+    strcpy (buf + bufsize - sizeof bufsuffix, bufsuffix);
+    ASSERT (parse_datetime (&result, buf, &now));
+    LOG (buf, now, result);
+    ASSERT (result.tv_sec == 1 * 60 * 60 + 2 * 60 + 3
+            && result.tv_nsec == 123456789);
+  }
+
   return 0;
 }