This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix p_secstodate overflow handling (bug 22463)


On Tue, 21 Nov 2017, Paul Eggert wrote:

> Joseph Myers wrote:
> > If __gmtime_r returns NULL (because the year overflows the range of
> > int), this will dereference a null pointer.
> 
> 'clock' must be in the range 0 .. 2**32-1, so the year cannot overflow the
> range of int. This range is enforced by all callers of p_secstodate, and is
> required by Internet RFC 4034 section 3.2.

p_secstodate is a public interface, exported from libresolv.so as a 
non-compat symbol (under the name __p_secstodate, with a #define of 
p_secstodate in the public resolv.h header).  Thus, arguments outside that 
range need to be handled in some safe way, absent a specification saying 
they are undefined, and as far as I can see they can produce buffer 
overruns at present.  RFC 4034 does not provide a specification of the 
function p_secstodate.

> That's a false alarm since the buffer cannot overrun. In Gnulib I work around
> such alarms by using the 'assume' macro defined by verify.h. Or one can use
> __builtin_unreachable () directly, as is done in the attached patch. (The
> 'assume' macro is more readable, but is one more thing to add....)

I tried using __builtin_unreachable.  One version of the patch had

+       if (time->tm_mon < 0 || time->tm_mon > 11
+           || time->tm_mday < 1 || time->tm_mday > 31
+           || time->tm_hour < 0 || time->tm_hour > 23
+           || time->tm_min < 0 || time->tm_min > 59
+           || time->tm_sec < 0 || time->tm_sec > 60)
+               __builtin_unreachable ();

but it didn't help to avoid the errors (I don't know why VRP didn't 
operate effectively in that case).

> code mishandles timestamps after 2038. Proposed patches attached. Although I
> haven't tested them, the first patch just adds a Gnulib include file that is
> well-tested so it should be OK.

It adds a gnulib include file that there have been multiple objections to 
including in glibc in the past.

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]