diff options
| -rw-r--r-- | ChangeLog | 5 | ||||
| -rw-r--r-- | time/tzfile.c | 99 |
2 files changed, 55 insertions, 49 deletions
diff --git a/ChangeLog b/ChangeLog index 82604298de..bc1b17ffa7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2019-02-03 Florian Weimer <fweimer@redhat.com> + + * time/tzfile.c (__tzfile_read): Use struct alloc_buffer and its + implicit overflow checks. + 2019-02-03 Aurelien Jarno <aurelien@aurel32.net> * stdlib/isomac.c: Include <unistd.h>. diff --git a/time/tzfile.c b/time/tzfile.c index a07e7c5037..e107b33a82 100644 --- a/time/tzfile.c +++ b/time/tzfile.c @@ -25,6 +25,7 @@ #include <unistd.h> #include <sys/stat.h> #include <stdint.h> +#include <alloc_buffer.h> #include <timezone/tzfile.h> @@ -105,12 +106,8 @@ __tzfile_read (const char *file, size_t extra, char **extrap) struct tzhead tzhead; size_t chars; size_t i; - size_t total_size; - size_t types_idx; - size_t leaps_idx; int was_using_tzfile = __use_tzfile; int trans_width = 4; - size_t tzspec_len; char *new = NULL; _Static_assert (sizeof (__time64_t) == 8, @@ -215,32 +212,9 @@ __tzfile_read (const char *file, size_t extra, char **extrap) goto read_again; } - if (__builtin_expect (num_transitions - > ((SIZE_MAX - (__alignof__ (struct ttinfo) - 1)) - / (sizeof (__time64_t) + 1)), 0)) - goto lose; - total_size = num_transitions * (sizeof (__time64_t) + 1); - total_size = ((total_size + __alignof__ (struct ttinfo) - 1) - & ~(__alignof__ (struct ttinfo) - 1)); - types_idx = total_size; - if (__builtin_expect (num_types - > (SIZE_MAX - total_size) / sizeof (struct ttinfo), 0)) - goto lose; - total_size += num_types * sizeof (struct ttinfo); - if (__glibc_unlikely (chars > SIZE_MAX - total_size)) - goto lose; - total_size += chars; - if (__builtin_expect (__alignof__ (struct leap) - 1 - > SIZE_MAX - total_size, 0)) - goto lose; - total_size = ((total_size + __alignof__ (struct leap) - 1) - & ~(__alignof__ (struct leap) - 1)); - leaps_idx = total_size; - if (__builtin_expect (num_leaps - > (SIZE_MAX - total_size) / sizeof (struct leap), 0)) - goto lose; - total_size += num_leaps * sizeof (struct leap); - tzspec_len = 0; + /* Compute the size of the POSIX time zone specification in the + file. */ + size_t tzspec_len; if (trans_width == 8) { off_t rem = st.st_size - __ftello (f); @@ -262,30 +236,56 @@ __tzfile_read (const char *file, size_t extra, char **extrap) if (__glibc_unlikely (tzspec_len == 0 || tzspec_len - 1 < num_isgmt)) goto lose; tzspec_len -= num_isgmt + 1; - if (__glibc_unlikely (tzspec_len == 0 - || SIZE_MAX - total_size < tzspec_len)) + if (tzspec_len == 0) goto lose; } - if (__glibc_unlikely (SIZE_MAX - total_size - tzspec_len < extra)) - goto lose; - - /* Allocate enough memory including the extra block requested by the - caller. */ - transitions = malloc (total_size + tzspec_len + extra); - if (transitions == NULL) - goto lose; - - type_idxs = (unsigned char *) transitions + (num_transitions - * sizeof (__time64_t)); - types = (struct ttinfo *) ((char *) transitions + types_idx); - zone_names = (char *) types + num_types * sizeof (struct ttinfo); - leaps = (struct leap *) ((char *) transitions + leaps_idx); + else + tzspec_len = 0; + + /* The file is parsed into a single heap allocation, comprising of + the following arrays: + + __time64_t transitions[num_transitions]; + struct ttinfo types[num_types]; + unsigned char type_idxs[num_types]; + char zone_names[chars]; + struct leap leaps[num_leaps]; + char extra_array[extra]; // Stored into *pextras if requested. + char tzspec[tzspec_len]; + + The piece-wise allocations from buf below verify that no + overflow/wraparound occurred in these computations. */ + struct alloc_buffer buf; + { + size_t total_size = num_transitions * (sizeof (__time64_t) + 1); + total_size = ((total_size + __alignof__ (struct ttinfo) - 1) + & ~(__alignof__ (struct ttinfo) - 1)); + total_size += num_types * sizeof (struct ttinfo) + chars; + total_size = ((total_size + __alignof__ (struct leap) - 1) + & ~(__alignof__ (struct leap) - 1)); + total_size += num_leaps * sizeof (struct leap) + tzspec_len + extra; + + transitions = malloc (total_size); + if (transitions == NULL) + goto lose; + buf = alloc_buffer_create (transitions, total_size); + } + + /* The address of the first allocation is already stored in the + pointer transitions. */ + (void) alloc_buffer_alloc_array (&buf, __time64_t, num_transitions); + type_idxs = alloc_buffer_alloc_array (&buf, unsigned char, num_transitions); + types = alloc_buffer_alloc_array (&buf, struct ttinfo, num_types); + zone_names = alloc_buffer_alloc_array (&buf, char, chars); + leaps = alloc_buffer_alloc_array (&buf, struct leap, num_leaps); + if (extra > 0) + *extrap = alloc_buffer_alloc_array (&buf, char, extra); if (trans_width == 8) - tzspec = (char *) leaps + num_leaps * sizeof (struct leap) + extra; + tzspec = alloc_buffer_alloc_array (&buf, char, tzspec_len); else tzspec = NULL; - if (extra > 0) - *extrap = (char *) &leaps[num_leaps]; + if (alloc_buffer_has_failed (&buf)) + goto lose; if (__builtin_expect (trans_width == 8, 1)) { @@ -390,6 +390,7 @@ __tzfile_read (const char *file, size_t extra, char **extrap) /* 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) |