bpo-31339: Rewrite time.asctime() and time.ctime() (#3293)

vstinner · web-flow · commit eeadf5fc2311 · 2017-09-06T01:35:39.000+02:00
* bpo-31339: Rewrite time.asctime() and time.ctime() Backport and adapt the _asctime() function from the master branch to not depend on the implementation of asctime() and ctime() from the external C library. This change fixes a bug when Python is run using the musl C library. * bound checks for time.asctime() * bound checks for time.strftime()
diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py
@@ -2,6 +2,12 @@
 import time
 import unittest
 import sys
+import sysconfig
+
+
+# Max year is only limited by the size of C int.
+SIZEOF_INT = sysconfig.get_config_var('SIZEOF_INT') or 4
+TIME_MAXYEAR = (1 << 8 * SIZEOF_INT - 1) - 1
 
 
 class TimeTestCase(unittest.TestCase):
@@ -45,6 +51,66 @@ def test_strftime(self):
             with self.assertRaises(ValueError):
                 time.strftime('%f')
 
+    def _bounds_checking(self, func):
+        # Make sure that strftime() checks the bounds of the various parts
+        # of the time tuple (0 is valid for *all* values).
+
+        # The year field is tested by other test cases above
+
+        # Check month [1, 12] + zero support
+        func((1900, 0, 1, 0, 0, 0, 0, 1, -1))
+        func((1900, 12, 1, 0, 0, 0, 0, 1, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, -1, 1, 0, 0, 0, 0, 1, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, 13, 1, 0, 0, 0, 0, 1, -1))
+        # Check day of month [1, 31] + zero support
+        func((1900, 1, 0, 0, 0, 0, 0, 1, -1))
+        func((1900, 1, 31, 0, 0, 0, 0, 1, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, 1, -1, 0, 0, 0, 0, 1, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, 1, 32, 0, 0, 0, 0, 1, -1))
+        # Check hour [0, 23]
+        func((1900, 1, 1, 23, 0, 0, 0, 1, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, 1, 1, -1, 0, 0, 0, 1, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, 1, 1, 24, 0, 0, 0, 1, -1))
+        # Check minute [0, 59]
+        func((1900, 1, 1, 0, 59, 0, 0, 1, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, 1, 1, 0, -1, 0, 0, 1, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, 1, 1, 0, 60, 0, 0, 1, -1))
+        # Check second [0, 61]
+        self.assertRaises(ValueError, func,
+                            (1900, 1, 1, 0, 0, -1, 0, 1, -1))
+        # C99 only requires allowing for one leap second, but Python's docs say
+        # allow two leap seconds (0..61)
+        func((1900, 1, 1, 0, 0, 60, 0, 1, -1))
+        func((1900, 1, 1, 0, 0, 61, 0, 1, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, 1, 1, 0, 0, 62, 0, 1, -1))
+        # No check for upper-bound day of week;
+        #  value forced into range by a ``% 7`` calculation.
+        # Start check at -2 since gettmarg() increments value before taking
+        #  modulo.
+        self.assertEqual(func((1900, 1, 1, 0, 0, 0, -1, 1, -1)),
+                         func((1900, 1, 1, 0, 0, 0, +6, 1, -1)))
+        self.assertRaises(ValueError, func,
+                            (1900, 1, 1, 0, 0, 0, -2, 1, -1))
+        # Check day of the year [1, 366] + zero support
+        func((1900, 1, 1, 0, 0, 0, 0, 0, -1))
+        func((1900, 1, 1, 0, 0, 0, 0, 366, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, 1, 1, 0, 0, 0, 0, -1, -1))
+        self.assertRaises(ValueError, func,
+                            (1900, 1, 1, 0, 0, 0, 0, 367, -1))
+
+    def test_strftime_bounding_check(self):
+        self._bounds_checking(lambda tup: time.strftime('', tup))
+
     def test_strftime_bounds_checking(self):
         # Make sure that strftime() checks the bounds of the various parts
         #of the time tuple (0 is valid for *all* values).
@@ -123,15 +189,15 @@ def test_asctime(self):
         time.asctime(time.gmtime(self.t))
         self.assertRaises(TypeError, time.asctime, 0)
         self.assertRaises(TypeError, time.asctime, ())
-        # XXX: Posix compiant asctime should refuse to convert
-        # year > 9999, but Linux implementation does not.
-        # self.assertRaises(ValueError, time.asctime,
-        #                  (12345, 1, 0, 0, 0, 0, 0, 0, 0))
-        # XXX: For now, just make sure we don't have a crash:
-        try:
-            time.asctime((12345, 1, 1, 0, 0, 0, 0, 1, 0))
-        except ValueError:
-            pass
+
+        # Max year is only limited by the size of C int.
+        asc = time.asctime((TIME_MAXYEAR, 6, 1) + (0,) * 6)
+        self.assertEqual(asc[-len(str(TIME_MAXYEAR)):], str(TIME_MAXYEAR))
+        self.assertRaises(OverflowError, time.asctime,
+                          (TIME_MAXYEAR + 1,) + (0,) * 8)
+        self.assertRaises(TypeError, time.asctime, 0)
+        self.assertRaises(TypeError, time.asctime, ())
+        self.assertRaises(TypeError, time.asctime, (0,) * 10)
 
     @unittest.skipIf(not hasattr(time, "tzset"),
         "time module has no attribute tzset")
diff --git a/Misc/NEWS.d/next/Security/2017-09-04-21-24-51.bpo-31339.YSczZN.rst b/Misc/NEWS.d/next/Security/2017-09-04-21-24-51.bpo-31339.YSczZN.rst
@@ -0,0 +1,4 @@
+Rewrite time.asctime() and time.ctime(). Backport and adapt the _asctime()
+function from the master branch to not depend on the implementation of
+asctime() and ctime() from the external C library. This change fixes a bug
+when Python is run using the musl C library.
diff --git a/Modules/timemodule.c b/Modules/timemodule.c
@@ -388,6 +388,76 @@ gettmarg(PyObject *args, struct tm *p)
     return 1;
 }
 
+/* Check values of the struct tm fields before it is passed to strftime() and
+ * asctime().  Return 1 if all values are valid, otherwise set an exception
+ * and returns 0.
+ */
+static int
+checktm(struct tm* buf)
+{
+    /* Checks added to make sure strftime() and asctime() does not crash Python by
+       indexing blindly into some array for a textual representation
+       by some bad index (fixes bug #897625 and #6608).
+
+       Also support values of zero from Python code for arguments in which
+       that is out of range by forcing that value to the lowest value that
+       is valid (fixed bug #1520914).
+
+       Valid ranges based on what is allowed in struct tm:
+
+       - tm_year: [0, max(int)] (1)
+       - tm_mon: [0, 11] (2)
+       - tm_mday: [1, 31]
+       - tm_hour: [0, 23]
+       - tm_min: [0, 59]
+       - tm_sec: [0, 60]
+       - tm_wday: [0, 6] (1)
+       - tm_yday: [0, 365] (2)
+       - tm_isdst: [-max(int), max(int)]
+
+       (1) gettmarg() handles bounds-checking.
+       (2) Python's acceptable range is one greater than the range in C,
+       thus need to check against automatic decrement by gettmarg().
+    */
+    if (buf->tm_mon == -1)
+        buf->tm_mon = 0;
+    else if (buf->tm_mon < 0 || buf->tm_mon > 11) {
+        PyErr_SetString(PyExc_ValueError, "month out of range");
+        return 0;
+    }
+    if (buf->tm_mday == 0)
+        buf->tm_mday = 1;
+    else if (buf->tm_mday < 0 || buf->tm_mday > 31) {
+        PyErr_SetString(PyExc_ValueError, "day of month out of range");
+        return 0;
+    }
+    if (buf->tm_hour < 0 || buf->tm_hour > 23) {
+        PyErr_SetString(PyExc_ValueError, "hour out of range");
+        return 0;
+    }
+    if (buf->tm_min < 0 || buf->tm_min > 59) {
+        PyErr_SetString(PyExc_ValueError, "minute out of range");
+        return 0;
+    }
+    if (buf->tm_sec < 0 || buf->tm_sec > 61) {
+        PyErr_SetString(PyExc_ValueError, "seconds out of range");
+        return 0;
+    }
+    /* tm_wday does not need checking of its upper-bound since taking
+    ``% 7`` in gettmarg() automatically restricts the range. */
+    if (buf->tm_wday < 0) {
+        PyErr_SetString(PyExc_ValueError, "day of week out of range");
+        return 0;
+    }
+    if (buf->tm_yday == -1)
+        buf->tm_yday = 0;
+    else if (buf->tm_yday < 0 || buf->tm_yday > 365) {
+        PyErr_SetString(PyExc_ValueError, "day of year out of range");
+        return 0;
+    }
+    return 1;
+}
+
 #ifdef HAVE_STRFTIME
 static PyObject *
 time_strftime(PyObject *self, PyObject *args)
@@ -407,8 +477,10 @@ time_strftime(PyObject *self, PyObject *args)
     if (tup == NULL) {
         time_t tt = time(NULL);
         buf = *localtime(&tt);
-    } else if (!gettmarg(tup, &buf))
+    } else if (!gettmarg(tup, &buf)
+               || !checktm(&buf)) {
         return NULL;
+    }
 
     /* Checks added to make sure strftime() does not crash Python by
        indexing blindly into some array for a textual representation
@@ -558,27 +630,51 @@ Parse a string to a time tuple according to a format specification.\n\
 See the library reference manual for formatting codes (same as strftime()).");
 
 
+static PyObject *
+_asctime(struct tm *timeptr)
+{
+    /* Inspired by Open Group reference implementation available at
+     * http://pubs.opengroup.org/onlinepubs/009695399/functions/asctime.html */
+    static const char wday_name[7][4] = {
+        "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+    };
+    static const char mon_name[12][4] = {
+        "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+        "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+    };
+    PyObject *unicode, *str;
+    /* PyString_FromString() cannot be used because it doesn't support %3d */
+    unicode = PyUnicode_FromFormat(
+        "%s %s%3d %.2d:%.2d:%.2d %d",
+        wday_name[timeptr->tm_wday],
+        mon_name[timeptr->tm_mon],
+        timeptr->tm_mday, timeptr->tm_hour,
+        timeptr->tm_min, timeptr->tm_sec,
+        1900 + timeptr->tm_year);
+    if (unicode == NULL) {
+        return NULL;
+    }
+
+    str = PyUnicode_AsASCIIString(unicode);
+    Py_DECREF(unicode);
+    return str;
+}
+
 static PyObject *
 time_asctime(PyObject *self, PyObject *args)
 {
     PyObject *tup = NULL;
     struct tm buf;
-    char *p;
     if (!PyArg_UnpackTuple(args, "asctime", 0, 1, &tup))
         return NULL;
     if (tup == NULL) {
         time_t tt = time(NULL);
         buf = *localtime(&tt);
-    } else if (!gettmarg(tup, &buf))
-        return NULL;
-    p = asctime(&buf);
-    if (p == NULL) {
-        PyErr_SetString(PyExc_ValueError, "invalid time");
+    } else if (!gettmarg(tup, &buf)
+               || !checktm(&buf)) {
         return NULL;
     }
-    if (p[24] == '\n')
-        p[24] = '\0';
-    return PyString_FromString(p);
+    return _asctime(&buf);
 }
 
 PyDoc_STRVAR(asctime_doc,
@@ -593,7 +689,7 @@ time_ctime(PyObject *self, PyObject *args)
 {
     PyObject *ot = NULL;
     time_t tt;
-    char *p;
+    struct tm *buf;
 
     if (!PyArg_UnpackTuple(args, "ctime", 0, 1, &ot))
         return NULL;
@@ -607,14 +703,16 @@ time_ctime(PyObject *self, PyObject *args)
         if (tt == (time_t)-1 && PyErr_Occurred())
             return NULL;
     }
-    p = ctime(&tt);
-    if (p == NULL) {
-        PyErr_SetString(PyExc_ValueError, "unconvertible time");
-        return NULL;
+    buf = localtime(&tt);
+    if (buf == NULL) {
+#ifdef EINVAL
+        if (errno == 0) {
+            errno = EINVAL;
+        }
+#endif
+        return PyErr_SetFromErrno(PyExc_ValueError);
     }
-    if (p[24] == '\n')
-        p[24] = '\0';
-    return PyString_FromString(p);
+    return _asctime(buf);
 }
 
 PyDoc_STRVAR(ctime_doc,
