[PATCH] tzset robustness [BZ#17715]

[Florian Weimer]

On 01/15/2015 09:25 PM, Joseph Myers wrote:
> On Wed, 14 Jan 2015, Florian Weimer wrote:
> 
>> @@ -434,6 +437,10 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
>>  	goto lose;
>>  
>>        tzspec_len = st.st_size - off - 1;
>> +      if (tzspec_len >= 256)
>> +	/* POSIX time zone specifiers are much shorter than 256
>> +	   characters.  */
>> +	goto lose;
>>        char *tzstr = alloca (tzspec_len);
>>        if (getc_unlocked (f) != '\n'
>>  	  || (__fread_unlocked (tzstr, 1, tzspec_len - 1, f)
> 
> Is it possible to have tzspec_len == 0 here?  The code doesn't look safe 
> if tzspec_len is 0 - it would pass (size_t)-1 to __fread_unlocked.
> 
> This code is for the case where time_t is 4-byte (and so size_t is 
> 4-byte).  tzspec_len is of type size_t.  st.st_size is of type off64_t (st 
> is struct stat64), so 8-byte.  If st.st_size < off + 2 we didn't get here, 
> but if st.st_size is off + 4GB + 1 it seems to me you could then get 
> tzspec_len being 0.  (This file is opened with fopen not fopen64 so the 
> open should fail if it's a large file at fopen time, but one might suppose 
> it only becomes a large file between the fopen call and the fstat64 call.)

I have verified that the problem you describe actually exists, with
another test case and by replacing fopen with fopen64.  Thanks.

This is probably a real bug on x32, which has a 64-bit off_t.

The test case also revealed that the code would read an arbitrary large
POSIX time zone specifier on 64-bit architectures.  I added another
check to catch that, too.

Re-tested on x86_64 and i386.

-- 
Florian Weimer / Red Hat Product Security
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Make-time-zone-file-parser-more-robust-BZ-17715.patch
Type: text/x-patch
Size: 12312 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20150120/e6514c8f/attachment.bin>