time: Use struct alloc_buffer in __tzfile_read

The computation of tzspec_len is moved in front of the total_size
computation, so that the allocation size computation and the
suballocations are next to each other.  Also add an assert that
tzspec_len is positive when it is actually used later.
This commit is contained in:
Florian Weimer 2019-02-03 22:13:51 +01:00
parent 11f382ee78
commit b8c7238167
2 changed files with 53 additions and 47 deletions

View file

@ -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>.

View file

@ -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;
else
tzspec_len = 0;
/* Allocate enough memory including the extra block requested by the
caller. */
transitions = malloc (total_size + tzspec_len + extra);
if (transitions == NULL)
goto lose;
/* The file is parsed into a single heap allocation, comprising of
the following arrays:
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);
__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)