[Python-checkins] bpo-31339: Rewrite time.asctime() and time.ctime() (#3293)
Victor Stinner
webhook-mailer at python.org
Tue Sep 5 19:35:42 EDT 2017
https://github.com/python/cpython/commit/eeadf5fc231163ec97a8010754d9c995c7c14876
commit: eeadf5fc231163ec97a8010754d9c995c7c14876
branch: 2.7
author: Victor Stinner <victor.stinner at gmail.com>
committer: GitHub <noreply at github.com>
date: 2017-09-06T01:35:39+02:00
summary:
bpo-31339: Rewrite time.asctime() and time.ctime() (#3293)
* 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()
files:
A Misc/NEWS.d/next/Security/2017-09-04-21-24-51.bpo-31339.YSczZN.rst
M Lib/test/test_time.py
M Modules/timemodule.c
diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py
index 4571c108d67..4da6703c56c 100644
--- 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
new file mode 100644
index 00000000000..a02a407b5d6
--- /dev/null
+++ 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
index 12c43b08fe4..61b8d612a4a 100644
--- 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
@@ -559,26 +631,50 @@ 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,
More information about the Python-checkins
mailing list