Have strftime() check its time tuple argument to make sure the tuple's values

are within proper boundaries as specified in the docs.

This can break possible code (datetime module needed changing, for instance)
that uses 0 for values that need to be greater 1 or greater (month, day, and
day of year).

Fixes bug #897625.

diff --git a/Doc/lib/libtime.tex b/Doc/lib/libtime.tex
index 2769452947243d1f0ae913a60dcd1f502612ca81..87dd271333a12a8974a016f2d72eba3373d044c7 100644 (file)
--- a/Doc/lib/libtime.tex
+++ b/Doc/lib/libtime.tex
@@ -211,8 +211,11 @@ Convert a tuple or \class{struct_time} representing a time as returned
 by \function{gmtime()} or \function{localtime()} to a string as
 specified by the \var{format} argument.  If \var{t} is not
 provided, the current time as returned by \function{localtime()} is
-used.  \var{format} must be a string.
+used.  \var{format} must be a string.  \exception{ValueError} is raised
+if any field in \var{t} is outside of the allowed range.
 \versionchanged[Allowed \var{t} to be omitted]{2.1}
+\versionchanged[\exception{ValueError} raised if a field in \var{t} is
+out of range.]{2.4}
 
 The following directives can be embedded in the \var{format} string.
 They are shown without the optional field width and precision

diff --git a/Lib/test/test_strftime.py b/Lib/test/test_strftime.py
index 9bd045d8944786527a6be014eace29cf3600891b..44e2ae2b0361126ce9ea2f0b451b6f61791b50c8 100755 (executable)
--- a/Lib/test/test_strftime.py
+++ b/Lib/test/test_strftime.py
@@ -38,7 +38,7 @@ def strftest(now):
     if now[3] < 12: ampm='(AM|am)'
     else: ampm='(PM|pm)'
 
-    jan1 = time.localtime(time.mktime((now[0], 1, 1) + (0,)*6))
+    jan1 = time.localtime(time.mktime((now[0], 1, 1, 0, 0, 0, 0, 1, 0)))
 
     try:
         if now[8]: tz = time.tzname[1]

diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py
index 4b9ed99a5f4f0d119907e019ec856ca9a4ba9356..9e16d0b12222c37fe13c6af13129acc59a76eec9 100644 (file)
--- a/Lib/test/test_time.py
+++ b/Lib/test/test_time.py
@@ -37,6 +37,62 @@ class TimeTestCase(unittest.TestCase):
             except ValueError:
                 self.fail('conversion specifier: %r failed.' % format)
 
+    def test_strftime_bounds_checking(self):
+        # Make sure that strftime() checks the bounds of the various parts
+        #of the time tuple.
+
+        # Check year
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1899, 1, 1, 0, 0, 0, 0, 1, -1))
+        if time.accept2dyear:
+            self.assertRaises(ValueError, time.strftime, '',
+                                (-1, 1, 1, 0, 0, 0, 0, 1, -1))
+            self.assertRaises(ValueError, time.strftime, '',
+                                (100, 1, 1, 0, 0, 0, 0, 1, -1))
+        # Check month
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 0, 1, 0, 0, 0, 0, 1, -1))
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 13, 1, 0, 0, 0, 0, 1, -1))
+        # Check day of month
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 0, 0, 0, 0, 0, 1, -1))
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 32, 0, 0, 0, 0, 1, -1))
+        # Check hour
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 1, -1, 0, 0, 0, 1, -1))
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 1, 24, 0, 0, 0, 1, -1))
+        # Check minute
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 1, 0, -1, 0, 0, 1, -1))
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 1, 0, 60, 0, 0, 1, -1))
+        # Check second
+        self.assertRaises(ValueError, time.strftime, '',
+                            (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)
+        self.assertRaises(ValueError, time.strftime, '',
+                            (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.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 1, 0, 0, 0, -2, 1, -1))
+        # Check day of the year
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 1, 0, 0, 0, 0, 0, -1))
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 1, 0, 0, 0, 0, 367, -1))
+        # Check daylight savings flag
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 1, 0, 0, 0, 0, 1, -2))
+        self.assertRaises(ValueError, time.strftime, '',
+                            (1900, 1, 1, 0, 0, 0, 0, 1, 2))
+
     def test_strptime(self):
         tt = time.gmtime(self.t)
         for directive in ('a', 'A', 'b', 'B', 'c', 'd', 'H', 'I',

diff --git a/Misc/NEWS b/Misc/NEWS
index 2d02ed1ca6b6c6768a7fe509f0e9441a7cc13fb8..892db2987ec8e73acf28b9cf32654aae43536eda 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -167,6 +167,13 @@ Core and builtins
 Extension modules
 -----------------
 
+- time.strftime() now checks that the values in its time tuple argument
+  are within the proper boundaries to prevent possible crashes from the
+  platform's C library implementation of strftime().  Can possibly
+  break code that uses values outside the range that didn't cause
+  problems previously (such as sitting day of year to 0).  Fixes bug
+  #897625.
+
 - The socket module now supports Bluetooth sockets, if the
   system has <bluetooth/bluetooth.h>
 

diff --git a/Modules/datetimemodule.c b/Modules/datetimemodule.c
index 3de1c65edb17b93e8777891a4e0d5403e1c3c546..c68c368b7ddcfbe0d59cc9af46f7d9813246f597 100644 (file)
--- a/Modules/datetimemodule.c
+++ b/Modules/datetimemodule.c
@@ -3189,11 +3189,11 @@ time_strftime(PyDateTime_Time *self, PyObject *args, PyObject *kw)
         * 1900 to worm around that.
         */
        tuple = Py_BuildValue("iiiiiiiii",
-                             1900, 0, 0, /* year, month, day */
+                             1900, 1, 1, /* year, month, day */
                              TIME_GET_HOUR(self),
                              TIME_GET_MINUTE(self),
                              TIME_GET_SECOND(self),
-                             0, 0, -1); /* weekday, daynum, dst */
+                             0, 1, -1); /* weekday, daynum, dst */
        if (tuple == NULL)
                return NULL;
        assert(PyTuple_Size(tuple) == 9);

diff --git a/Modules/timemodule.c b/Modules/timemodule.c
index d60f32038a5eac0a21632128c95dd452020519cd..ef6ee3e229b826454ef1760320992ce7a8ed6f59 100644 (file)
--- a/Modules/timemodule.c
+++ b/Modules/timemodule.c
@@ -346,6 +346,48 @@ time_strftime(PyObject *self, PyObject *args)
        } else if (!gettmarg(tup, &buf))
                return NULL;
 
+        /* Checks added to make sure strftime() does not crash Python by
+            indexing blindly into some array for a textual representation
+            by some bad index (fixes bug #897625).
+        
+            No check for year since handled in gettmarg().
+        */
+        if (buf.tm_mon < 0 || buf.tm_mon > 11) {
+            PyErr_SetString(PyExc_ValueError, "month out of range");
+                        return NULL;
+        }
+        if (buf.tm_mday < 1 || buf.tm_mday > 31) {
+            PyErr_SetString(PyExc_ValueError, "day of month out of range");
+                        return NULL;
+        }
+        if (buf.tm_hour < 0 || buf.tm_hour > 23) {
+            PyErr_SetString(PyExc_ValueError, "hour out of range");
+            return NULL;
+        }
+        if (buf.tm_min < 0 || buf.tm_min > 59) {
+            PyErr_SetString(PyExc_ValueError, "minute out of range");
+            return NULL;
+        }
+        if (buf.tm_sec < 0 || buf.tm_sec > 61) {
+            PyErr_SetString(PyExc_ValueError, "seconds out of range");
+            return NULL;
+        }
+        /* 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 NULL;
+        }
+        if (buf.tm_yday < 0 || buf.tm_yday > 365) {
+            PyErr_SetString(PyExc_ValueError, "day of year out of range");
+            return NULL;
+        }
+        if (buf.tm_isdst < -1 || buf.tm_isdst > 1) {
+            PyErr_SetString(PyExc_ValueError,
+                            "daylight savings flag out of range");
+            return NULL;
+        }
+
        fmtlen = strlen(fmt);
 
        /* I hate these functions that presume you know how big the output