Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

>>    If the input check would be "approximately January 2, 1001 through
>>    approximately December 30, 9998 AD" (to avoid time zone and leap
>>    second issues), then it would be somewhat tricky to write the code
>>    portably, since the numbers involved will exceed 2**32.  And this
>>    would reject a few valid inputs.
>
> I think that is best.  It should be straightforward, too, if you first
> use localtime to extract the year.

OK, thanks, I've done that.  The revised patch is enclosed below.  The
check is equivalent to "-999 <= year <= 9999", and is done after
invoking localtime.

> I think it is possible, even trivial, to fix ctime.  The largest year
> value that can be represented in a 64-bit time-value is around 1
> trillion, and that will fit in 12 digits.  Isn't this trivial to fix?

No, because ctime is implemented in terms of localtime, localtime is
defined to use an 'int' to represent the year, and 'int' is only 32
bits on most hosts.  Fixing this would require some nontrivial
interface twiddling (presumably with a localtime variant).

> Fixing asctime as well, on systems where int is 64 bits, requires checking
> for year numbers that are too big.  But it is not hard to do.

Yes.  If the year fits into an 'int', then glibc ctime already does
the right thing, and outputs the full year without buffer overflow.
(Other C libraries crash, which causes the Emacs core dump.)

The problem is when the year does not fit into an 'int', as mentioned
above.  In this case glibc ctime reliably returns NULL, which is
certainly better than overrunning a buffer, but is not ideal.


Anyway, here's the revised fix for Emacs.

This fix attempts to avoid duplicated code, which I think was part of
the underlying objection to the earlier fix.  I am still reluctant to
use the system asctime and/or asctime_r to do the real work, since I
know of bugs on real-world systems with time values outside the
traditional range; so this patch still does its own implementation
instead of invoking asctime or asctime_r.  This substitute
implementation is only 15 lines of simple code, so I don't think it's
a big deal; but if you really want the code to invoke the system
asctime/asctime_r despite their bugs I can submit an alternate patch
that does that.


2006-03-19  Paul Eggert  <eggert@cs.ucla.edu>

        Do not use ctime, since it has undefined behavior with
        out-of-range time stamps.  This fixes a bug where
        (current-time-string '(2814749767106 0)) would make Emacs dump
        core on 64-bit Solaris 8; the fix is to remove all uses of ctime
        from the Emacs source code.  Please see
        <http://www.opengroup.org/austin/mailarchives/ag/msg09294.html>
        for more details about this portability problem.

        * src/ctime-r.h: New file.
        * lib-src/Makefile.in ($b2m${EXEEXT}, fakemail${EXEEXT}): Depend on it.
        * src/Makefile.in (editfns.o): Likewise.
        * lib-src/b2m.c: Include it.  Include <limits.h>, for CHAR_BIT.
        (main): Use emacs_ctime_r instead of ctime, to avoid buffer overflows
        with outlandish time stamps.  Report an error if an outlandish time
        stamp is discovered.
        * lib-src/fakemail.c: Likewise.
        * src/editfns.c: Include <limits.h>, for CHAR_BIT.
        Include ctime-r.h.
        (TM_YEAR_BASE): Move up, so changes can use it.
        (Fdecode_time, Fencode_time):
        Use TM_YEAR_BASE instead of hardwiring 1900.
        (Fdecode_time): Cast tm_year to EMACS_INT, to avoid overflow when
        int is narrow than EMACS_INT.
        (Fcurrent_time_string): Use emacs_ctime_r rather than ctime.
        As with Fformat_time_string, report an invalid time specification
        if the argument is invalid, and report an out-of-range time stamp
        if that problem occurs.  This is better than overrunning the buffer,
        and it preserves the historic behavior of always returning a
        fixed-size string.
        * lib-src/ntlib.c (sys_ctime): Remove, since Emacs never calls
        ctime any more.
        * lib-src/ntlib.h (ctime): Likewise.
        * src/w32.c (sys_ctime): Likewise.
        * src/s/ms-w32.h (ctime): Likewise.

*** /dev/null   Sat Sep 24 22:00:15 2005
--- src/ctime-r.h       Sun Mar 19 15:02:34 2006
***************
*** 0 ****
--- 1,56 ----
+ /* A safer replacement for ctime_r.
+ 
+    Copyright (C) 2006 Free Software Foundation, Inc.
+ 
+ This file is part of GNU Emacs.
+ 
+ GNU Emacs is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2, or (at your option)
+ any later version.
+ 
+ GNU Emacs is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU General Public License for more details.
+ 
+ You should have received a copy of the GNU General Public License
+ along with GNU Emacs; see the file COPYING.  If not, write to
+ the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ Boston, MA 02110-1301, USA.  */
+ 
+ #define TM_YEAR_BASE 1900
+ 
+ #define CTIME_R_BUFSIZE (sizeof "Sun Sep 16 01:03:52 1973")
+ 
+ /* A safer replacement for ctime_r.  This version reliably returns
+    NULL when given out-of-range values, instead of having undefined
+    behavior that possibly overruns buffers.  */
+ 
+ static char *
+ emacs_ctime_r (time_t const *t, char *buf)
+ {
+   struct tm const *tm = localtime (t);
+ 
+   if (tm
+       && -999 - TM_YEAR_BASE <= tm->tm_year
+       && tm->tm_year <= 9999 - TM_YEAR_BASE)
+     {
+       static char const wday[][4] =
+       {
+         "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+       };
+       static char const mon[][4] =
+       {
+         "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+         "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+       };
+       sprintf (buf, "%s %s %2d %02d:%02d:%02d %04d\n",
+              wday[tm->tm_wday], mon[tm->tm_mon], tm->tm_mday,
+              tm->tm_hour, tm->tm_min, tm->tm_sec,
+              TM_YEAR_BASE + tm->tm_year);
+       return buf;
+     }
+ 
+   return NULL;
+ }
Index: lib-src/Makefile.in
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/Makefile.in,v
retrieving revision 1.148
diff -p -c -r1.148 Makefile.in
*** lib-src/Makefile.in 3 Mar 2006 12:02:31 -0000       1.148
--- lib-src/Makefile.in 19 Mar 2006 23:30:59 -0000
*************** digest-doc${EXEEXT}: ${srcdir}/digest-do
*** 430,436 ****
  sorted-doc${EXEEXT}: ${srcdir}/sorted-doc.c
        $(CC) ${ALL_CFLAGS} ${srcdir}/sorted-doc.c $(LOADLIBES) -o sorted-doc
  
! b2m${EXEEXT}: ${srcdir}/b2m.c ../src/config.h $(GETOPTDEPS)
        $(CC) ${ALL_CFLAGS} ${srcdir}/b2m.c  -DVERSION="\"${version}\"" \
           $(GETOPTOBJS) $(LOADLIBES) -o b2m
  
--- 430,437 ----
  sorted-doc${EXEEXT}: ${srcdir}/sorted-doc.c
        $(CC) ${ALL_CFLAGS} ${srcdir}/sorted-doc.c $(LOADLIBES) -o sorted-doc
  
! b2m${EXEEXT}: ${srcdir}/b2m.c ${srcdir}/../src/ctime-r.h \
!   ../src/config.h $(GETOPTDEPS)
        $(CC) ${ALL_CFLAGS} ${srcdir}/b2m.c  -DVERSION="\"${version}\"" \
           $(GETOPTOBJS) $(LOADLIBES) -o b2m
  
*************** pop.o: ${srcdir}/pop.c  ../src/config.h
*** 446,452 ****
  cvtmail${EXEEXT}: ${srcdir}/cvtmail.c
        $(CC) ${ALL_CFLAGS} ${srcdir}/cvtmail.c $(LOADLIBES) -o cvtmail
  
! fakemail${EXEEXT}: ${srcdir}/fakemail.c ../src/config.h
        $(CC) ${ALL_CFLAGS} ${srcdir}/fakemail.c $(LOADLIBES) -o fakemail
  
  yow${EXEEXT}: ${srcdir}/yow.c ../src/epaths.h
--- 447,454 ----
  cvtmail${EXEEXT}: ${srcdir}/cvtmail.c
        $(CC) ${ALL_CFLAGS} ${srcdir}/cvtmail.c $(LOADLIBES) -o cvtmail
  
! fakemail${EXEEXT}: ${srcdir}/fakemail.c ${srcdir}/../src/ctime-r.h \
!   ../src/config.h
        $(CC) ${ALL_CFLAGS} ${srcdir}/fakemail.c $(LOADLIBES) -o fakemail
  
  yow${EXEEXT}: ${srcdir}/yow.c ../src/epaths.h
Index: lib-src/b2m.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/b2m.c,v
retrieving revision 1.30
diff -p -c -r1.30 b2m.c
*** lib-src/b2m.c       7 May 2004 15:26:21 -0000       1.30
--- lib-src/b2m.c       19 Mar 2006 23:30:59 -0000
***************
*** 26,31 ****
--- 26,32 ----
  #undef static
  #endif
  
+ #include <limits.h>
  #include <stdio.h>
  #include <time.h>
  #include <sys/types.h>
***************
*** 34,39 ****
--- 35,42 ----
  #include <fcntl.h>
  #endif
  
+ #include "../src/ctime-r.h"
+ 
  #undef TRUE
  #define TRUE  1
  #undef FALSE
*************** main (argc, argv)
*** 89,94 ****
--- 92,98 ----
    time_t ltoday;
    char *labels, *p, *today;
    struct linebuffer data;
+   char buf[CTIME_R_BUFSIZE];
  
  #ifdef MSDOS
    _fmode = O_BINARY;          /* all of files are treated as binary files */
*************** main (argc, argv)
*** 131,137 ****
  
    labels_saved = printing = header = FALSE;
    ltoday = time (0);
!   today = ctime (&ltoday);
    data.size = 200;
    data.buffer = xnew (200, char);
  
--- 135,143 ----
  
    labels_saved = printing = header = FALSE;
    ltoday = time (0);
!   today = emacs_ctime_r (&ltoday, buf);
!   if (! today)
!     fatal ("current time is out of range");
    data.size = 200;
    data.buffer = xnew (200, char);
  
Index: lib-src/fakemail.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/fakemail.c,v
retrieving revision 1.35
diff -p -c -r1.35 fakemail.c
*** lib-src/fakemail.c  6 Feb 2006 11:28:28 -0000       1.35
--- lib-src/fakemail.c  19 Mar 2006 23:30:59 -0000
*************** main ()
*** 53,58 ****
--- 53,59 ----
  #include "ntlib.h"
  #endif
  
+ #include <limits.h>
  #include <stdio.h>
  #include <string.h>
  #include <ctype.h>
*************** main ()
*** 63,68 ****
--- 64,71 ----
  #ifdef HAVE_UNISTD_H
  #include <unistd.h>
  #endif
+ 
+ #include "../src/ctime-r.h"
  
  /* Type definitions */
  
*************** line_list
*** 353,359 ****
  make_file_preface ()
  {
    char *the_string, *temp;
!   long idiotic_interface;
    long prefix_length;
    long user_length;
    long date_length;
--- 356,363 ----
  make_file_preface ()
  {
    char *the_string, *temp;
!   char buf[CTIME_R_BUFSIZE];
!   time_t idiotic_interface;
    long prefix_length;
    long user_length;
    long date_length;
*************** make_file_preface ()
*** 361,367 ****
  
    prefix_length = strlen (FROM_PREFIX);
    time (&idiotic_interface);
!   the_date = ctime (&idiotic_interface);
    /* the_date has an unwanted newline at the end */
    date_length = strlen (the_date) - 1;
    the_date[date_length] = '\0';
--- 365,373 ----
  
    prefix_length = strlen (FROM_PREFIX);
    time (&idiotic_interface);
!   the_date = emacs_ctime_r (&idiotic_interface, buf);
!   if (! the_date)
!     fatal ("current time is out of range", 0);
    /* the_date has an unwanted newline at the end */
    date_length = strlen (the_date) - 1;
    the_date[date_length] = '\0';
Index: lib-src/ntlib.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/ntlib.c,v
retrieving revision 1.13
diff -p -c -r1.13 ntlib.c
*** lib-src/ntlib.c     6 Feb 2006 11:28:28 -0000       1.13
--- lib-src/ntlib.c     19 Mar 2006 23:31:00 -0000
*************** fchown (int fd, int uid, int gid)
*** 188,203 ****
    return 0;
  }
  
- /* Place a wrapper around the MSVC version of ctime.  It returns NULL
-    on network directories, so we handle that case here.
-    (Ulrich Leodolter, 1/11/95).  */
- char *
- sys_ctime (const time_t *t)
- {
-   char *str = (char *) ctime (t);
-   return (str ? str : "Sun Jan 01 00:00:00 1970");
- }
- 
  FILE *
  sys_fopen(const char * path, const char * mode)
  {
--- 188,193 ----
Index: lib-src/ntlib.h
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/ntlib.h,v
retrieving revision 1.11
diff -p -c -r1.11 ntlib.h
*** lib-src/ntlib.h     6 Feb 2006 11:28:28 -0000       1.11
--- lib-src/ntlib.h     19 Mar 2006 23:31:00 -0000
*************** int fchown (int fd, int uid, int gid);
*** 61,67 ****
  #define close   _close
  #undef creat
  #define creat   _creat
- #undef ctime
  #undef dup
  #define dup     _dup
  #undef dup2
--- 61,66 ----
Index: mac/inc/s-mac.h
===================================================================
RCS file: /cvsroot/emacs/emacs/mac/inc/s-mac.h,v
retrieving revision 1.10
diff -p -c -r1.10 s-mac.h
*** mac/inc/s-mac.h     6 Feb 2006 12:03:35 -0000       1.10
--- mac/inc/s-mac.h     19 Mar 2006 23:31:00 -0000
*************** Boston, MA 02110-1301, USA.  */
*** 293,299 ****
  
  #define gmtime sys_gmtime
  #define localtime sys_localtime
- #define ctime sys_ctime
  #define time sys_time
  
  #define index strchr
--- 293,298 ----
Index: src/Makefile.in
===================================================================
RCS file: /cvsroot/emacs/emacs/src/Makefile.in,v
retrieving revision 1.325
diff -p -c -r1.325 Makefile.in
*** src/Makefile.in     20 Feb 2006 22:16:00 -0000      1.325
--- src/Makefile.in     19 Mar 2006 23:31:00 -0000
*************** doc.o: doc.c $(config_h) epaths.h buffer
*** 1117,1124 ****
  doprnt.o: doprnt.c charset.h $(config_h)
  dosfns.o: buffer.h termchar.h termhooks.h frame.h blockinput.h window.h \
     msdos.h dosfns.h dispextern.h charset.h coding.h $(config_h)
! editfns.o: editfns.c window.h buffer.h systime.h $(INTERVAL_SRC) charset.h \
!    coding.h dispextern.h frame.h $(config_h)
  emacs.o: emacs.c commands.h systty.h syssignal.h blockinput.h process.h \
     termhooks.h buffer.h atimer.h systime.h $(INTERVAL_SRC) $(config_h) \
     window.h dispextern.h keyboard.h keymap.h
--- 1117,1124 ----
  doprnt.o: doprnt.c charset.h $(config_h)
  dosfns.o: buffer.h termchar.h termhooks.h frame.h blockinput.h window.h \
     msdos.h dosfns.h dispextern.h charset.h coding.h $(config_h)
! editfns.o: editfns.c ctime-r.h window.h buffer.h systime.h $(INTERVAL_SRC) \
!    charset.h coding.h dispextern.h frame.h $(config_h)
  emacs.o: emacs.c commands.h systty.h syssignal.h blockinput.h process.h \
     termhooks.h buffer.h atimer.h systime.h $(INTERVAL_SRC) $(config_h) \
     window.h dispextern.h keyboard.h keymap.h
Index: src/editfns.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/editfns.c,v
retrieving revision 1.409
diff -p -c -r1.409 editfns.c
*** src/editfns.c       7 Feb 2006 09:08:53 -0000       1.409
--- src/editfns.c       19 Mar 2006 23:31:00 -0000
*************** Boston, MA 02110-1301, USA.  */
*** 49,54 ****
--- 49,55 ----
  #endif
  
  #include <ctype.h>
+ #include <limits.h>
  
  #include "intervals.h"
  #include "buffer.h"
*************** Boston, MA 02110-1301, USA.  */
*** 64,69 ****
--- 65,72 ----
  #define MAX_10_EXP    310
  #endif
  
+ #include "ctime-r.h"
+ 
  #ifndef NULL
  #define NULL 0
  #endif
*************** Boston, MA 02110-1301, USA.  */
*** 72,77 ****
--- 75,82 ----
  extern char **environ;
  #endif
  
+ #define TM_YEAR_BASE 1900
+ 
  extern size_t emacs_strftimeu P_ ((char *, size_t, const char *,
                                   const struct tm *, int));
  static int tm_diff P_ ((struct tm *, struct tm *));
*************** DOW and ZONE.)  */)
*** 1722,1728 ****
    XSETFASTINT (list_args[2], decoded_time->tm_hour);
    XSETFASTINT (list_args[3], decoded_time->tm_mday);
    XSETFASTINT (list_args[4], decoded_time->tm_mon + 1);
!   XSETINT (list_args[5], decoded_time->tm_year + 1900);
    XSETFASTINT (list_args[6], decoded_time->tm_wday);
    list_args[7] = (decoded_time->tm_isdst)? Qt : Qnil;
  
--- 1727,1733 ----
    XSETFASTINT (list_args[2], decoded_time->tm_hour);
    XSETFASTINT (list_args[3], decoded_time->tm_mday);
    XSETFASTINT (list_args[4], decoded_time->tm_mon + 1);
!   XSETINT (list_args[5], TM_YEAR_BASE + (EMACS_INT) decoded_time->tm_year);
    XSETFASTINT (list_args[6], decoded_time->tm_wday);
    list_args[7] = (decoded_time->tm_isdst)? Qt : Qnil;
  
*************** usage: (encode-time SECOND MINUTE HOUR D
*** 1778,1784 ****
    tm.tm_hour = XINT (args[2]);
    tm.tm_mday = XINT (args[3]);
    tm.tm_mon = XINT (args[4]) - 1;
!   tm.tm_year = XINT (args[5]) - 1900;
    tm.tm_isdst = -1;
  
    if (CONSP (zone))
--- 1783,1789 ----
    tm.tm_hour = XINT (args[2]);
    tm.tm_mday = XINT (args[3]);
    tm.tm_mon = XINT (args[4]) - 1;
!   tm.tm_year = XINT (args[5]) - TM_YEAR_BASE;
    tm.tm_isdst = -1;
  
    if (CONSP (zone))
*************** but this is considered obsolete.  */)
*** 1843,1863 ****
       Lisp_Object specified_time;
  {
    time_t value;
!   char buf[30];
!   register char *tem;
  
    if (! lisp_time_argument (specified_time, &value, NULL))
!     value = -1;
!   tem = (char *) ctime (&value);
! 
!   strncpy (buf, tem, 24);
    buf[24] = 0;
  
    return build_string (buf);
  }
  
- #define TM_YEAR_BASE 1900
- 
  /* Yield A - B, measured in seconds.
     This function is copied from the GNU C Library.  */
  static int
--- 1848,1864 ----
       Lisp_Object specified_time;
  {
    time_t value;
!   char buf[CTIME_R_BUFSIZE];
  
    if (! lisp_time_argument (specified_time, &value, NULL))
!     error ("Invalid time specification");
!   if (! emacs_ctime_r (&value, buf))
!     error ("Specified time is not representable");
    buf[24] = 0;
  
    return build_string (buf);
  }
  
  /* Yield A - B, measured in seconds.
     This function is copied from the GNU C Library.  */
  static int
Index: src/mac.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/mac.c,v
retrieving revision 1.55
diff -p -c -r1.55 mac.c
*** src/mac.c   12 Mar 2006 08:19:42 -0000      1.55
--- src/mac.c   19 Mar 2006 23:31:00 -0000
*************** sys_localtime (const time_t *timer)
*** 2500,2520 ****
  }
  
  
- #undef ctime
- extern char *ctime (const time_t *);
- char *
- sys_ctime (const time_t *timer)
- {
- #if __MSL__ >= 0x6000
-   time_t unix_time = *timer;
- #else
-   time_t unix_time = *timer + CW_OR_MPW_UNIX_EPOCH_DIFF;
- #endif
- 
-   return ctime (&unix_time);
- }
- 
- 
  #undef time
  extern time_t time (time_t *);
  time_t
--- 2500,2505 ----
Index: src/w32.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/w32.c,v
retrieving revision 1.100
diff -p -c -r1.100 w32.c
*** src/w32.c   27 Feb 2006 02:07:37 -0000      1.100
--- src/w32.c   19 Mar 2006 23:31:00 -0000
*************** Boston, MA 02110-1301, USA.
*** 43,49 ****
  #undef chdir
  #undef chmod
  #undef creat
- #undef ctime
  #undef fopen
  #undef link
  #undef mkdir
--- 43,48 ----
*************** gettimeofday (struct timeval *tv, struct
*** 1325,1340 ****
  /* IO support and wrapper functions for W32 API. */
  /* ------------------------------------------------------------------------- 
*/
  
- /* Place a wrapper around the MSVC version of ctime.  It returns NULL
-    on network directories, so we handle that case here.
-    (Ulrich Leodolter, 1/11/95).  */
- char *
- sys_ctime (const time_t *t)
- {
-   char *str = (char *) ctime (t);
-   return (str ? str : "Sun Jan 01 00:00:00 1970");
- }
- 
  /* Emulate sleep...we could have done this with a define, but that
     would necessitate including windows.h in the files that used it.
     This is much easier.  */
--- 1324,1329 ----
Index: src/s/ms-w32.h
===================================================================
RCS file: /cvsroot/emacs/emacs/src/s/ms-w32.h,v
retrieving revision 1.37
diff -p -c -r1.37 ms-w32.h
*** src/s/ms-w32.h      6 Feb 2006 15:23:23 -0000       1.37
--- src/s/ms-w32.h      19 Mar 2006 23:31:00 -0000
*************** Boston, MA 02110-1301, USA.  */
*** 317,323 ****
  #define close   sys_close
  #undef creat
  #define creat   sys_creat
- #define ctime sys_ctime
  #undef dup
  #define dup     sys_dup
  #undef dup2
--- 317,322 ----